Re: [PATCH v11 2/2] binder: report txn errors via generic netlink

Carlos Llamas
2025-01-08T21:59:54Z

On Wed, Jan 08, 2025 at 11:56:35AM -0800, Li Li wrote:
> This is a valid concern. Adding GENL_ADMIN_PERM should be enough to solve it.

Right! That seems to ask the genl stack to check for CAP_NET_ADMIN:

if ((op.flags & GENL_ADMIN_PERM) &&
!netlink_capable(skb, CAP_NET_ADMIN))
return -EPERM;

... which is a much better option and we could drop the portid check to
validate permissions. Something like the following (untested)?

diff --git a/Documentation/netlink/specs/binder.yaml b/Documentation/netlink/specs/binder.yaml
index 23f26c83a7c9..a0ef31cba666 100644
--- a/Documentation/netlink/specs/binder.yaml
+++ b/Documentation/netlink/specs/binder.yaml
@@ -81,6 +81,7 @@ operations:
name: report-setup
doc: Set flags from user space.
attribute-set: cmd
+ flags: [ admin-perm ]

do:
request: &params
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 536be42c531e..f6791f5f231a 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -6500,13 +6500,6 @@ int binder_nl_report_setup_doit(struct sk_buff *skb, struct genl_info *info)
pid = nla_get_u32(info->attrs[BINDER_A_CMD_PID]);
flags = nla_get_u32(info->attrs[BINDER_A_CMD_FLAGS]);

- if (context->report_portid && context->report_portid != portid) {
- NL_SET_ERR_MSG_FMT(info->extack,
- "No permission to set flags from %d",
- portid);
- return -EPERM;
- }
-
if (!pid) {
/* Set the global flags for the whole binder context */
context->report_flags = flags;
diff --git a/drivers/android/binder_netlink.c b/drivers/android/binder_netlink.c
index ea008f4f3635..6b3d93ff7c5d 100644
--- a/drivers/android/binder_netlink.c
+++ b/drivers/android/binder_netlink.c
@@ -24,7 +24,7 @@ static const struct genl_split_ops binder_nl_ops[] = {
.doit = binder_nl_report_setup_doit,
.policy = binder_report_setup_nl_policy,
.maxattr = BINDER_A_CMD_FLAGS,
- .flags = GENL_CMD_CAP_DO,
+ .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
},
};