linux-kernel - Re: fanotify use after free.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140122233622.GB27916@quack.suse.cz>
Date:	Thu, 23 Jan 2014 00:36:22 +0100
From:	Jan Kara <jack@...e.cz>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Dave Jones <davej@...hat.com>, Jan Kara <jack@...e.cz>,
	Linux Kernel <linux-kernel@...r.kernel.org>, jkosina@...e.cz
Subject: Re: fanotify use after free.

On Wed 22-01-14 10:20:01, Linus Torvalds wrote:
> On Tue, Jan 21, 2014 at 10:27 PM, Dave Jones <davej@...hat.com> 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.
  Yeah, I messed that up. They aren't tied in any way - well, in fact they
end up being tied but in a wrong way. fanotify_event_info lives from the
moment event happens to the moment user reads the event. At that moment the
fanotify_response_event gets created (for those special permission events),
pointing to fanotify_event_info which will get freed just several lines
further :-|

But refcounting seems like an overkill for this - there is exactly one
fanotify_response_event structure iff it is a permission event. So
something like the (completely untested) attached patch should fix the
problem. But I agree it's a bit ugly so we might want something different.
I'll try to think about something better tomorrow.

> 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.

								Honza
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR

View attachment "fanotify_corruption.diff" of type "text/x-patch" (1099 bytes)