Date: Sun, 2 Feb 2020 15:10:30 +0000 From: Al Viro <viro@...iv.linux.org.uk> To: Solar Designer <solar@...nwall.com> Cc: oss-security@...ts.openwall.com, Salvatore Mesoraca <s.mesoraca16@...il.com>, Kees Cook <keescook@...omium.org>, Linus Torvalds <torvalds@...ux-foundation.org>, Dan Carpenter <dan.carpenter@...cle.com>, Andrew Morton <akpm@...ux-foundation.org> Subject: Re: Linux kernel: user-triggerable read-after-free crash or 1-bit infoleak oracle in open(2) On Sun, Feb 02, 2020 at 01:22:11PM +0100, Solar Designer wrote: > If I understand correctly (but I'm not confident!), this time it's just > a crash. I am not going to request another CVE ID because the security > impact is unclear to me (perhaps an Oops with some resources held?) Well, it won't be dereferencing anything freed - if we are in RCU mode, we have observed (with sufficient barriers) an earlier value of ->d_inode of the same dentry after rcu_read_lock(), and freeing of inodes that ever had been anyone's ->d_inode is guaranteed to be RCU-delayed. So we can see NULL there, but any non-NULL will be a yet-to-be-freed struct inode. IOW, we can hit NULL pointer derefs there (with small offsets) and the values we try to fetch are not pointers themselves. We can't hit iomem or anything of that sort; if the attacker has managed to mmap something at virtual address 0, they can get garbage into dir_uid and dir_mode, but that'll only get may_create_in_sticky() produce a wrong value and since they'd been able to rmdir the parent, they controlled what may_create_in_sticky() would've produced anyway. As for the resources held... Not in open(2) case (we are not holding any locks at that point); some driver calling filp_open() with some locks held is theoretically possible, but I doubt that it's a viable attack. And if attacker can play silly buggers with e.g. directory a driver loads firmware from, we have a much worse problem. Triggering that in coredump... not sure; you'll probably get some stuck threads, can't tell for certain how much would be possible to get pinned down that way. > The kernel uses many complicated conventions these days (for performance > reasons), up to the point where it's difficult even for the most active > upstream developers to make bug-free changes and to review proposed > changes. A lot of context needs to be considered and a lot of potential > pitfalls kept in mind. do_last()/lookup_open()/atomic_open() is the single worst place in fs/namei.c in terms of complexity, and one of the worst places anywhere in VFS. It needs to be cleaned up; FWIW, the tentative cleanups I have in the local tree (hadn't published that branch yet) get it somewhat saner. Incidentally, do_last() in there did get those fetches right. Not a deliberate fix - it's just that 'dir' got buried in open_last_lookups(), so there was no temptation to try and use that pathway to the fields we want to read. Still not the final variant - I want to take may_open()/vfs_open()/ima_file_check() out of scope of mnt_want_write()/mnt_drop_write() pair, provided that I can prove correctness of such massage; got sidetracked by other stuff last few days... > Thanks to @grsecurity for at-mentioning me on the tweet pointing to the > above commit. I was otherwise out of the loop this time. That's fine, > but since I did bring the previous set of issues in here, I felt I also > needed to post this follow-up. My apologies; should've Cc'd you on that one.
Powered by blists - more mailing lists
Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.