[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200128214809.GA27151@openwall.com>
Date: Tue, 28 Jan 2020 22:48:10 +0100
From: Solar Designer <solar@...nwall.com>
To: oss-security@...ts.openwall.com
Cc: Al Viro <viro@...iv.linux.org.uk>,
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: Linux kernel: user-triggerable read-after-free crash or 1-bit infoleak oracle in open(2)
Hi,
First, to avoid such questions or potential duplicate CVE ID assignment:
I intend to request a CVE ID and post it as a follow-up to this thread.
[ I use square braces in the message below to describe issues that are
related to the bug mentioned in the Subject, yet are distinct from it. ]
Al Viro found and analyzed the security impact of and fixed a bug in
Linux 4.19+ where open(2)'s eventual call to may_create_in_sticky() was
"done when we already have dropped the reference to dir" and thus with
dir (a "struct dentry" pointer) being potentially stale and potentially
pointing to reused memory. This (presumably) can happen e.g. in the
unusual case that the file being opened gets moved to a different
directory and its old parent directory gets removed concurrently with
the open(2) call being still in progress. (I am not aware of attempts
to trigger this problematic behavior. If anyone tries and has either
kind of results, please post a follow-up to this thread.)
may_create_in_sticky() performs reads via the dir pointer and returns
either 0 or -EACCES. Thus, the potential impact is either a kernel Oops
or a 1-bit oracle infoleak. It feels like a useful infoleak wouldn't be
easy to trigger, but then many bugs that could have seemed unexploitable
at first were proven otherwise.
Since the reads via dir are of dir->d_inode, which is then dereferenced
and further reads are performed, I wonder if behavior (hopefully just
DoS) worse than Oops is also possible when dir->d_inode hits a device's
MMIO range. This feels almost unrealistic, but who knows.
may_create_in_sticky() only reaches any of those reads when the
protected_fifos and/or protected_regular sysctl's are set to non-zero,
or (something I just discovered when writing this message) when the file
is neither a FIFO nor a regular file.
[ In fact, looks like the "protection" unintentionally applies to such
files (and is always enabled for them), and I guess this remained
unnoticed because such files are rare in world-writable sticky
directories, and in the case of Unix domain sockets they're normally
connect()'ed to rather than open()'ed. Also, there's a check for the
file to be opened not being a directory before the call to
may_create_in_sticky() is made. ]
Al also pointed out that the two sysctl's are enabled by default with
systemd 241 and later.
With the new finding above, the bug might be triggerable by a user
(e.g., by creating and trying to open() a Unix domain socket maybe?) on
all systems with Linux 4.19+, until Al's recent fix - not just on those
that enabled the sysctl's. Nasty.
The bug was introduced with commit 30aba6656f61 and first included in
Linux 4.19. Al fixed it with commit d0cb50185ae9 two days ago, and the
fix is already in Linux 5.5 and Greg KH is getting it into stable.
[ Another issue Al pointed out is that these sysctl's are ineffective on
filesystems that use ->atomic_open() in the kernel, such as (but not
only) networked filesystems. That's a different code path, bypassing
the check. We need to document this limitation at the very least. ]
Al also shared the story of this bug's discovery during untangling of
control flow in do_last() and refactoring the code, which appeared to be
so complicated that merely reading the code hadn't resulted in the
discovery.
I wonder if a fuzzer is able to trigger bugs like this, or if a fuzzer
should be taught to test for such bugs - e.g., explicitly try moving
files being accessed and removing their former parent directories -
this might be too specialized, but on the other hand it wouldn't be
limited to open(2).
I also think we might want to annotate dereferences of pointers to
objects that require pinning, so that a debugging build of the kernel
could check for and complain when the accessed object is not pinned.
This would work (at least) when the memory is not yet reused, which
means in the majority of cases - and precisely in those that would go
unnoticed without such checking. I think this change would involve a
lot of source code edits (nasty!), but is otherwise non-invasive - in
fact, non-debugging builds could produce the same binaries that they
produce now. In other words, making this change is complicated, but not
complex, and the resulting source code should be neither (I imagine it
can be just uses of a macro in place of explicit pointer dereferences).
My sentiment and other thoughts regarding bug(s) introduced with commit
30aba6656f61 are as follows:
This makes me sad and sorry. I had introduced the "protected FIFOs"
feature in -ow patches for Linux 2.0 over 20 years ago. (Obviously, the
implementation was different.) When hardening changes started getting
merged into upstream Linux kernel in the last few years, I mentioned
that one and also that regular files could be used for similar attacks
and that maybe we could afford to restrict them too:
https://www.openwall.com/lists/kernel-hardening/2017/06/24/4
However, I had mostly moved on (hadn't been seriously working on kernel
hardening for years), so I didn't accept working on getting these
changes upstream myself (if upstream were interested in these changes in
the late 1990s, I probably would). So another contributor worked on the
new changes and getting them accepted. I provided some advice, but
(now) evidently didn't review the changes in context and to a sufficient
extent (and accordingly am on Suggested-by, but not Reviewed-by). With
code tangled like that a mere code review might not have helped, but
there's a chance it might have.
So we got a userspace hardening feature into the kernel, which we now
know also introduced a security vulnerability into the kernel. Ouch.
I am not complaining and not blaming anyone. Rather, I accept that I
share responsibility for the bug(s).
I also greatly appreciate Al's cleaning up this mess, and am sorry we
had created more work for him.
Alexander
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.
