Re: Use after free in __dst_destroy_metrics_generic

	Hello,

On Fri, 15 Sep 2017, Eric Dumazet wrote:

On Fri, 2017-09-08 at 09:10 -0700, Cong Wang wrote:

quoted

On Thu, Sep 7, 2017 at 5:52 PM, Subash Abhinov Kasiviswanathan
[off-list ref] wrote:

quoted

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!

Hi Cong

I believe your patch makes a lot of sense, please submit it formally ?

	Cong's patch is wrong for few reasons:

- it will stop to kfree non-refcounted metrics

- report was for IPV6 and we set DST_METRICS_REFCOUNTED only
for IPv4, for DST_METRICS_READ_ONLY metrics

- __dst_destroy_metrics_generic is called for val without
DST_METRICS_READ_ONLY flag and such metrics are not with
DST_METRICS_REFCOUNTED flag

- ->cow_metrics and dst_cow_metrics_generic are called with 
DST_METRICS_READ_ONLY flag set, there is no chance to write
new value twice, especially to write DST_METRICS_REFCOUNTED flag
and later to see this flag in __dst_destroy_metrics_generic

	So, I'm not sure where exactly is the bug with the
metrics.

	May be I'm missing some posting but I don't see if
the patch was tested successfully.

Regards