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

[Carlos Llamas <cmllamas@xxxxxxxxxx>]

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,
 	},
 };