linux-stable.git/drivers/android/binderfs.c, branch linux-5.10.y

binderfs: fix ida_alloc_max() upper bound

2026-02-11T12:34:21+00:00

commit ec4ddc90d201d09ef4e4bef8a2c6d9624525ad68 upstream.

The 'max' argument of ida_alloc_max() takes the maximum valid ID and not
the "count". Using an ID of BINDERFS_MAX_MINOR (1 << 20) for dev->minor
would exceed the limits of minor numbers (20-bits). Fix this off-by-one
error by subtracting 1 from the 'max'.

Cc: stable@vger.kernel.org
Fixes: 3ad20fe393b3 ("binder: implement binderfs")
Signed-off-by: Carlos Llamas 
Link: https://patch.msgid.link/20260127235545.2307876-2-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman

binderfs: make symbol 'binderfs_fs_parameters' static

2020-09-03T16:24:39+00:00

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 
Signed-off-by: Wei Yongjun 
Acked-by: Christian Brauner 
Link: https://lore.kernel.org/r/20200818112245.43891-1-weiyongjun1@huawei.com
Signed-off-by: Greg Kroah-Hartman

drivers: android: Fix the SPDX comment style

2020-07-29T15:05:44+00:00

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

Signed-off-by: Mrinal Pandey 
Link: https://lore.kernel.org/r/20200724131449.zvjutbemg3vqhrzh@mrinalpandey
Signed-off-by: Greg Kroah-Hartman

drivers: android: Fix a variable declaration coding style issue

2020-07-29T15:05:44+00:00

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

Signed-off-by: Mrinal Pandey 
Link: https://lore.kernel.org/r/20200724131433.stf3ycooogawyzb3@mrinalpandey
Signed-off-by: Greg Kroah-Hartman

binderfs: remove redundant assignment to pointer ctx

2020-04-23T14:48:11+00:00

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 
Acked-by: Christian Brauner 
Link: https://lore.kernel.org/r/20200402105000.506296-1-colin.king@canonical.com
Signed-off-by: Greg Kroah-Hartman

binderfs: Fix binderfs.c selftest compilation warning

2020-04-23T14:48:00+00:00

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 
Signed-off-by: Tang Bin 
Link: https://lore.kernel.org/r/20200411145151.5576-1-tangbin@cmss.chinamobile.com
Signed-off-by: Greg Kroah-Hartman

Merge 5.6-rc7 into char-misc-next

2020-03-23T06:59:38+00:00

We need the char/misc driver fixes in here as well.

Signed-off-by: Greg Kroah-Hartman

binderfs: port to new mount api

2020-03-19T06:41:01+00:00

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 
Signed-off-by: Christian Brauner 
Reviewed-by: Kees Cook 
Link: https://lore.kernel.org/r/20200313153427.141789-1-christian.brauner@ubuntu.com
Signed-off-by: Greg Kroah-Hartman

binderfs: use refcount for binder control devices too

2020-03-11T18:33:52+00:00

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 
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner 
Acked-by: Todd Kjos 
Link: https://lore.kernel.org/r/20200311105309.1742827-1-christian.brauner@ubuntu.com
Signed-off-by: Greg Kroah-Hartman

binder: prevent UAF for binderfs devices II

2020-03-03T18:58:37+00:00

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: stable@vger.kernel.org
Signed-off-by: Christian Brauner 
Acked-by: Todd Kjos 
Link: https://lore.kernel.org/r/20200303164340.670054-1-christian.brauner@ubuntu.com
Signed-off-by: Greg Kroah-Hartman