Re: fanotify use after free.

From: Linus Torvalds
Date: Wed Jan 22 2014 - 13:20:10 EST


On Tue, Jan 21, 2014 at 10:27 PM, Dave Jones <davej@xxxxxxxxxx> wrote:
>
> BUG fanotify_event_info (Not tainted): Poison overwritten

Looking at the poison data, it seems that is is the

u32 response;

field that has been overwritten (with all zero).

That doesn't really help me guess where the bug is, though. That code
is crazy. For example, looking at one place where it is set, we have:

- process_access_response():

re->event->response = response;

wake_up(&group->fanotify_data.access_waitq);

kmem_cache_free(fanotify_response_event_cache, re);

which looks buggy in *so* many ways. In particular, we're doing a
kmem_cache_free() on "re", but what happened to "re->event" that we
just used? There was no release of that anywhere. Wut?

So it seems that the lifetime of these "fanotify_event_info"
structures is completely buggered. I don't even see any *attempt* to
maintain reference counts or other lifetime info. People free the
containers that point to them without doing anything at all about the
fsnotify_event that contains the fanotify_event_info that they point
to.

Jan - how is the lifetime of the fanotify_event_info tied to the
lifetime of the fanotify_response_event structure that contains
pointers into it? Because I don't see it.

And considering that it's the response field that gets overwritten, it
really sounds like *that* is the exact issue at play here - there is
some fanotify_response_event structure holding a pointer to the
fanotify_event that is embedded into a fanotify_event_info that has
been freed.

Jan?

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/