mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] binder: restrict BINDER_NLGRP_REPORT to CAP_NET_ADMIN and fix binderfs minor leak
@ 2026-09-19 21:36 Hui Peng
  2026-09-20  5:01 ` Greg KH
  0 siblings, 1 reply; 2+ messages in thread
From: Hui Peng @ 2026-09-19 21:36 UTC (permalink / raw)
  To: gregkh, arve, tkjos, brauner, cmllamas, aliceryhl; +Cc: linux-kernel

Fix three security and resource-accounting issues in `binder_netlink.c`
and `binderfs.c`:

1. In `drivers/android/binder_netlink.c`,
   `binder_nl_mcgrps[BINDER_NLGRP_REPORT]` is declared without `.flags =
   GENL_MCAST_CAP_NET_ADMIN`, allowing any unprivileged local user to
   subscribe to the `BINDER_NLGRP_REPORT` Generic Netlink multicast
   group and monitor system-wide binder transaction error reports
   (including sender/target PIDs and transaction metadata).
2. In `drivers/android/binderfs.c`, `binderfs_binder_ctl_create()`
   allocates an IDA minor via `ida_alloc_max(&binderfs_minors, ...)`,
   and if `d_alloc_name(root, "binder-control")` fails, jumps to `out:`
   (which calls `kfree(device)`) without freeing `minor` from
   `binderfs_minors`.
3. When the `binder-control` inode (`device->context.name == NULL`) is
   evicted in `binderfs_evict_inode()`, `--info->device_count` is
   decremented even though `binderfs_binder_ctl_create()` never
   incremented `info->device_count`, underflowing `info->device_count`.
   Only decrement `info->device_count` when `device->context.name` is
   non-NULL.

Fixes: 63740349eba7 ("binder: introduce transaction reports via netlink")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>

---
 drivers/android/binder_netlink.c | 2 +-
 drivers/android/binderfs.c       | 9 +++++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/android/binder_netlink.c b/drivers/android/binder_netlink.c
index 81e8432b5904..3f1a4210aa52 100644
--- a/drivers/android/binder_netlink.c
+++ b/drivers/android/binder_netlink.c
@@ -16,7 +16,7 @@ static const struct genl_split_ops binder_nl_ops[] = {
 };
 
 static const struct genl_multicast_group binder_nl_mcgrps[] = {
-	[BINDER_NLGRP_REPORT] = { "report", },
+	[BINDER_NLGRP_REPORT] = { "report", .flags = GENL_MCAST_CAP_NET_ADMIN },
 };
 
 struct genl_family binder_nl_family __ro_after_init = {
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 361d69f756f5..661fda3ecb59 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -258,7 +258,8 @@ static void binderfs_evict_inode(struct inode *inode)
 		return;
 
 	mutex_lock(&binderfs_minors_mutex);
-	--info->device_count;
+	if (device->context.name)
+		--info->device_count;
 	ida_free(&binderfs_minors, device->miscdev.minor);
 	mutex_unlock(&binderfs_minors_mutex);
 
@@ -430,8 +431,12 @@ static int binderfs_binder_ctl_create(struct super_block *sb)
 	device->miscdev.minor = minor;
 
 	dentry = d_alloc_name(root, "binder-control");
-	if (!dentry)
+	if (!dentry) {
+		mutex_lock(&binderfs_minors_mutex);
+		ida_free(&binderfs_minors, minor);
+		mutex_unlock(&binderfs_minors_mutex);
 		goto out;
+	}
 
 	inode->i_private = device;
 	info->control_dentry = dentry;

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] binder: restrict BINDER_NLGRP_REPORT to CAP_NET_ADMIN and fix binderfs minor leak
  2026-09-19 21:36 [PATCH] binder: restrict BINDER_NLGRP_REPORT to CAP_NET_ADMIN and fix binderfs minor leak Hui Peng
@ 2026-09-20  5:01 ` Greg KH
  0 siblings, 0 replies; 2+ messages in thread
From: Greg KH @ 2026-09-20  5:01 UTC (permalink / raw)
  To: Hui Peng; +Cc: arve, tkjos, brauner, cmllamas, aliceryhl, linux-kernel

On Sat, Sep 19, 2026 at 09:36:54PM +0000, Hui Peng wrote:
> Fix three security and resource-accounting issues in `binder_netlink.c`
> and `binderfs.c`:
> 
> 1. In `drivers/android/binder_netlink.c`,
>    `binder_nl_mcgrps[BINDER_NLGRP_REPORT]` is declared without `.flags =
>    GENL_MCAST_CAP_NET_ADMIN`, allowing any unprivileged local user to
>    subscribe to the `BINDER_NLGRP_REPORT` Generic Netlink multicast
>    group and monitor system-wide binder transaction error reports
>    (including sender/target PIDs and transaction metadata).
> 2. In `drivers/android/binderfs.c`, `binderfs_binder_ctl_create()`
>    allocates an IDA minor via `ida_alloc_max(&binderfs_minors, ...)`,
>    and if `d_alloc_name(root, "binder-control")` fails, jumps to `out:`
>    (which calls `kfree(device)`) without freeing `minor` from
>    `binderfs_minors`.
> 3. When the `binder-control` inode (`device->context.name == NULL`) is
>    evicted in `binderfs_evict_inode()`, `--info->device_count` is
>    decremented even though `binderfs_binder_ctl_create()` never
>    incremented `info->device_count`, underflowing `info->device_count`.
>    Only decrement `info->device_count` when `device->context.name` is
>    non-NULL.

Why isn't this 3 different patches?

thanks,

greg kh

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-20  5:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-19 21:36 [PATCH] binder: restrict BINDER_NLGRP_REPORT to CAP_NET_ADMIN and fix binderfs minor leak Hui Peng
2026-09-20  5:01 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®