Re: Use after free in __dst_destroy_metrics_generic

[Cong Wang]

On Thu, Sep 7, 2017 at 5:52 PM, Subash Abhinov Kasiviswanathan
<subas...@codeaurora.org> wrote:
> We are seeing a possible use after free in ip6_dst_destroy.
>
> It appears as if memory of the __DST_METRICS_PTR(old) was freed in some path
> and allocated
> to ion driver. ion driver has also freed it. Finally the memory is freed by
> the
> fib gc and crashes since it is already deallocated.

Does the attach (compile-only) patch help anything?

>From my _quick_ glance, it seems we miss the refcnt'ing
right in __dst_destroy_metrics_generic().

Thanks!

diff --git a/net/core/dst.c b/net/core/dst.c
index 00aa972ad1a1..b293aeae3018 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -241,8 +241,14 @@ void __dst_destroy_metrics_generic(struct dst_entry *dst, 
unsigned long old)
 
        new = ((unsigned long) &dst_default_metrics) | DST_METRICS_READ_ONLY;
        prev = cmpxchg(&dst->_metrics, old, new);
-       if (prev == old)
-               kfree(__DST_METRICS_PTR(old));
+       if (prev == old) {
+               struct dst_metrics *old_p = (struct dst_metrics 
*)__DST_METRICS_PTR(old);
+
+               if (prev & DST_METRICS_REFCOUNTED) {
+                       if (atomic_dec_and_test(&old_p->refcnt))
+                               kfree(old_p);
+               }
+       }
 }
 EXPORT_SYMBOL(__dst_destroy_metrics_generic);