From: Christian Brauner <christian.brauner@ubuntu.com>
To: naresh.kamboju@linaro.org
Cc: ard.biesheuvel@linaro.org, ardb@kernel.org,
christian.brauner@ubuntu.com, gregkh@linuxfoundation.org,
john.stultz@linaro.org, keescook@chromium.org,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
lkft-triage@lists.linaro.org, shuah@kernel.org, tkjos@google.com,
stable@vger.kernel.org
Subject: [PATCH] binderfs: use refcount for binder control devices too
Date: Wed, 11 Mar 2020 11:53:09 +0100 [thread overview]
Message-ID: <20200311105309.1742827-1-christian.brauner@ubuntu.com> (raw)
In-Reply-To: <CA+G9fYusdfg7PMfC9Xce-xLT7NiyKSbgojpK35GOm=Pf9jXXrA@mail.gmail.com>
Binderfs binder-control devices are cleaned up via binderfs_evict_inode
too() which will use refcount_dec_and_test(). However, we missed to set
the refcount for binderfs binder-control devices and so we underflowed
when the binderfs instance got unmounted. Pretty obvious oversight and
should have been part of the more general UAF fix. The good news is that
having test cases (suprisingly) helps.
Technically, we could detect that we're about to cleanup the
binder-control dentry in binderfs_evict_inode() and then simply clean it
up. But that makes the assumption that the binder driver itself will
never make use of a binderfs binder-control device after the binderfs
instance it belongs to has been unmounted and the superblock for it been
destroyed. While it is unlikely to ever come to this let's be on the
safe side. Performance-wise this also really doesn't matter since the
binder-control device is only every really when creating the binderfs
filesystem or creating additional binder devices. Both operations are
pretty rare.
Fixes: f0fe2c0f050d ("binder: prevent UAF for binderfs devices II")
Link: https://lore.kernel.org/r/CA+G9fYusdfg7PMfC9Xce-xLT7NiyKSbgojpK35GOm=Pf9jXXrA@mail.gmail.com
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Cc: stable@vger.kernel.org
Cc: Todd Kjos <tkjos@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
drivers/android/binderfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 110e41f920c2..f303106b3362 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -448,6 +448,7 @@ static int binderfs_binder_ctl_create(struct super_block *sb)
inode->i_uid = info->root_uid;
inode->i_gid = info->root_gid;
+ refcount_set(&device->ref, 1);
device->binderfs_inode = inode;
device->miscdev.minor = minor;
base-commit: 2c523b344dfa65a3738e7039832044aa133c75fb
--
2.25.1
next prev parent reply other threads:[~2020-03-11 10:53 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-11 7:52 WARNING: at refcount.c:190 refcount_sub_and_test_checked+0xac/0xc8 - refcount_t: underflow; use-after-free Naresh Kamboju
2020-03-11 7:52 ` Naresh Kamboju
2020-03-11 9:13 ` Christian Brauner
2020-03-11 9:13 ` Christian Brauner
2020-03-11 10:53 ` Christian Brauner [this message]
2020-03-11 18:25 ` [PATCH] binderfs: use refcount for binder control devices too Todd Kjos
2020-03-12 13:15 ` [PATCH 1/3] binderfs: port tests to test harness infrastructure Christian Brauner
2020-03-12 13:15 ` [PATCH 2/3] binderfs: add stress test for binderfs binder devices Christian Brauner
2020-03-12 23:53 ` Kees Cook
2020-03-13 12:54 ` Christian Brauner
2020-03-12 13:15 ` [PATCH 3/3] binderfs_test: switch from /dev to /tmp as mountpoint Christian Brauner
2020-03-12 23:54 ` Kees Cook
2020-03-13 12:55 ` Christian Brauner
2020-03-12 21:24 ` [PATCH] binderfs: port to new mount api Christian Brauner
2020-03-12 23:56 ` Kees Cook
2020-03-13 12:55 ` Christian Brauner
2020-03-13 12:56 ` Christian Brauner
2020-03-12 23:51 ` [PATCH 1/3] binderfs: port tests to test harness infrastructure Kees Cook
2020-03-13 15:24 ` [PATCH v2 " Christian Brauner
2020-03-13 15:24 ` [PATCH v2 2/3] binderfs_test: switch from /dev to a unique per-test mountpoint Christian Brauner
2020-03-13 23:07 ` Kees Cook
2020-03-13 15:24 ` [PATCH v2 3/3] binderfs: add stress test for binderfs binder devices Christian Brauner
2020-03-13 23:08 ` Kees Cook
2020-03-16 22:44 ` Hridya Valsaraju
2020-03-17 8:27 ` Christian Brauner
2020-03-13 23:07 ` [PATCH v2 1/3] binderfs: port tests to test harness infrastructure Kees Cook
2020-03-13 15:34 ` [PATCH v2] binderfs: port to new mount api Christian Brauner
2020-03-13 23:08 ` Kees Cook
2020-03-18 12:29 ` Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200311105309.1742827-1-christian.brauner@ubuntu.com \
--to=christian.brauner@ubuntu.com \
--cc=ard.biesheuvel@linaro.org \
--cc=ardb@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=john.stultz@linaro.org \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=lkft-triage@lists.linaro.org \
--cc=naresh.kamboju@linaro.org \
--cc=shuah@kernel.org \
--cc=stable@vger.kernel.org \
--cc=tkjos@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).