binderfs: fix use-after-free in binder_devices

Devices created through binderfs are added to the global binder_devices
list but are not removed before being destroyed. This leads to dangling
pointer

binderfs: fix use-after-free in binder_devices

Devices created through binderfs are added to the global binder_devices
list but are not removed before being destroyed. This leads to dangling
pointers in the list and subsequent use-after-free errors:

==================================================================
BUG: KASAN: slab-use-after-free in binder_add_device+0x5c/0x9c
Write of size 8 at addr ffff0000c258d708 by task mount/653

CPU: 7 UID: 0 PID: 653 Comm: mount Not tainted 6.13.0-09030-g6d61a53dd6f5 #1
Hardware name: linux,dummy-virt (DT)
Call trace:
binder_add_device+0x5c/0x9c
binderfs_binder_device_create+0x690/0x84c
[...]
__arm64_sys_mount+0x324/0x3bc

Allocated by task 632:
binderfs_binder_device_create+0x168/0x84c
binder_ctl_ioctl+0xfc/0x184
[...]
__arm64_sys_ioctl+0x110/0x150

Freed by task 649:
kfree+0xe0/0x338
binderfs_evict_inode+0x138/0x1dc
[...]
==================================================================

Remove devices from binder_devices before destroying them.

Cc: Li Li <[email protected]>
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=7015dcf45953112c8b45
Fixes: 12d909cac1e1 ("binderfs: add new binder devices to binder_devices")
Signed-off-by: Carlos Llamas <[email protected]>
Tested-by: [email protected]
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binderfs: add new binder devices to binder_devices

When binderfs is not enabled, the binder driver parses the kernel
config to create all binder devices. All of the new binder devices
are stored in

binderfs: add new binder devices to binder_devices

When binderfs is not enabled, the binder driver parses the kernel
config to create all binder devices. All of the new binder devices
are stored in the list binder_devices.

When binderfs is enabled, the binder driver creates new binder devices
dynamically when userspace applications call BINDER_CTL_ADD ioctl. But
the devices created in this way are not stored in the same list.

This patch fixes that.

Signed-off-by: Li Li <[email protected]>
Acked-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: frozen notification binder_features flag

Add a flag to binder_features to indicate that the freeze notification
feature is available.

Signed-off-by: Yu-Ting Tseng <[email protected]>
A

binder: frozen notification binder_features flag

Add a flag to binder_features to indicate that the freeze notification
feature is available.

Signed-off-by: Yu-Ting Tseng <[email protected]>
Acked-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

android: removed duplicate linux/errno

There are two linux/errno.h inclusions in this file. The second one has
been removed and the file builds correctly.

Fixes: 54ffdab82080 ("android: binder: bin

android: removed duplicate linux/errno

There are two linux/errno.h inclusions in this file. The second one has
been removed and the file builds correctly.

Fixes: 54ffdab82080 ("android: binder: binderfs.c: removed asm-generic/errno-base.h")
Reviewed-by: Nick Desaulniers <[email protected]>
Tested-by: Nick Desaulniers <[email protected]>
Signed-off-by: Tanzir Hasan <[email protected]>
Acked-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

android: binder: binderfs.c: removed asm-generic/errno-base.h

asm-generic/errno-base.h can be replaced by linux/errno.h and the file
will still build correctly. It is an asm-generic file which shoul

android: binder: binderfs.c: removed asm-generic/errno-base.h

asm-generic/errno-base.h can be replaced by linux/errno.h and the file
will still build correctly. It is an asm-generic file which should be
avoided if possible.

Suggested-by: Al Viro <[email protected]>
Signed-off-by: Tanzir Hasan <[email protected]>
Acked-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

android: convert to new timestamp accessors

Convert to using the new inode timestamp accessor functions.

Signed-off-by: Jeff Layton <[email protected]>
Link: https://lore.kernel.org/r/202310041853

android: convert to new timestamp accessors

Convert to using the new inode timestamp accessor functions.

Signed-off-by: Jeff Layton <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Christian Brauner <[email protected]>

show more ...

binderfs: fix typo in binderfs.c

The word "wich"
was corrected to "which" for spelling accuracy.

Signed-off-by: Kuan-Wei Chiu <[email protected]>
Acked-by: Carlos Llamas <[email protected]>
Li

binderfs: fix typo in binderfs.c

The word "wich"
was corrected to "which" for spelling accuracy.

Signed-off-by: Kuan-Wei Chiu <[email protected]>
Acked-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binderfs: Drop unused #include <linux/radix-tree.h>

binderfs.c doens't use any of the symbols provided by linux/radix-tree.h
and compiles just fine without this include. So drop the #include.

Signe

binderfs: Drop unused #include <linux/radix-tree.h>

binderfs.c doens't use any of the symbols provided by linux/radix-tree.h
and compiles just fine without this include. So drop the #include.

Signed-off-by: Uwe Kleine-König <[email protected]>
Acked-by: Christian Brauner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binderfs: convert to ctime accessor functions

In later patches, we're going to change how the inode's ctime field is
used. Switch to using accessor functions instead of raw accesses of
inode->i_ctim

binderfs: convert to ctime accessor functions

In later patches, we're going to change how the inode's ctime field is
used. Switch to using accessor functions instead of raw accesses of
inode->i_ctime.

Reviewed-by: Jan Kara <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
Message-Id: <[email protected]>
Signed-off-by: Christian Brauner <[email protected]>

show more ...

android: fix W=1 kernel-doc warnings

Clean up kernel-doc notation, use correct function and parameter
names.

drivers/android/binderfs.c:236: warning: expecting prototype for binderfs_ctl_ioctl(). P

android: fix W=1 kernel-doc warnings

Clean up kernel-doc notation, use correct function and parameter
names.

drivers/android/binderfs.c:236: warning: expecting prototype for binderfs_ctl_ioctl(). Prototype was for binder_ctl_ioctl() instead
drivers/android/binder.c:386: warning: expecting prototype for binder_node_unlock(). Prototype was for binder_node_inner_unlock() instead
drivers/android/binder.c:1206: warning: expecting prototype for binder_dec_ref(). Prototype was for binder_dec_ref_olocked() instead
drivers/andrond/binder.c:284: warning: Excess function parameter 'proc' description in 'binder_proc_unlock'
drivers/andrond/binder.c:387: warning: expecting prototype for binder_node_unlock(). Prototype was for binder_node_inner_unlock() instead

Signed-off-by: Randy Dunlap <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Arve Hjønnevåg <[email protected]>
Cc: Todd Kjos <[email protected]>
Cc: Martijn Coenen <[email protected]>
Cc: Joel Fernandes <[email protected]>
Cc: Christian Brauner <[email protected]>
Cc: Carlos Llamas <[email protected]>
Cc: Suren Baghdasaryan <[email protected]>
Acked-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

fs: port ->rename() to pass mnt_idmap

Convert to struct mnt_idmap.

Last cycle we merged the necessary infrastructure in
256c8aed2b42 ("fs: introduce dedicated idmap type for mounts").
This is just

fs: port ->rename() to pass mnt_idmap

Convert to struct mnt_idmap.

Last cycle we merged the necessary infrastructure in
256c8aed2b42 ("fs: introduce dedicated idmap type for mounts").
This is just the conversion to struct mnt_idmap.

Currently we still pass around the plain namespace that was attached to a
mount. This is in general pretty convenient but it makes it easy to
conflate namespaces that are relevant on the filesystem with namespaces
that are relevent on the mount level. Especially for non-vfs developers
without detailed knowledge in this area this can be a potential source for
bugs.

Once the conversion to struct mnt_idmap is done all helpers down to the
really low-level helpers will take a struct mnt_idmap argument instead of
two namespace arguments. This way it becomes impossible to conflate the two
eliminating the possibility of any bugs. All of the vfs and all filesystems
only operate on struct mnt_idmap.

Acked-by: Dave Chinner <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Christian Brauner (Microsoft) <[email protected]>

show more ...

binderfs: rework superblock destruction

So far we relied on
.put_super = binderfs_put_super()
to destroy info we stashed in sb->s_fs_info. This gave us the required ordering
between ->evict_inode()

binderfs: rework superblock destruction

So far we relied on
.put_super = binderfs_put_super()
to destroy info we stashed in sb->s_fs_info. This gave us the required ordering
between ->evict_inode() and sb->s_fs_info destruction.

But the current implementation of binderfs_fill_super() has a memory leak in
the rare circumstance that d_make_root() fails because ->put_super() is only
called when sb->s_root is initialized. Fix this by removing ->put_super() and
simply do all that work in binderfs_kill_super().

Reported-by: Dongliang Mu <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Signed-off-by: Christian Brauner (Microsoft) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binderfs: remove unused INTSTRLEN macro

Fix the following W=1 build error:

drivers/android/binderfs.c:42: error: macro "INTSTRLEN" is not used [-Werror=unused-macros]
42 | #define INTSTRLEN 21

binderfs: remove unused INTSTRLEN macro

Fix the following W=1 build error:

drivers/android/binderfs.c:42: error: macro "INTSTRLEN" is not used [-Werror=unused-macros]
42 | #define INTSTRLEN 21
|

No functional changes in this patch.

Reviewed-by: Christian Brauner (Microsoft) <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix redefinition of seq_file attributes

The patchset in [1] exported some definitions to binder_internal.h in
order to make the debugfs entries such as 'stats' and 'transaction_log'
availabl

binder: fix redefinition of seq_file attributes

The patchset in [1] exported some definitions to binder_internal.h in
order to make the debugfs entries such as 'stats' and 'transaction_log'
available in a binderfs instance. However, the DEFINE_SHOW_ATTRIBUTE
macro expands into a static function/variable pair, which in turn get
redefined each time a source file includes this internal header.

This problem was made evident after a report from the kernel test robot
<[email protected]> where several W=1 build warnings are seen in downstream
kernels. See the following example:

include/../drivers/android/binder_internal.h:111:23: warning: 'binder_stats_fops' defined but not used [-Wunused-const-variable=]
111 | DEFINE_SHOW_ATTRIBUTE(binder_stats);
| ^~~~~~~~~~~~
include/linux/seq_file.h:174:37: note: in definition of macro 'DEFINE_SHOW_ATTRIBUTE'
174 | static const struct file_operations __name ## _fops = { \
| ^~~~~~

This patch fixes the above issues by moving back the definitions into
binder.c and instead creates an array of the debugfs entries which is
more convenient to share with binderfs and iterate through.

[1] https://lore.kernel.org/all/[email protected]/

Fixes: 0e13e452dafc ("binder: Add stats, state and transactions files")
Fixes: 03e2e07e3814 ("binder: Make transaction_log available in binderfs")
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binderfs: add extended_error feature entry

Add extended_error to the binderfs feature list, to help userspace
determine whether the BINDER_GET_EXTENDED_ERROR ioctl is supported by
the binder driver.

binderfs: add extended_error feature entry

Add extended_error to the binderfs feature list, to help userspace
determine whether the BINDER_GET_EXTENDED_ERROR ioctl is supported by
the binder driver.

Reviewed-by: Christian Brauner (Microsoft) <[email protected]>
Acked-by: Todd Kjos <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binderfs: add support for feature files

Provide userspace with a mechanism to discover features supported by
the binder driver to refrain from using any unsupported ones in the
first place. Starting

binderfs: add support for feature files

Provide userspace with a mechanism to discover features supported by
the binder driver to refrain from using any unsupported ones in the
first place. Starting with "oneway_spam_detection" only new features
are to be listed under binderfs and all previous ones are assumed to
be supported.

Assuming an instance of binderfs has been mounted at /dev/binderfs,
binder feature files can be found under /dev/binderfs/features/.
Usage example:

$ mkdir /dev/binderfs
$ mount -t binder binder /dev/binderfs
$ cat /dev/binderfs/features/oneway_spam_detection
1

Acked-by: Christian Brauner <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

fs: make helpers idmap mount aware

Extend some inode methods with an additional user namespace argument. A
filesystem that is aware of idmapped mounts will receive the user
namespace the mount has b

fs: make helpers idmap mount aware

Extend some inode methods with an additional user namespace argument. A
filesystem that is aware of idmapped mounts will receive the user
namespace the mount has been marked with. This can be used for
additional permission checking and also to enable filesystems to
translate between uids and gids if they need to. We have implemented all
relevant helpers in earlier patches.

As requested we simply extend the exisiting inode method instead of
introducing new ones. This is a little more code churn but it's mostly
mechanical and doesnt't leave us with additional inode methods.

Link: https://lore.kernel.org/r/[email protected]
Cc: Christoph Hellwig <[email protected]>
Cc: David Howells <[email protected]>
Cc: Al Viro <[email protected]>
Cc: [email protected]
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Christian Brauner <[email protected]>

show more ...

binderfs: make symbol 'binderfs_fs_parameters' static

The sparse tool complains as follows:

drivers/android/binderfs.c:66:32: warning:
symbol 'binderfs_fs_parameters' was not declared. Should it b

binderfs: make symbol 'binderfs_fs_parameters' static

The sparse tool complains as follows:

drivers/android/binderfs.c:66:32: warning:
symbol 'binderfs_fs_parameters' was not declared. Should it be static?

This variable is not used outside of binderfs.c, so this commit
marks it static.

Fixes: 095cf502b31e ("binderfs: port to new mount api")
Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Wei Yongjun <[email protected]>
Acked-by: Christian Brauner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

drivers: android: Fix the SPDX comment style

C source files should have `//` as SPDX comment and not `/**/`. Fix this
by running checkpatch on the file.

Signed-off-by: Mrinal Pandey <mrinalmni@gmai

drivers: android: Fix the SPDX comment style

C source files should have `//` as SPDX comment and not `/**/`. Fix this
by running checkpatch on the file.

Signed-off-by: Mrinal Pandey <[email protected]>
Link: https://lore.kernel.org/r/20200724131449.zvjutbemg3vqhrzh@mrinalpandey
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

drivers: android: Fix a variable declaration coding style issue

Add a blank line after variable declarations as suggested by checkpatch.

Signed-off-by: Mrinal Pandey <[email protected]>
Link: htt

drivers: android: Fix a variable declaration coding style issue

Add a blank line after variable declarations as suggested by checkpatch.

Signed-off-by: Mrinal Pandey <[email protected]>
Link: https://lore.kernel.org/r/20200724131433.stf3ycooogawyzb3@mrinalpandey
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binderfs: remove redundant assignment to pointer ctx

The pointer ctx is being initialized with a value that is never read
and it is being updated later with a new value. The initialization
is redund

binderfs: remove redundant assignment to pointer ctx

The pointer ctx is being initialized with a value that is never read
and it is being updated later with a new value. The initialization
is redundant and can be removed.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <[email protected]>
Acked-by: Christian Brauner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binderfs: Fix binderfs.c selftest compilation warning

Fix missing braces compilation warning in the ARM
compiler environment:
drivers/android/binderfs.c: In function 'binderfs_fill_super':
d

binderfs: Fix binderfs.c selftest compilation warning

Fix missing braces compilation warning in the ARM
compiler environment:
drivers/android/binderfs.c: In function 'binderfs_fill_super':
drivers/android/binderfs.c:650:9: warning: missing braces around initializer [-Wmissing-braces]
struct binderfs_device device_info = { 0 };
drivers/android/binderfs.c:650:9: warning: (near initialization for ‘device_info.name’) [-Wmissing-braces]

Acked-by: Christian Brauner <[email protected]>
Signed-off-by: Tang Bin <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binderfs: port to new mount api

When I first wrote binderfs the new mount api had not yet landed. Now
that it has been around for a little while and a bunch of filesystems
have already been ported w

binderfs: port to new mount api

When I first wrote binderfs the new mount api had not yet landed. Now
that it has been around for a little while and a bunch of filesystems
have already been ported we should do so too. When Al sent his
mount-api-conversion pr he requested that binderfs (and a few others) be
ported separately. It's time we port binderfs. We can make use of the
new option parser, get nicer infrastructure and it will be easier if we
ever add any new mount options.

This survives testing with the binderfs selftests:

for i in `seq 1 1000`; do ./binderfs_test; done

including the new stress tests I sent out for review today:

TAP version 13
1..1
# selftests: filesystems/binderfs: binderfs_test
# [==========] Running 3 tests from 1 test cases.
# [ RUN ] global.binderfs_stress
# [ XFAIL! ] Tests are not run as root. Skipping privileged tests
# [==========] Running 3 tests from 1 test cases.
# [ RUN ] global.binderfs_stress
# [ OK ] global.binderfs_stress
# [ RUN ] global.binderfs_test_privileged
# [ OK ] global.binderfs_test_privileged
# [ RUN ] global.binderfs_test_unprivileged
# # Allocated new binder device with major 243, minor 4, and name my-binder
# # Detected binder version: 8
# [==========] Running 3 tests from 1 test cases.
# [ RUN ] global.binderfs_stress
# [ OK ] global.binderfs_stress
# [ RUN ] global.binderfs_test_privileged
# [ OK ] global.binderfs_test_privileged
# [ RUN ] global.binderfs_test_unprivileged
# [ OK ] global.binderfs_test_unprivileged
# [==========] 3 / 3 tests passed.
# [ PASSED ]
ok 1 selftests: filesystems/binderfs: binderfs_test

Cc: Todd Kjos <[email protected]>
Signed-off-by: Christian Brauner <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binderfs: use refcount for binder control devices too

Binderfs binder-control devices are cleaned up via binderfs_evict_inode
too() which will use refcount_dec_and_test(). However, we missed to set

binderfs: use refcount for binder control devices too

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 <[email protected]>
Cc: [email protected]
Signed-off-by: Christian Brauner <[email protected]>
Acked-by: Todd Kjos <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: prevent UAF for binderfs devices II

This is a necessary follow up to the first fix I proposed and we merged
in 2669b8b0c79 ("binder: prevent UAF for binderfs devices"). I have been
overly op

binder: prevent UAF for binderfs devices II

This is a necessary follow up to the first fix I proposed and we merged
in 2669b8b0c79 ("binder: prevent UAF for binderfs devices"). I have been
overly optimistic that the simple fix I proposed would work. But alas,
ihold() + iput() won't work since the inodes won't survive the
destruction of the superblock.
So all we get with my prior fix is a different race with a tinier
race-window but it doesn't solve the issue. Fwiw, the problem lies with
generic_shutdown_super(). It even has this cozy Al-style comment:

if (!list_empty(&sb->s_inodes)) {
printk("VFS: Busy inodes after unmount of %s. "
"Self-destruct in 5 seconds. Have a nice day...\n",
sb->s_id);
}

On binder_release(), binder_defer_work(proc, BINDER_DEFERRED_RELEASE) is
called which punts the actual cleanup operation to a workqueue. At some
point, binder_deferred_func() will be called which will end up calling
binder_deferred_release() which will retrieve and cleanup the
binder_context attach to this struct binder_proc.

If we trace back where this binder_context is attached to binder_proc we
see that it is set in binder_open() and is taken from the struct
binder_device it is associated with. This obviously assumes that the
struct binder_device that context is attached to is _never_ freed. While
that might be true for devtmpfs binder devices it is most certainly
wrong for binderfs binder devices.

So, assume binder_open() is called on a binderfs binder devices. We now
stash away the struct binder_context associated with that struct
binder_devices:
proc->context = &binder_dev->context;
/* binderfs stashes devices in i_private */
if (is_binderfs_device(nodp)) {
binder_dev = nodp->i_private;
info = nodp->i_sb->s_fs_info;
binder_binderfs_dir_entry_proc = info->proc_log_dir;
} else {
.
.
.
proc->context = &binder_dev->context;

Now let's assume that the binderfs instance for that binder devices is
shutdown via umount() and/or the mount namespace associated with it goes
away. As long as there is still an fd open for that binderfs binder
device things are fine. But let's assume we now close the last fd for
that binderfs binder device. Now binder_release() is called and punts to
the workqueue. Assume that the workqueue has quite a bit of stuff to do
and doesn't get to cleaning up the struct binder_proc and the associated
struct binder_context with it for that binderfs binder device right
away. In the meantime, the VFS is killing the super block and is
ultimately calling sb->evict_inode() which means it will call
binderfs_evict_inode() which does:

static void binderfs_evict_inode(struct inode *inode)
{
struct binder_device *device = inode->i_private;
struct binderfs_info *info = BINDERFS_I(inode);

clear_inode(inode);

if (!S_ISCHR(inode->i_mode) || !device)
return;

mutex_lock(&binderfs_minors_mutex);
--info->device_count;
ida_free(&binderfs_minors, device->miscdev.minor);
mutex_unlock(&binderfs_minors_mutex);

kfree(device->context.name);
kfree(device);
}

thereby freeing the struct binder_device including struct
binder_context.

Now the workqueue finally has time to get around to cleaning up struct
binder_proc and is now trying to access the associate struct
binder_context. Since it's already freed it will OOPs.

Fix this by introducing a refounct on binder devices.

This is an alternative fix to 51d8a7eca677 ("binder: prevent UAF read in
print_binder_transaction_log_entry()").

Fixes: 3ad20fe393b3 ("binder: implement binderfs")
Fixes: 2669b8b0c798 ("binder: prevent UAF for binderfs devices")
Fixes: 03e2e07e3814 ("binder: Make transaction_log available in binderfs")
Related : 51d8a7eca677 ("binder: prevent UAF read in print_binder_transaction_log_entry()")
Cc: [email protected]
Signed-off-by: Christian Brauner <[email protected]>
Acked-by: Todd Kjos <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...