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: <20140128080201.GA13676@quack.suse.cz>
Date:	Tue, 28 Jan 2014 09:02:01 +0100
From:	Jan Kara <jack@...e.cz>
To:	Dave Jones <davej@...hat.com>
Cc:	Jan Kara <jack@...e.cz>, Jiri Kosina <jkosina@...e.cz>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: fanotify use after free.

On Tue 28-01-14 01:10:37, Dave Jones wrote:
> On Tue, Jan 28, 2014 at 12:40:17AM +0100, Jan Kara wrote:
>  > On Fri 24-01-14 08:26:45, Jiri Kosina wrote:
>  > > On Fri, 24 Jan 2014, Jan Kara wrote:
>  > > 
>  > > >   Strange. I've installed systemd system (openSUSE 13.1) and it boots 
>  > > > with the latest Linus' kernel just fine (and I have at least FANOTIFY 
>  > > > and SLAB debugging set the same way as you). But it was only a KVM 
>  > > > guest. I'll try tomorrow with a physical machine I guess.
>  > > 
>  > > FWIW the system I am reliably able to reproduce this on is opensuse 12.3 
>  > > with this systemd version: 
>  > > 
>  > > Version     : 195
>  > > Release     : 13.18.1
>  >   Hum, still no luck with reproduction (either on physical machine or with
>  > KVM). Anyway, I've looked at the code again and the previous patch had a
>  > stupid bug (passing different pointer to fsnotify_destroy_event() than we
>  > should have), plus also the merging function in fanotify was too
>  > aggressive. Can you try the attached patch? It boots for me but that means
>  > nothing since I cannot reproduce the issue... Thanks!
> 
> still not good I'm afraid. I still see corruption very early on in boot
> and now it panics and locks up too.
  Ew, thanks for testing.

> Again, this happens so early that I can't grab it over usb-serial.
> I stuck an mdelay(10000) in the slub corruption detector, and managed
> to grab a photo of the first trace.
> 
> Trace:
> ? preempt_schedule
> lock_acquire
> ? lockref_put_or_lock
> _raw_spin_lock
> ? lockref_put_or_lock
> dput
> path_put
> fanotify_free_event
> fsnotify_destroy_event
> fanotify_handle_event
> ? mntput
> ? path_openat
> ? handle_mm_fault
> send_to_group
> ? fsnotify
> fsnotify
> do_sys_open
> sys_open
> RIP: lock_acquire
> 
>   2b:*	4d 8b 64 c6 08       	mov    0x8(%r14,%rax,8),%r12     <-- trapping instruction
> 
> R14 is 0x6b6b6b6b6b6b6c03, which looks like a use-after-free.
  Yup. But I'm somewhat puzzled by the trace. We crash when calling
fsnotify_destroy_event() from fanotify_handle_event(). The fsnotify code
has been called from do_sys_open() so the event was a 'FS_OPEN' which fails
the fsn_event->mask & FAN_ALL_PERM_EVENTS test.

Slapping my forehead, that's a really stupid bug. The event
fsnotify_add_notify_event() returns may be freed by the time we return
because we already dropped the notification mutex. And then fsn_event->mask
& FAN_ALL_PERM_EVENTS test will pass because FAN_ALL_PERM_EVENTS matches
with the poison pattern 0x6b6b6b6b. So yet another hacked up version of
fanotify fix is attached. And I have to seriously think about use counts
for fanotify version of that struct.

> I also notice you mention SLAB above, but I've been using SLUB. I don't
> know if the choice of allocator makes a difference in reproducability.
  Jiri Kosina has SLAB so SLAB/SLUB apparently doesn't matter.

> It's also worth noting that I have lockdep enabled, which may be perturbing things
> to some degree.
  And I've compiled my kernel with lockdep as well since Jiri has it in his
config. But no luck.

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

View attachment "fanotify.diff" of type "text/x-patch" (1727 bytes)