[PATCH net-next v10 0/2] binder: report txn errors via generic netlink

[PATCH net-next v10 0/2] binder: report txn errors via generic netlink

[Li Li]

From: Li Li <dualli@google.com>

It's a known issue that neither the frozen processes nor the system
administration process of the OS can correctly deal with failed binder
transactions. The reason is that there's no reliable way for the user
space administration process to fetch the binder errors from the kernel
binder driver.

Android is such an OS suffering from this issue. Since cgroup freezer
was used to freeze user applications to save battery, innocent frozen
apps have to be killed when they receive sync binder transactions or
when their async binder buffer is running out.

This patch introduces the Linux generic netlink messages into the binder
driver so that the Linux/Android system administration process can
listen to important events and take corresponding actions, like stopping
a broken app from attacking the OS by sending huge amount of spamming
binder transactiions.

The 1st version uses a global generic netlink for all binder contexts,
raising potential security concerns. There were a few other feedbacks
like request to kernel docs and test code. The thread can be found at
https://lore.kernel.org/lkml/20240812211844.4107494-1-dualli@chromium.org/

The 2nd version fixes those issues and has been tested on the latest
version of AOSP. See https://r.android.com/3305462 for how userspace is
going to use this feature and the test code. It can be found at
https://lore.kernel.org/lkml/20241011064427.1565287-1-dualli@chromium.org/

The 3rd version replaces the handcrafted netlink source code with the
netlink protocal specs in YAML. It also fixes the documentation issues.
https://lore.kernel.org/lkml/20241021182821.1259487-1-dualli@chromium.org/

The 4th version just containsi trivial fixes, making the subject of the
patch aligned with the subject of the cover letter.
https://lore.kernel.org/lkml/20241021191233.1334897-1-dualli@chromium.org/

The 5th version incorporates the suggested fixes to the kernel doc and
the init function. It also removes the unsupported uapi-header in YAML
that contains "/" for subdirectory.
https://lore.kernel.org/lkml/20241025075102.1785960-1-dualli@chromium.org/

The 6th version has some trivial kernel doc fixes, without modifying
any other source code.
https://lore.kernel.org/lkml/20241028101952.775731-1-dualli@chromium.org/

The 7th version breaks the binary struct netlink message into individual
attributes to better support automatic error checking. Thanks Jakub for
improving ynl-gen.
https://lore.kernel.org/all/20241031092504.840708-1-dualli@chromium.org/

The 8th version solves the multi-genl-family issue by demuxing the
messages based on a new context attribute. It also improves the YAML
spec to be consistent with netlink tradition. A Huge 'Thank You' to
Jakub who taught me a lot about the netlink protocol!
https://lore.kernel.org/all/20241113193239.2113577-1-dualli@chromium.org/

The 9th version only contains a few trivial fixes, removing a redundant
pr_err and unnecessary payload check. The ynl-gen patch to allow uapi
header in sub-dirs has been merged so it's no longer included in this
patch set.
https://lore.kernel.org/all/20241209192247.3371436-1-dualli@chromium.org/

The 10th version renames binder genl to binder netlink, improves the
readability of the kernel doc and uses more descriptive variable names.
The function binder_add_device() is moved out to a new commit per request.
It also fixes a warning about newline used in NL_SET_ERR_MSG.
Thanks Carlos for his valuable suggestions!

v1: add a global binder genl socket for all contexts
v2: change to per-context binder genl for security reason
    replace the new ioctl with a netlink command
    add corresponding doc Documentation/admin-guide/binder_genl.rst
    add user space test code in AOSP
v3: use YNL spec (./tools/net/ynl/ynl-regen.sh)
    fix documentation index
v4: change the subject of the patch and remove unsed #if 0
v5: improve the kernel doc and the init function
    remove unsupported uapi-header in YAML
v6: fix some trivial kernel doc issues
v7: break the binary struct binder_report into individual attributes
v8: use multiplex netlink message in a unified netlink family
    improve the YAML spec to be consistent with netlink tradition
v9: remove unnecessary check to netlink flags and message payloads.
v10: improve the readability of kernel doc and variable names.

Li Li (2):
  binderfs: add new binder devices to binder_devices
  binder: report txn errors via generic netlink

 Documentation/admin-guide/binder_genl.rst     | 110 ++++++++
 Documentation/admin-guide/index.rst           |   1 +
 .../netlink/specs/binder_netlink.yaml         | 108 ++++++++
 drivers/android/Kconfig                       |   1 +
 drivers/android/Makefile                      |   2 +-
 drivers/android/binder.c                      | 242 +++++++++++++++++-
 drivers/android/binder_internal.h             |  29 ++-
 drivers/android/binder_netlink.c              |  39 +++
 drivers/android/binder_netlink.h              |  19 ++
 drivers/android/binder_trace.h                |  35 +++
 drivers/android/binderfs.c                    |   2 +
 include/uapi/linux/android/binder_netlink.h   |  55 ++++
 12 files changed, 637 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/admin-guide/binder_genl.rst
 create mode 100644 Documentation/netlink/specs/binder_netlink.yaml
 create mode 100644 drivers/android/binder_netlink.c
 create mode 100644 drivers/android/binder_netlink.h
 create mode 100644 include/uapi/linux/android/binder_netlink.h


base-commit: 6145fefc1e42c1895c0c1c2c8593de2c085d8c56
-- 
2.47.1.613.gc27f4b7a9f-goog

[PATCH net-next v10 1/2] binderfs: add new binder devices to binder_devices

[Li Li]

From: Li Li <dualli@google.com>

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 <dualli@google.com>
---
 drivers/android/binder.c          | 5 +++++
 drivers/android/binder_internal.h | 8 ++++++++
 drivers/android/binderfs.c        | 2 ++
 3 files changed, 15 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index ef353ca13c35..0a16acd29653 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -6928,6 +6928,11 @@ const struct binder_debugfs_entry binder_debugfs_entries[] = {
 	{} /* terminator */
 };
 
+void binder_add_device(struct binder_device *device)
+{
+	hlist_add_head(&device->hlist, &binder_devices);
+}
+
 static int __init init_binder_device(const char *name)
 {
 	int ret;
diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
index f8d6be682f23..1f21ad3963b1 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -582,4 +582,12 @@ struct binder_object {
 	};
 };
 
+/**
+ * Add a binder device to binder_devices
+ * @device: the new binder device to add to the global list
+ *
+ * Not reentrant as the list is not protected by any locks
+ */
+void binder_add_device(struct binder_device *device);
+
 #endif /* _LINUX_BINDER_INTERNAL_H */
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index ad1fa7abc323..bc6bae76ccaf 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -207,6 +207,8 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
 	fsnotify_create(root->d_inode, dentry);
 	inode_unlock(d_inode(root));
 
+	binder_add_device(device);
+
 	return 0;
 
 err:
-- 
2.47.1.613.gc27f4b7a9f-goog

[PATCH net-next v10 2/2] binder: report txn errors via generic netlink

[Li Li]

From: Li Li <dualli@google.com>

Introduce generic netlink messages into the binder driver so that the
Linux/Android system administration process can listen to important
events and take corresponding actions, like stopping a broken app from
attacking the OS by sending huge amount of spamming binder transactions.

The binder netlink sources and headers are automatically generated from
the corresponding binder_netlink YAML spec. Don't modify them directly.

Signed-off-by: Li Li <dualli@google.com>
---
 Documentation/admin-guide/binder_genl.rst     | 110 ++++++++
 Documentation/admin-guide/index.rst           |   1 +
 .../netlink/specs/binder_netlink.yaml         | 108 ++++++++
 drivers/android/Kconfig                       |   1 +
 drivers/android/Makefile                      |   2 +-
 drivers/android/binder.c                      | 237 +++++++++++++++++-
 drivers/android/binder_internal.h             |  21 +-
 drivers/android/binder_netlink.c              |  39 +++
 drivers/android/binder_netlink.h              |  19 ++
 drivers/android/binder_trace.h                |  35 +++
 include/uapi/linux/android/binder_netlink.h   |  55 ++++
 11 files changed, 622 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/admin-guide/binder_genl.rst
 create mode 100644 Documentation/netlink/specs/binder_netlink.yaml
 create mode 100644 drivers/android/binder_netlink.c
 create mode 100644 drivers/android/binder_netlink.h
 create mode 100644 include/uapi/linux/android/binder_netlink.h

diff --git a/Documentation/admin-guide/binder_genl.rst b/Documentation/admin-guide/binder_genl.rst
new file mode 100644
index 000000000000..71b4c6596c5b
--- /dev/null
+++ b/Documentation/admin-guide/binder_genl.rst
@@ -0,0 +1,110 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==============================================================
+Generic Netlink for the Android Binder Driver (Binder Netlink)
+==============================================================
+
+The Generic Netlink subsystem in the Linux kernel provides a generic way for
+the Linux kernel to communicate with the user space applications via binder
+driver. It is used to report binder transaction errors and warnings to user
+space administration process. The driver allows multiple binder devices and
+their corresponding binder contexts. Each context has an independent Generic
+Netlink for security reason. To prevent untrusted user applications from
+accessing the netlink data, the kernel driver uses unicast mode instead of
+multicast.
+
+Basically, the user space code uses the BINDER_NETLINK_CMD_REPORT_SETUP
+command to request what kind of binder transactions should be reported by
+the driver. The driver then echoes the attributes in a reply message to
+acknowledge the request. The BINDER_NETLINK_CMD_REPORT_SETUP command also
+registers the current user space process to receive the reports. When the
+user space process exits, the previous request will be reset automatically.
+
+Currently the driver reports these binder transaction errors and warnings.
+1. "FAILED" transactions that fail to reach the target process;
+2. "ASYNC_FROZEN" transactions that are delayed due to the target process
+being frozen by cgroup freezer; or
+3. "SPAM" transactions that are considered spamming according to existing
+logic in binder_alloc.c.
+
+When the specified binder transactions happen, the driver uses the
+BINDER_NETLINK_CMD_REPORT command to send a generic netlink message to the
+registered process, containing the payload defined in binder_netlink.yaml.
+
+More details about the flags, attributes and operations can be found at the
+the doc sections in Documentations/netlink/specs/binder_netlink.yaml and the
+kernel-doc comments of the new source code in binder.{h|c}.
+
+Using Binder Netlink
+--------------------
+
+The Binder Netlink can be used in the same way as any other generic netlink
+drivers. Userspace application uses a raw netlink socket to send commands
+to and receive packets from the kernel driver.
+
+.. note::
+    If the userspace application that talks to the driver exits, the kernel
+    driver will automatically reset the configuration to the default and
+    stop sending more reports, which would otherwise fail.
+
+Usage example (user space pseudo code):
+
+::
+    /*
+     * send() below is overloaded to pack netlink commands and attributes
+     * to nlattr/genlmsghdr/nlmsghdr and then send to the netlink socket.
+     *
+     * recv() below is overloaded to receive the raw netlink message from
+     * the netlink socket, parse nlmsghdr/genlmsghdr to find the netlink
+     * command and then return the nlattr payload.
+     */
+
+    // open netlink socket
+    int fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_GENERIC);
+
+    // bind netlink socket
+    bind(fd, struct socketaddr);
+
+    // get the family id of the binder netlink
+    send(fd, CTRL_CMD_GETFAMILY, CTRL_ATTR_FAMILY_NAME,
+            BINDER_NETLINK_FAMILY_NAME);
+    void *data = recv(CTRL_CMD_NEWFAMILY);
+    if (!has_nla_type(data, CTRL_ATTR_FAMILY_ID)) {
+        // Binder Netlink isn't available on this version of Linux kernel
+        return;
+    }
+    __u16 id = nla(data)[CTRL_ATTR_FAMILY_ID];
+
+    // enable per-context binder report
+    send(fd, id, BINDER_NETLINK_CMD_REPORT_SETUP, "binder", 0,
+            BINDER_NETLINK_FLAG_FAILED | BINDER_NETLINK_FLAG_DELAYED);
+
+    // confirm the per-context configuration
+    data = recv(fd, BINDER_NETLINK_CMD_REPLY);
+    char *context = nla(data)[BINDER_NETLINK_A_CMD_CONTEXT];
+    __u32 pid =  nla(data)[BINDER_NETLINK_A_CMD_PID];
+    __u32 flags = nla(data)[BINDER_NETLINK_A_CMD_FLAGS];
+
+    // set optional per-process report, overriding the per-context one
+    send(fd, id, BINDER_NETLINK_CMD_REPORT_SETUP, "binder", getpid(),
+            BINDER_NETLINK_FLAG_SPAM | BINDER_REPORT_OVERRIDE);
+
+    // confirm the optional per-process configuration
+    data = recv(fd, BINDER_NETLINK_CMD_REPLY);
+    context = nla(data)[BINDER_NETLINK_A_CMD_CONTEXT];
+    pid =  nla(data)[BINDER_NETLINK_A_CMD_PID];
+    flags = nla(data)[BINDER_NETLINK_A_CMD_FLAGS];
+
+    // wait and read all binder reports
+    while (running) {
+            data = recv(fd, BINDER_NETLINK_CMD_REPORT);
+            auto *attr = nla(data)[BINDER_NETLINK_A_REPORT_XXX];
+
+            // process binder report
+            do_something(*attr);
+    }
+
+    // clean up
+    send(fd, id, BINDER_NETLINK_CMD_REPORT_SETUP, 0, 0);
+    send(fd, id, BINDER_NETLINK_CMD_REPORT_SETUP, getpid(), 0);
+    close(fd);
diff --git a/Documentation/admin-guide/index.rst b/Documentation/admin-guide/index.rst
index e85b1adf5908..b3b5cfadffe5 100644
--- a/Documentation/admin-guide/index.rst
+++ b/Documentation/admin-guide/index.rst
@@ -79,6 +79,7 @@ configure specific aspects of kernel behavior to your liking.
    aoe/index
    auxdisplay/index
    bcache
+   binder_genl
    binderfs
    binfmt-misc
    blockdev/index
diff --git a/Documentation/netlink/specs/binder_netlink.yaml b/Documentation/netlink/specs/binder_netlink.yaml
new file mode 100644
index 000000000000..7eef013e6f07
--- /dev/null
+++ b/Documentation/netlink/specs/binder_netlink.yaml
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+
+name: binder_netlink
+protocol: genetlink
+uapi-header: linux/android/binder_netlink.h
+doc: Netlink protocol to report binder transaction errors and warnings.
+
+definitions:
+  -
+    type: flags
+    name: flag
+    doc: Define what kind of binder transactions should be reported.
+    entries: [ failed, async-frozen, spam, override ]
+
+attribute-sets:
+  -
+    name: cmd
+    doc: The supported attributes of "report-setup" command.
+    attributes:
+      -
+        name: context
+        type: string
+        doc: The binder context to enable binder netlink report.
+      -
+        name: pid
+        type: u32
+        doc: The binder proc to enable binder netlink report.
+      -
+        name: flags
+        type: u32
+        enum: flag
+        doc: What kind of binder transactions should be reported.
+  -
+    name: report
+    doc: The supported attributes of "report" command
+    attributes:
+      -
+        name: context
+        type: string
+        doc: The binder context where the binder netlink report happens.
+      -
+        name: err
+        type: u32
+        doc: Copy of binder_driver_return_protocol returned to the sender.
+      -
+        name: from_pid
+        type: u32
+        doc: Sender pid of the corresponding binder transaction.
+      -
+        name: from_tid
+        type: u32
+        doc: Sender tid of the corresponding binder transaction.
+      -
+        name: to_pid
+        type: u32
+        doc: Target pid of the corresponding binder transaction.
+      -
+        name: to_tid
+        type: u32
+        doc: Target tid of the corresponding binder transaction.
+      -
+        name: reply
+        type: u32
+        doc: 1 means the transaction is a reply, 0 otherwise.
+      -
+        name: flags
+        type: u32
+        doc: Copy of binder_transaction_data->flags.
+      -
+        name: code
+        type: u32
+        doc: Copy of binder_transaction_data->code.
+      -
+        name: data_size
+        type: u32
+        doc: Copy of binder_transaction_data->data_size.
+
+operations:
+  list:
+    -
+      name: report-setup
+      doc: Set flags from user space.
+      attribute-set: cmd
+
+      do:
+        request: &params
+          attributes:
+            - context
+            - pid
+            - flags
+        reply: *params
+    -
+      name: report
+      doc: Send the requested reports to user space.
+      attribute-set: report
+
+      event:
+        attributes:
+          - context
+          - err
+          - from_pid
+          - from_tid
+          - to_pid
+          - to_tid
+          - reply
+          - flags
+          - code
+          - data_size
diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
index 07aa8ae0a058..e2fa620934e2 100644
--- a/drivers/android/Kconfig
+++ b/drivers/android/Kconfig
@@ -4,6 +4,7 @@ menu "Android"
 config ANDROID_BINDER_IPC
 	bool "Android Binder IPC Driver"
 	depends on MMU
+	depends on NET
 	default n
 	help
 	  Binder is used in Android for both communication between processes,
diff --git a/drivers/android/Makefile b/drivers/android/Makefile
index c9d3d0c99c25..b8874dba884e 100644
--- a/drivers/android/Makefile
+++ b/drivers/android/Makefile
@@ -2,5 +2,5 @@
 ccflags-y += -I$(src)			# needed for trace events
 
 obj-$(CONFIG_ANDROID_BINDERFS)		+= binderfs.o
-obj-$(CONFIG_ANDROID_BINDER_IPC)	+= binder.o binder_alloc.o
+obj-$(CONFIG_ANDROID_BINDER_IPC)	+= binder.o binder_alloc.o binder_netlink.o
 obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 0a16acd29653..44932659051c 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -72,6 +72,7 @@
 
 #include <linux/cacheflush.h>
 
+#include "binder_netlink.h"
 #include "binder_internal.h"
 #include "binder_trace.h"
 
@@ -2990,6 +2991,111 @@ static void binder_set_txn_from_error(struct binder_transaction *t, int id,
 	binder_thread_dec_tmpref(from);
 }
 
+/**
+ * binder_find_proc() - set binder report flags
+ * @pid:	the target process
+ */
+static struct binder_proc *binder_find_proc(int pid)
+{
+	struct binder_proc *proc;
+
+	mutex_lock(&binder_procs_lock);
+	hlist_for_each_entry(proc, &binder_procs, proc_node) {
+		if (proc->pid == pid) {
+			mutex_unlock(&binder_procs_lock);
+			return proc;
+		}
+	}
+	mutex_unlock(&binder_procs_lock);
+
+	return NULL;
+}
+
+/**
+ * binder_netlink_enabled() - check if binder netlink reports are enabled
+ * @proc:	the binder_proc to check
+ * @mask:	the categories of binder netlink reports
+ *
+ * Returns true if certain binder netlink reports are enabled for this binder
+ * proc (when per-process overriding takes effect) or context.
+ */
+static bool binder_netlink_enabled(struct binder_proc *proc, u32 mask)
+{
+	struct binder_context *context = proc->context;
+
+	if (!context->report_portid)
+		return false;
+
+	if (proc->report_flags & BINDER_NETLINK_FLAG_OVERRIDE)
+		return (proc->report_flags & mask) != 0;
+	else
+		return (context->report_flags & mask) != 0;
+}
+
+/**
+ * binder_netlink_report() - report one binder netlink event
+ * @context:	the binder context
+ * @err:	copy of binder_driver_return_protocol returned to the sender
+ * @pid:	sender process
+ * @tid:	sender thread
+ * @to_pid:	target process
+ * @to_tid:	target thread
+ * @reply:	whether the binder transaction is a reply
+ * @tr:		the binder transaction data
+ *
+ * Packs the report data into a binder netlink message and send it.
+ */
+static void binder_netlink_report(struct binder_context *context, u32 err,
+				  u32 pid, u32 tid, u32 to_pid, u32 to_tid,
+				  u32 reply,
+				  struct binder_transaction_data *tr)
+{
+	int ret;
+	struct sk_buff *skb;
+	void *hdr;
+
+	trace_binder_netlink_report(context->name, err, pid, tid, to_pid,
+				    to_tid, reply, tr);
+
+	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!skb) {
+		pr_err("Failed to alloc binder netlink message\n");
+		return;
+	}
+
+	hdr = genlmsg_put(skb, 0, atomic_inc_return(&context->report_seq),
+			  &binder_netlink_nl_family, 0, BINDER_NETLINK_CMD_REPORT);
+	if (!hdr)
+		goto free_skb;
+
+	if (nla_put_string(skb, BINDER_NETLINK_A_REPORT_CONTEXT, context->name) ||
+	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_ERR, err) ||
+	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_FROM_PID, pid) ||
+	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_FROM_TID, tid) ||
+	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_TO_PID, to_pid) ||
+	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_TO_TID, to_tid) ||
+	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_REPLY, reply) ||
+	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_FLAGS, tr->flags) ||
+	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_CODE, tr->code) ||
+	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_DATA_SIZE, tr->data_size))
+		goto cancel_skb;
+
+	genlmsg_end(skb, hdr);
+
+	ret = genlmsg_unicast(&init_net, skb, context->report_portid);
+	if (ret < 0)
+		pr_err("Failed to send binder netlink message to %d: %d\n",
+		       context->report_portid, ret);
+	return;
+
+cancel_skb:
+	pr_err("Failed to add attributes to binder netlink message\n");
+	genlmsg_cancel(skb, hdr);
+free_skb:
+	pr_err("Free binder netlink report message on error\n");
+	nlmsg_free(skb);
+}
+
 static void binder_transaction(struct binder_proc *proc,
 			       struct binder_thread *thread,
 			       struct binder_transaction_data *tr, int reply,
@@ -3684,10 +3790,17 @@ static void binder_transaction(struct binder_proc *proc,
 		return_error_line = __LINE__;
 		goto err_copy_data_failed;
 	}
-	if (t->buffer->oneway_spam_suspect)
+	if (t->buffer->oneway_spam_suspect) {
 		tcomplete->type = BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT;
-	else
+		if (binder_netlink_enabled(proc, BINDER_NETLINK_FLAG_SPAM))
+			binder_netlink_report(context, BR_ONEWAY_SPAM_SUSPECT,
+					      proc->pid, thread->pid,
+					      target_proc ? target_proc->pid : 0,
+					      target_thread ? target_thread->pid : 0,
+					      reply, tr);
+	} else {
 		tcomplete->type = BINDER_WORK_TRANSACTION_COMPLETE;
+	}
 	t->work.type = BINDER_WORK_TRANSACTION;
 
 	if (reply) {
@@ -3737,8 +3850,15 @@ static void binder_transaction(struct binder_proc *proc,
 		 * process and is put in a pending queue, waiting for the target
 		 * process to be unfrozen.
 		 */
-		if (return_error == BR_TRANSACTION_PENDING_FROZEN)
+		if (return_error == BR_TRANSACTION_PENDING_FROZEN) {
 			tcomplete->type = BINDER_WORK_TRANSACTION_PENDING;
+			if (binder_netlink_enabled(proc, BINDER_NETLINK_FLAG_ASYNC_FROZEN))
+				binder_netlink_report(context, return_error,
+						      proc->pid, thread->pid,
+						      target_proc ? target_proc->pid : 0,
+						      target_thread ? target_thread->pid : 0,
+						      reply, tr);
+		}
 		binder_enqueue_thread_work(thread, tcomplete);
 		if (return_error &&
 		    return_error != BR_TRANSACTION_PENDING_FROZEN)
@@ -3800,6 +3920,13 @@ static void binder_transaction(struct binder_proc *proc,
 		binder_dec_node_tmpref(target_node);
 	}
 
+	if (binder_netlink_enabled(proc, BINDER_NETLINK_FLAG_FAILED))
+		binder_netlink_report(context, return_error,
+				      proc->pid, thread->pid,
+				      target_proc ? target_proc->pid : 0,
+				      target_thread ? target_thread->pid : 0,
+				      reply, tr);
+
 	binder_debug(BINDER_DEBUG_FAILED_TRANSACTION,
 		     "%d:%d transaction %s to %d:%d failed %d/%d/%d, size %lld-%lld line %d\n",
 		     proc->pid, thread->pid, reply ? "reply" :
@@ -6137,6 +6264,11 @@ static int binder_release(struct inode *nodp, struct file *filp)
 
 	binder_defer_work(proc, BINDER_DEFERRED_RELEASE);
 
+	if (proc->pid == proc->context->report_portid) {
+		proc->context->report_portid = 0;
+		proc->context->report_flags = 0;
+	}
+
 	return 0;
 }
 
@@ -6335,6 +6467,99 @@ binder_defer_work(struct binder_proc *proc, enum binder_deferred_state defer)
 	mutex_unlock(&binder_deferred_lock);
 }
 
+/**
+ * binder_netlink_nl_report_setup_doit() - netlink .doit handler
+ * @skb:	the metadata struct passed from netlink driver
+ * @info:	the generic netlink struct passed from netlink driver
+ *
+ * Implements the .doit function to process binder netlink commands.
+ */
+int binder_netlink_nl_report_setup_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	int portid;
+	u32 pid;
+	u32 flags;
+	void *hdr;
+	struct binder_proc *proc;
+	struct binder_device *device;
+	struct binder_context *context = NULL;
+
+	hlist_for_each_entry(device, &binder_devices, hlist) {
+		if (!nla_strcmp(info->attrs[BINDER_NETLINK_A_CMD_CONTEXT],
+				device->context.name)) {
+			context = &device->context;
+			break;
+		}
+	}
+
+	if (!context) {
+		NL_SET_ERR_MSG(info->extack, "Unknown binder context");
+		return -EINVAL;
+	}
+
+	portid = nlmsg_hdr(skb)->nlmsg_pid;
+	pid = nla_get_u32(info->attrs[BINDER_NETLINK_A_CMD_PID]);
+	flags = nla_get_u32(info->attrs[BINDER_NETLINK_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;
+	} else {
+		/* Set the per-process flags */
+		proc = binder_find_proc(pid);
+		if (!proc) {
+			NL_SET_ERR_MSG_FMT(info->extack,
+					   "Invalid binder report pid %u",
+					   pid);
+			return -EINVAL;
+		}
+
+		proc->report_flags = flags;
+	}
+
+	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!skb) {
+		pr_err("Failed to alloc binder netlink reply message\n");
+		return -ENOMEM;
+	}
+
+	hdr = genlmsg_iput(skb, info);
+	if (!hdr)
+		goto free_skb;
+
+	if (nla_put_string(skb, BINDER_NETLINK_A_CMD_CONTEXT, context->name) ||
+	    nla_put_u32(skb, BINDER_NETLINK_A_CMD_PID, pid) ||
+	    nla_put_u32(skb, BINDER_NETLINK_A_CMD_FLAGS, flags))
+		goto cancel_skb;
+
+	genlmsg_end(skb, hdr);
+
+	if (genlmsg_reply(skb, info)) {
+		pr_err("Failed to send binder netlink reply message\n");
+		return -EFAULT;
+	}
+
+	if (!context->report_portid)
+		context->report_portid = portid;
+
+	return 0;
+
+cancel_skb:
+	pr_err("Failed to add reply attributes to binder netlink message\n");
+	genlmsg_cancel(skb, hdr);
+free_skb:
+	pr_err("Free binder netlink reply message on error\n");
+	nlmsg_free(skb);
+	return -EMSGSIZE;
+}
+
 static void print_binder_transaction_ilocked(struct seq_file *m,
 					     struct binder_proc *proc,
 					     const char *prefix,
@@ -7014,6 +7239,12 @@ static int __init binder_init(void)
 	if (ret)
 		goto err_init_binder_device_failed;
 
+	ret = genl_register_family(&binder_netlink_nl_family);
+	if (ret) {
+		pr_err("Failed to register binder netlink family\n");
+		goto err_init_binder_device_failed;
+	}
+
 	return ret;
 
 err_init_binder_device_failed:
diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
index 1f21ad3963b1..c67eba88f89a 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -12,21 +12,35 @@
 #include <linux/stddef.h>
 #include <linux/types.h>
 #include <linux/uidgid.h>
+#include <net/genetlink.h>
 #include <uapi/linux/android/binderfs.h>
 #include "binder_alloc.h"
 #include "dbitmap.h"
 
+/**
+ * struct binder_context - information about a binder domain
+ * @binder_context_mgr_node: the context manager
+ * @context_mgr_node_lock:   the lock protecting the above context manager node
+ * @binder_context_mgr_uid:  the uid of the above context manager
+ * @name:                    the name of the binder device
+ * @report_portid:           the netlink socket to receive binder reports
+ * @report_flags:            the categories of binder transactions that would
+ *                           be reported (see enum binder_report_flag).
+ * @report_seq:              the seq number of the generic netlink report
+ */
 struct binder_context {
 	struct binder_node *binder_context_mgr_node;
 	struct mutex context_mgr_node_lock;
 	kuid_t binder_context_mgr_uid;
 	const char *name;
+	u32 report_portid;
+	u32 report_flags;
+	atomic_t report_seq;
 };
 
 /**
  * struct binder_device - information about a binder device node
- * @hlist:          list of binder devices (only used for devices requested via
- *                  CONFIG_ANDROID_BINDER_DEVICES)
+ * @hlist:          list of binder devices
  * @miscdev:        information about a binder character device node
  * @context:        binder context information
  * @binderfs_inode: This is the inode of the root dentry of the super block
@@ -415,6 +429,8 @@ struct binder_ref {
  * @binderfs_entry:       process-specific binderfs log file
  * @oneway_spam_detection_enabled: process enabled oneway spam detection
  *                        or not
+ * @report_flags:         the categories of binder transactions that would
+ *                        be reported (see enum binder_genl_flag).
  *
  * Bookkeeping structure for binder processes
  */
@@ -453,6 +469,7 @@ struct binder_proc {
 	spinlock_t outer_lock;
 	struct dentry *binderfs_entry;
 	bool oneway_spam_detection_enabled;
+	u32 report_flags;
 };
 
 /**
diff --git a/drivers/android/binder_netlink.c b/drivers/android/binder_netlink.c
new file mode 100644
index 000000000000..2081b4319268
--- /dev/null
+++ b/drivers/android/binder_netlink.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/binder_netlink.yaml */
+/* YNL-GEN kernel source */
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include "binder_netlink.h"
+
+#include <uapi/linux/android/binder_netlink.h>
+
+/* BINDER_NETLINK_CMD_REPORT_SETUP - do */
+static const struct nla_policy binder_netlink_report_setup_nl_policy[BINDER_NETLINK_A_CMD_FLAGS + 1] = {
+	[BINDER_NETLINK_A_CMD_CONTEXT] = { .type = NLA_NUL_STRING, },
+	[BINDER_NETLINK_A_CMD_PID] = { .type = NLA_U32, },
+	[BINDER_NETLINK_A_CMD_FLAGS] = NLA_POLICY_MASK(NLA_U32, 0xf),
+};
+
+/* Ops table for binder_netlink */
+static const struct genl_split_ops binder_netlink_nl_ops[] = {
+	{
+		.cmd		= BINDER_NETLINK_CMD_REPORT_SETUP,
+		.doit		= binder_netlink_nl_report_setup_doit,
+		.policy		= binder_netlink_report_setup_nl_policy,
+		.maxattr	= BINDER_NETLINK_A_CMD_FLAGS,
+		.flags		= GENL_CMD_CAP_DO,
+	},
+};
+
+struct genl_family binder_netlink_nl_family __ro_after_init = {
+	.name		= BINDER_NETLINK_FAMILY_NAME,
+	.version	= BINDER_NETLINK_FAMILY_VERSION,
+	.netnsok	= true,
+	.parallel_ops	= true,
+	.module		= THIS_MODULE,
+	.split_ops	= binder_netlink_nl_ops,
+	.n_split_ops	= ARRAY_SIZE(binder_netlink_nl_ops),
+};
diff --git a/drivers/android/binder_netlink.h b/drivers/android/binder_netlink.h
new file mode 100644
index 000000000000..022cff5f7c38
--- /dev/null
+++ b/drivers/android/binder_netlink.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/binder_netlink.yaml */
+/* YNL-GEN kernel header */
+
+#ifndef _LINUX_BINDER_NETLINK_GEN_H
+#define _LINUX_BINDER_NETLINK_GEN_H
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include <uapi/linux/android/binder_netlink.h>
+
+int binder_netlink_nl_report_setup_doit(struct sk_buff *skb,
+					struct genl_info *info);
+
+extern struct genl_family binder_netlink_nl_family;
+
+#endif /* _LINUX_BINDER_NETLINK_GEN_H */
diff --git a/drivers/android/binder_trace.h b/drivers/android/binder_trace.h
index fe38c6fc65d0..8976fb5ee2db 100644
--- a/drivers/android/binder_trace.h
+++ b/drivers/android/binder_trace.h
@@ -423,6 +423,41 @@ TRACE_EVENT(binder_return,
 			  "unknown")
 );
 
+TRACE_EVENT(binder_netlink_report,
+	TP_PROTO(const char *name, u32 err, u32 pid, u32 tid, u32 to_pid,
+		 u32 to_tid, u32 reply, struct binder_transaction_data *tr),
+	TP_ARGS(name, err, pid, tid, to_pid, to_tid, reply, tr),
+	TP_STRUCT__entry(
+		__field(const char *, name)
+		__field(u32, err)
+		__field(u32, pid)
+		__field(u32, tid)
+		__field(u32, to_pid)
+		__field(u32, to_tid)
+		__field(u32, reply)
+		__field(u32, flags)
+		__field(u32, code)
+		__field(binder_size_t, data_size)
+	),
+	TP_fast_assign(
+		__entry->name = name;
+		__entry->err = err;
+		__entry->pid = pid;
+		__entry->tid = tid;
+		__entry->to_pid = to_pid;
+		__entry->to_tid = to_tid;
+		__entry->reply = reply;
+		__entry->flags = tr->flags;
+		__entry->code = tr->code;
+		__entry->data_size = tr->data_size;
+	),
+	TP_printk("%s: %d %d:%d -> %d:%d %s flags=0x08%x code=%d %llu",
+		  __entry->name, __entry->err, __entry->pid, __entry->tid,
+		  __entry->to_pid, __entry->to_tid,
+		  __entry->reply ? "reply" : "",
+		  __entry->flags, __entry->code, __entry->data_size)
+);
+
 #endif /* _BINDER_TRACE_H */
 
 #undef TRACE_INCLUDE_PATH
diff --git a/include/uapi/linux/android/binder_netlink.h b/include/uapi/linux/android/binder_netlink.h
new file mode 100644
index 000000000000..2b1460387597
--- /dev/null
+++ b/include/uapi/linux/android/binder_netlink.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/binder_netlink.yaml */
+/* YNL-GEN uapi header */
+
+#ifndef _UAPI_LINUX_ANDROID_BINDER_NETLINK_H
+#define _UAPI_LINUX_ANDROID_BINDER_NETLINK_H
+
+#define BINDER_NETLINK_FAMILY_NAME	"binder_netlink"
+#define BINDER_NETLINK_FAMILY_VERSION	1
+
+/*
+ * Define what kind of binder transactions should be reported.
+ */
+enum binder_netlink_flag {
+	BINDER_NETLINK_FLAG_FAILED = 1,
+	BINDER_NETLINK_FLAG_ASYNC_FROZEN = 2,
+	BINDER_NETLINK_FLAG_SPAM = 4,
+	BINDER_NETLINK_FLAG_OVERRIDE = 8,
+};
+
+enum {
+	BINDER_NETLINK_A_CMD_CONTEXT = 1,
+	BINDER_NETLINK_A_CMD_PID,
+	BINDER_NETLINK_A_CMD_FLAGS,
+
+	__BINDER_NETLINK_A_CMD_MAX,
+	BINDER_NETLINK_A_CMD_MAX = (__BINDER_NETLINK_A_CMD_MAX - 1)
+};
+
+enum {
+	BINDER_NETLINK_A_REPORT_CONTEXT = 1,
+	BINDER_NETLINK_A_REPORT_ERR,
+	BINDER_NETLINK_A_REPORT_FROM_PID,
+	BINDER_NETLINK_A_REPORT_FROM_TID,
+	BINDER_NETLINK_A_REPORT_TO_PID,
+	BINDER_NETLINK_A_REPORT_TO_TID,
+	BINDER_NETLINK_A_REPORT_REPLY,
+	BINDER_NETLINK_A_REPORT_FLAGS,
+	BINDER_NETLINK_A_REPORT_CODE,
+	BINDER_NETLINK_A_REPORT_DATA_SIZE,
+
+	__BINDER_NETLINK_A_REPORT_MAX,
+	BINDER_NETLINK_A_REPORT_MAX = (__BINDER_NETLINK_A_REPORT_MAX - 1)
+};
+
+enum {
+	BINDER_NETLINK_CMD_REPORT_SETUP = 1,
+	BINDER_NETLINK_CMD_REPORT,
+
+	__BINDER_NETLINK_CMD_MAX,
+	BINDER_NETLINK_CMD_MAX = (__BINDER_NETLINK_CMD_MAX - 1)
+};
+
+#endif /* _UAPI_LINUX_ANDROID_BINDER_NETLINK_H */
-- 
2.47.1.613.gc27f4b7a9f-goog

Re: [PATCH net-next v10 1/2] binderfs: add new binder devices to binder_devices

[Jakub Kicinski]

On Thu, 12 Dec 2024 14:41:13 -0800 Li Li wrote:
> +/**
> + * Add a binder device to binder_devices

nit: kdoc is missing function name

> + * @device: the new binder device to add to the global list
> + *
> + * Not reentrant as the list is not protected by any locks
> + */
> +void binder_add_device(struct binder_device *device);

To be clear we do not intend to apply these patches to net-next,
looks like binder patches are mostly handled by Greg KH. Please
drop the net-next from the subject on future revisions to avoid
confusion.

Re: [PATCH net-next v10 1/2] binderfs: add new binder devices to binder_devices

[Li Li]

On Sun, Dec 15, 2024 at 2:27 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 12 Dec 2024 14:41:13 -0800 Li Li wrote:
> > +/**
> > + * Add a binder device to binder_devices
>
> nit: kdoc is missing function name
>
> > + * @device: the new binder device to add to the global list
> > + *
> > + * Not reentrant as the list is not protected by any locks
> > + */
> > +void binder_add_device(struct binder_device *device);
>
> To be clear we do not intend to apply these patches to net-next,
> looks like binder patches are mostly handled by Greg KH. Please
> drop the net-next from the subject on future revisions to avoid
> confusion.


Got it. I'll modify the subject accordingly.

Meanwhile, Greg KH did say we need netlink experts to review
the netlink code. Please let me know if you have any more
comments about the netlink part of this patch so that I can
fix them in the next revision. Thank you!

Re: [PATCH net-next v10 1/2] binderfs: add new binder devices to binder_devices

[Carlos Llamas]

On Thu, Dec 12, 2024 at 02:41:13PM -0800, Li Li wrote:
> From: Li Li <dualli@google.com>
> 
> 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 <dualli@google.com>
> ---
>  drivers/android/binder.c          | 5 +++++
>  drivers/android/binder_internal.h | 8 ++++++++
>  drivers/android/binderfs.c        | 2 ++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index ef353ca13c35..0a16acd29653 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -6928,6 +6928,11 @@ const struct binder_debugfs_entry binder_debugfs_entries[] = {
>  	{} /* terminator */
>  };
>  
> +void binder_add_device(struct binder_device *device)
> +{
> +	hlist_add_head(&device->hlist, &binder_devices);
> +}
> +
>  static int __init init_binder_device(const char *name)
>  {
>  	int ret;
> diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h

nit: I believe you the following hunk should be in this patch no?

 /**
  * struct binder_device - information about a binder device node
- * @hlist:          list of binder devices (only used for devices requested via
- *                  CONFIG_ANDROID_BINDER_DEVICES)
+ * @hlist:          list of binder devices
  * @miscdev:        information about a binder character device node
  * @context:        binder context information
  * @binderfs_inode: This is the inode of the root dentry of the super block


> index f8d6be682f23..1f21ad3963b1 100644
> --- a/drivers/android/binder_internal.h
> +++ b/drivers/android/binder_internal.h
> @@ -582,4 +582,12 @@ struct binder_object {
>  	};
>  };
>  
> +/**
> + * Add a binder device to binder_devices
> + * @device: the new binder device to add to the global list
> + *
> + * Not reentrant as the list is not protected by any locks
> + */
> +void binder_add_device(struct binder_device *device);
> +
>  #endif /* _LINUX_BINDER_INTERNAL_H */
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index ad1fa7abc323..bc6bae76ccaf 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -207,6 +207,8 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
>  	fsnotify_create(root->d_inode, dentry);
>  	inode_unlock(d_inode(root));
>  
> +	binder_add_device(device);
> +
>  	return 0;
>  
>  err:
> -- 
> 2.47.1.613.gc27f4b7a9f-goog
> 


Other than the comment above it LGTM. Feel free to add:

Acked-by: Carlos Llamas <cmllamas@google.com>

Re: [PATCH net-next v10 2/2] binder: report txn errors via generic netlink

[Carlos Llamas]

On Thu, Dec 12, 2024 at 02:41:14PM -0800, Li Li wrote:
> From: Li Li <dualli@google.com>
> 
> Introduce generic netlink messages into the binder driver so that the
> Linux/Android system administration process can listen to important
> events and take corresponding actions, like stopping a broken app from
> attacking the OS by sending huge amount of spamming binder transactions.
> 
> The binder netlink sources and headers are automatically generated from
> the corresponding binder_netlink YAML spec. Don't modify them directly.
> 
> Signed-off-by: Li Li <dualli@google.com>
> ---
>  Documentation/admin-guide/binder_genl.rst     | 110 ++++++++

Thanks for renaming to "Binder Netlink" this seems much better IMO.
Also, I belive the documentation should also be binder_netlink.rst in
such case?

>  Documentation/admin-guide/index.rst           |   1 +
>  .../netlink/specs/binder_netlink.yaml         | 108 ++++++++
>  drivers/android/Kconfig                       |   1 +
>  drivers/android/Makefile                      |   2 +-
>  drivers/android/binder.c                      | 237 +++++++++++++++++-
>  drivers/android/binder_internal.h             |  21 +-
>  drivers/android/binder_netlink.c              |  39 +++
>  drivers/android/binder_netlink.h              |  19 ++
>  drivers/android/binder_trace.h                |  35 +++
>  include/uapi/linux/android/binder_netlink.h   |  55 ++++
>  11 files changed, 622 insertions(+), 6 deletions(-)
>  create mode 100644 Documentation/admin-guide/binder_genl.rst
>  create mode 100644 Documentation/netlink/specs/binder_netlink.yaml
>  create mode 100644 drivers/android/binder_netlink.c
>  create mode 100644 drivers/android/binder_netlink.h
>  create mode 100644 include/uapi/linux/android/binder_netlink.h
> 
> diff --git a/Documentation/admin-guide/binder_genl.rst b/Documentation/admin-guide/binder_genl.rst
> new file mode 100644
> index 000000000000..71b4c6596c5b
> --- /dev/null
> +++ b/Documentation/admin-guide/binder_genl.rst
> @@ -0,0 +1,110 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +==============================================================
> +Generic Netlink for the Android Binder Driver (Binder Netlink)
> +==============================================================
> +
> +The Generic Netlink subsystem in the Linux kernel provides a generic way for
> +the Linux kernel to communicate with the user space applications via binder
> +driver. It is used to report binder transaction errors and warnings to user
> +space administration process. The driver allows multiple binder devices and
> +their corresponding binder contexts. Each context has an independent Generic
> +Netlink for security reason. To prevent untrusted user applications from
> +accessing the netlink data, the kernel driver uses unicast mode instead of
> +multicast.
> +
> +Basically, the user space code uses the BINDER_NETLINK_CMD_REPORT_SETUP
> +command to request what kind of binder transactions should be reported by
> +the driver. The driver then echoes the attributes in a reply message to
> +acknowledge the request. The BINDER_NETLINK_CMD_REPORT_SETUP command also
> +registers the current user space process to receive the reports. When the
> +user space process exits, the previous request will be reset automatically.
> +
> +Currently the driver reports these binder transaction errors and warnings.
> +1. "FAILED" transactions that fail to reach the target process;
> +2. "ASYNC_FROZEN" transactions that are delayed due to the target process
> +being frozen by cgroup freezer; or
> +3. "SPAM" transactions that are considered spamming according to existing
> +logic in binder_alloc.c.
> +
> +When the specified binder transactions happen, the driver uses the
> +BINDER_NETLINK_CMD_REPORT command to send a generic netlink message to the
> +registered process, containing the payload defined in binder_netlink.yaml.
> +
> +More details about the flags, attributes and operations can be found at the
> +the doc sections in Documentations/netlink/specs/binder_netlink.yaml and the
> +kernel-doc comments of the new source code in binder.{h|c}.
> +
> +Using Binder Netlink
> +--------------------
> +
> +The Binder Netlink can be used in the same way as any other generic netlink
> +drivers. Userspace application uses a raw netlink socket to send commands
> +to and receive packets from the kernel driver.
> +
> +.. note::
> +    If the userspace application that talks to the driver exits, the kernel
> +    driver will automatically reset the configuration to the default and
> +    stop sending more reports, which would otherwise fail.
> +
> +Usage example (user space pseudo code):
> +
> +::
> +    /*
> +     * send() below is overloaded to pack netlink commands and attributes
> +     * to nlattr/genlmsghdr/nlmsghdr and then send to the netlink socket.
> +     *
> +     * recv() below is overloaded to receive the raw netlink message from
> +     * the netlink socket, parse nlmsghdr/genlmsghdr to find the netlink
> +     * command and then return the nlattr payload.
> +     */
> +
> +    // open netlink socket
> +    int fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_GENERIC);
> +
> +    // bind netlink socket
> +    bind(fd, struct socketaddr);
> +
> +    // get the family id of the binder netlink
> +    send(fd, CTRL_CMD_GETFAMILY, CTRL_ATTR_FAMILY_NAME,
> +            BINDER_NETLINK_FAMILY_NAME);
> +    void *data = recv(CTRL_CMD_NEWFAMILY);
> +    if (!has_nla_type(data, CTRL_ATTR_FAMILY_ID)) {
> +        // Binder Netlink isn't available on this version of Linux kernel
> +        return;
> +    }
> +    __u16 id = nla(data)[CTRL_ATTR_FAMILY_ID];
> +
> +    // enable per-context binder report
> +    send(fd, id, BINDER_NETLINK_CMD_REPORT_SETUP, "binder", 0,
> +            BINDER_NETLINK_FLAG_FAILED | BINDER_NETLINK_FLAG_DELAYED);
> +
> +    // confirm the per-context configuration
> +    data = recv(fd, BINDER_NETLINK_CMD_REPLY);
> +    char *context = nla(data)[BINDER_NETLINK_A_CMD_CONTEXT];
> +    __u32 pid =  nla(data)[BINDER_NETLINK_A_CMD_PID];
> +    __u32 flags = nla(data)[BINDER_NETLINK_A_CMD_FLAGS];
> +
> +    // set optional per-process report, overriding the per-context one
> +    send(fd, id, BINDER_NETLINK_CMD_REPORT_SETUP, "binder", getpid(),
> +            BINDER_NETLINK_FLAG_SPAM | BINDER_REPORT_OVERRIDE);
> +
> +    // confirm the optional per-process configuration
> +    data = recv(fd, BINDER_NETLINK_CMD_REPLY);
> +    context = nla(data)[BINDER_NETLINK_A_CMD_CONTEXT];
> +    pid =  nla(data)[BINDER_NETLINK_A_CMD_PID];
> +    flags = nla(data)[BINDER_NETLINK_A_CMD_FLAGS];
> +
> +    // wait and read all binder reports
> +    while (running) {
> +            data = recv(fd, BINDER_NETLINK_CMD_REPORT);
> +            auto *attr = nla(data)[BINDER_NETLINK_A_REPORT_XXX];
> +
> +            // process binder report
> +            do_something(*attr);
> +    }
> +
> +    // clean up
> +    send(fd, id, BINDER_NETLINK_CMD_REPORT_SETUP, 0, 0);
> +    send(fd, id, BINDER_NETLINK_CMD_REPORT_SETUP, getpid(), 0);
> +    close(fd);
> diff --git a/Documentation/admin-guide/index.rst b/Documentation/admin-guide/index.rst
> index e85b1adf5908..b3b5cfadffe5 100644
> --- a/Documentation/admin-guide/index.rst
> +++ b/Documentation/admin-guide/index.rst
> @@ -79,6 +79,7 @@ configure specific aspects of kernel behavior to your liking.
>     aoe/index
>     auxdisplay/index
>     bcache
> +   binder_genl
>     binderfs
>     binfmt-misc
>     blockdev/index
> diff --git a/Documentation/netlink/specs/binder_netlink.yaml b/Documentation/netlink/specs/binder_netlink.yaml
> new file mode 100644
> index 000000000000..7eef013e6f07
> --- /dev/null
> +++ b/Documentation/netlink/specs/binder_netlink.yaml
> @@ -0,0 +1,108 @@
> +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)

I think you need a Copyright for this. I'm not sure if it would also be
needed for the Documentation though.

> +
> +name: binder_netlink
> +protocol: genetlink
> +uapi-header: linux/android/binder_netlink.h
> +doc: Netlink protocol to report binder transaction errors and warnings.
> +
> +definitions:
> +  -
> +    type: flags
> +    name: flag
> +    doc: Define what kind of binder transactions should be reported.
> +    entries: [ failed, async-frozen, spam, override ]
> +
> +attribute-sets:
> +  -
> +    name: cmd
> +    doc: The supported attributes of "report-setup" command.
> +    attributes:
> +      -
> +        name: context
> +        type: string
> +        doc: The binder context to enable binder netlink report.
> +      -
> +        name: pid
> +        type: u32
> +        doc: The binder proc to enable binder netlink report.
> +      -
> +        name: flags
> +        type: u32
> +        enum: flag
> +        doc: What kind of binder transactions should be reported.
> +  -
> +    name: report
> +    doc: The supported attributes of "report" command
> +    attributes:
> +      -
> +        name: context
> +        type: string
> +        doc: The binder context where the binder netlink report happens.
> +      -
> +        name: err
> +        type: u32
> +        doc: Copy of binder_driver_return_protocol returned to the sender.
> +      -
> +        name: from_pid
> +        type: u32
> +        doc: Sender pid of the corresponding binder transaction.
> +      -
> +        name: from_tid
> +        type: u32
> +        doc: Sender tid of the corresponding binder transaction.
> +      -
> +        name: to_pid
> +        type: u32
> +        doc: Target pid of the corresponding binder transaction.
> +      -
> +        name: to_tid
> +        type: u32
> +        doc: Target tid of the corresponding binder transaction.
> +      -
> +        name: reply
> +        type: u32
> +        doc: 1 means the transaction is a reply, 0 otherwise.
> +      -
> +        name: flags
> +        type: u32
> +        doc: Copy of binder_transaction_data->flags.
> +      -
> +        name: code
> +        type: u32
> +        doc: Copy of binder_transaction_data->code.
> +      -
> +        name: data_size
> +        type: u32
> +        doc: Copy of binder_transaction_data->data_size.
> +
> +operations:
> +  list:
> +    -
> +      name: report-setup
> +      doc: Set flags from user space.
> +      attribute-set: cmd
> +
> +      do:
> +        request: &params
> +          attributes:
> +            - context
> +            - pid
> +            - flags
> +        reply: *params
> +    -
> +      name: report
> +      doc: Send the requested reports to user space.
> +      attribute-set: report
> +
> +      event:
> +        attributes:
> +          - context
> +          - err
> +          - from_pid
> +          - from_tid
> +          - to_pid
> +          - to_tid
> +          - reply
> +          - flags
> +          - code
> +          - data_size
> diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
> index 07aa8ae0a058..e2fa620934e2 100644
> --- a/drivers/android/Kconfig
> +++ b/drivers/android/Kconfig
> @@ -4,6 +4,7 @@ menu "Android"
>  config ANDROID_BINDER_IPC
>  	bool "Android Binder IPC Driver"
>  	depends on MMU
> +	depends on NET
>  	default n
>  	help
>  	  Binder is used in Android for both communication between processes,
> diff --git a/drivers/android/Makefile b/drivers/android/Makefile
> index c9d3d0c99c25..b8874dba884e 100644
> --- a/drivers/android/Makefile
> +++ b/drivers/android/Makefile
> @@ -2,5 +2,5 @@
>  ccflags-y += -I$(src)			# needed for trace events
>  
>  obj-$(CONFIG_ANDROID_BINDERFS)		+= binderfs.o
> -obj-$(CONFIG_ANDROID_BINDER_IPC)	+= binder.o binder_alloc.o
> +obj-$(CONFIG_ANDROID_BINDER_IPC)	+= binder.o binder_alloc.o binder_netlink.o
>  obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 0a16acd29653..44932659051c 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -72,6 +72,7 @@
>  
>  #include <linux/cacheflush.h>
>  
> +#include "binder_netlink.h"
>  #include "binder_internal.h"
>  #include "binder_trace.h"
>  
> @@ -2990,6 +2991,111 @@ static void binder_set_txn_from_error(struct binder_transaction *t, int id,
>  	binder_thread_dec_tmpref(from);
>  }
>  
> +/**
> + * binder_find_proc() - set binder report flags

the description of "binder report flags" is no longer accurate here.

> + * @pid:	the target process
> + */
> +static struct binder_proc *binder_find_proc(int pid)
> +{
> +	struct binder_proc *proc;
> +
> +	mutex_lock(&binder_procs_lock);
> +	hlist_for_each_entry(proc, &binder_procs, proc_node) {
> +		if (proc->pid == pid) {
> +			mutex_unlock(&binder_procs_lock);
> +			return proc;

fwiw, the for_each stops when proc is NULL, so you can just break and
return proc everytime. e.g.:

	mutex_lock(&binder_procs_lock);
	hlist_for_each_entry(proc, &binder_procs, proc_node) {
		if (proc->pid == pid)
			break;
	}
	mutex_unlock(&binder_procs_lock);

	return proc;


> +		}
> +	}
> +	mutex_unlock(&binder_procs_lock);
> +
> +	return NULL;
> +}
> +
> +/**
> + * binder_netlink_enabled() - check if binder netlink reports are enabled
> + * @proc:	the binder_proc to check
> + * @mask:	the categories of binder netlink reports
> + *
> + * Returns true if certain binder netlink reports are enabled for this binder
> + * proc (when per-process overriding takes effect) or context.
> + */
> +static bool binder_netlink_enabled(struct binder_proc *proc, u32 mask)
> +{
> +	struct binder_context *context = proc->context;
> +
> +	if (!context->report_portid)
> +		return false;
> +
> +	if (proc->report_flags & BINDER_NETLINK_FLAG_OVERRIDE)
> +		return (proc->report_flags & mask) != 0;
> +	else
> +		return (context->report_flags & mask) != 0;
> +}
> +
> +/**
> + * binder_netlink_report() - report one binder netlink event
> + * @context:	the binder context
> + * @err:	copy of binder_driver_return_protocol returned to the sender
> + * @pid:	sender process
> + * @tid:	sender thread
> + * @to_pid:	target process
> + * @to_tid:	target thread
> + * @reply:	whether the binder transaction is a reply
> + * @tr:		the binder transaction data
> + *
> + * Packs the report data into a binder netlink message and send it.
> + */
> +static void binder_netlink_report(struct binder_context *context, u32 err,
> +				  u32 pid, u32 tid, u32 to_pid, u32 to_tid,
> +				  u32 reply,
> +				  struct binder_transaction_data *tr)
> +{
> +	int ret;
> +	struct sk_buff *skb;
> +	void *hdr;
> +
> +	trace_binder_netlink_report(context->name, err, pid, tid, to_pid,
> +				    to_tid, reply, tr);
> +
> +	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> +	if (!skb) {
> +		pr_err("Failed to alloc binder netlink message\n");
> +		return;
> +	}
> +
> +	hdr = genlmsg_put(skb, 0, atomic_inc_return(&context->report_seq),
> +			  &binder_netlink_nl_family, 0, BINDER_NETLINK_CMD_REPORT);
> +	if (!hdr)
> +		goto free_skb;
> +
> +	if (nla_put_string(skb, BINDER_NETLINK_A_REPORT_CONTEXT, context->name) ||
> +	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_ERR, err) ||
> +	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_FROM_PID, pid) ||
> +	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_FROM_TID, tid) ||
> +	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_TO_PID, to_pid) ||
> +	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_TO_TID, to_tid) ||
> +	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_REPLY, reply) ||
> +	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_FLAGS, tr->flags) ||
> +	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_CODE, tr->code) ||
> +	    nla_put_u32(skb, BINDER_NETLINK_A_REPORT_DATA_SIZE, tr->data_size))
> +		goto cancel_skb;
> +
> +	genlmsg_end(skb, hdr);
> +
> +	ret = genlmsg_unicast(&init_net, skb, context->report_portid);
> +	if (ret < 0)
> +		pr_err("Failed to send binder netlink message to %d: %d\n",
> +		       context->report_portid, ret);
> +	return;
> +
> +cancel_skb:
> +	pr_err("Failed to add attributes to binder netlink message\n");
> +	genlmsg_cancel(skb, hdr);
> +free_skb:
> +	pr_err("Free binder netlink report message on error\n");
> +	nlmsg_free(skb);
> +}
> +
>  static void binder_transaction(struct binder_proc *proc,
>  			       struct binder_thread *thread,
>  			       struct binder_transaction_data *tr, int reply,
> @@ -3684,10 +3790,17 @@ static void binder_transaction(struct binder_proc *proc,
>  		return_error_line = __LINE__;
>  		goto err_copy_data_failed;
>  	}
> -	if (t->buffer->oneway_spam_suspect)
> +	if (t->buffer->oneway_spam_suspect) {
>  		tcomplete->type = BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT;
> -	else
> +		if (binder_netlink_enabled(proc, BINDER_NETLINK_FLAG_SPAM))
> +			binder_netlink_report(context, BR_ONEWAY_SPAM_SUSPECT,
> +					      proc->pid, thread->pid,
> +					      target_proc ? target_proc->pid : 0,
> +					      target_thread ? target_thread->pid : 0,
> +					      reply, tr);
> +	} else {
>  		tcomplete->type = BINDER_WORK_TRANSACTION_COMPLETE;
> +	}
>  	t->work.type = BINDER_WORK_TRANSACTION;
>  
>  	if (reply) {
> @@ -3737,8 +3850,15 @@ static void binder_transaction(struct binder_proc *proc,
>  		 * process and is put in a pending queue, waiting for the target
>  		 * process to be unfrozen.
>  		 */
> -		if (return_error == BR_TRANSACTION_PENDING_FROZEN)
> +		if (return_error == BR_TRANSACTION_PENDING_FROZEN) {
>  			tcomplete->type = BINDER_WORK_TRANSACTION_PENDING;
> +			if (binder_netlink_enabled(proc, BINDER_NETLINK_FLAG_ASYNC_FROZEN))
> +				binder_netlink_report(context, return_error,
> +						      proc->pid, thread->pid,
> +						      target_proc ? target_proc->pid : 0,
> +						      target_thread ? target_thread->pid : 0,
> +						      reply, tr);
> +		}
>  		binder_enqueue_thread_work(thread, tcomplete);
>  		if (return_error &&
>  		    return_error != BR_TRANSACTION_PENDING_FROZEN)
> @@ -3800,6 +3920,13 @@ static void binder_transaction(struct binder_proc *proc,
>  		binder_dec_node_tmpref(target_node);
>  	}
>  
> +	if (binder_netlink_enabled(proc, BINDER_NETLINK_FLAG_FAILED))
> +		binder_netlink_report(context, return_error,
> +				      proc->pid, thread->pid,
> +				      target_proc ? target_proc->pid : 0,
> +				      target_thread ? target_thread->pid : 0,
> +				      reply, tr);
> +
>  	binder_debug(BINDER_DEBUG_FAILED_TRANSACTION,
>  		     "%d:%d transaction %s to %d:%d failed %d/%d/%d, size %lld-%lld line %d\n",
>  		     proc->pid, thread->pid, reply ? "reply" :
> @@ -6137,6 +6264,11 @@ static int binder_release(struct inode *nodp, struct file *filp)
>  
>  	binder_defer_work(proc, BINDER_DEFERRED_RELEASE);
>  
> +	if (proc->pid == proc->context->report_portid) {
> +		proc->context->report_portid = 0;
> +		proc->context->report_flags = 0;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -6335,6 +6467,99 @@ binder_defer_work(struct binder_proc *proc, enum binder_deferred_state defer)
>  	mutex_unlock(&binder_deferred_lock);
>  }
>  
> +/**
> + * binder_netlink_nl_report_setup_doit() - netlink .doit handler
> + * @skb:	the metadata struct passed from netlink driver
> + * @info:	the generic netlink struct passed from netlink driver
> + *
> + * Implements the .doit function to process binder netlink commands.
> + */
> +int binder_netlink_nl_report_setup_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> +	int portid;
> +	u32 pid;
> +	u32 flags;
> +	void *hdr;
> +	struct binder_proc *proc;
> +	struct binder_device *device;
> +	struct binder_context *context = NULL;
> +
> +	hlist_for_each_entry(device, &binder_devices, hlist) {
> +		if (!nla_strcmp(info->attrs[BINDER_NETLINK_A_CMD_CONTEXT],
> +				device->context.name)) {
> +			context = &device->context;
> +			break;
> +		}
> +	}
> +
> +	if (!context) {
> +		NL_SET_ERR_MSG(info->extack, "Unknown binder context");
> +		return -EINVAL;
> +	}
> +
> +	portid = nlmsg_hdr(skb)->nlmsg_pid;
> +	pid = nla_get_u32(info->attrs[BINDER_NETLINK_A_CMD_PID]);
> +	flags = nla_get_u32(info->attrs[BINDER_NETLINK_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;
> +	} else {
> +		/* Set the per-process flags */
> +		proc = binder_find_proc(pid);
> +		if (!proc) {
> +			NL_SET_ERR_MSG_FMT(info->extack,
> +					   "Invalid binder report pid %u",
> +					   pid);
> +			return -EINVAL;
> +		}
> +
> +		proc->report_flags = flags;
> +	}
> +
> +	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> +	if (!skb) {
> +		pr_err("Failed to alloc binder netlink reply message\n");
> +		return -ENOMEM;
> +	}
> +
> +	hdr = genlmsg_iput(skb, info);
> +	if (!hdr)
> +		goto free_skb;
> +
> +	if (nla_put_string(skb, BINDER_NETLINK_A_CMD_CONTEXT, context->name) ||
> +	    nla_put_u32(skb, BINDER_NETLINK_A_CMD_PID, pid) ||
> +	    nla_put_u32(skb, BINDER_NETLINK_A_CMD_FLAGS, flags))
> +		goto cancel_skb;
> +
> +	genlmsg_end(skb, hdr);
> +
> +	if (genlmsg_reply(skb, info)) {
> +		pr_err("Failed to send binder netlink reply message\n");
> +		return -EFAULT;
> +	}
> +
> +	if (!context->report_portid)
> +		context->report_portid = portid;
> +
> +	return 0;
> +
> +cancel_skb:
> +	pr_err("Failed to add reply attributes to binder netlink message\n");
> +	genlmsg_cancel(skb, hdr);
> +free_skb:
> +	pr_err("Free binder netlink reply message on error\n");
> +	nlmsg_free(skb);
> +	return -EMSGSIZE;
> +}
> +
>  static void print_binder_transaction_ilocked(struct seq_file *m,
>  					     struct binder_proc *proc,
>  					     const char *prefix,
> @@ -7014,6 +7239,12 @@ static int __init binder_init(void)
>  	if (ret)
>  		goto err_init_binder_device_failed;
>  
> +	ret = genl_register_family(&binder_netlink_nl_family);
> +	if (ret) {
> +		pr_err("Failed to register binder netlink family\n");
> +		goto err_init_binder_device_failed;
> +	}
> +
>  	return ret;
>  
>  err_init_binder_device_failed:
> diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
> index 1f21ad3963b1..c67eba88f89a 100644
> --- a/drivers/android/binder_internal.h
> +++ b/drivers/android/binder_internal.h
> @@ -12,21 +12,35 @@
>  #include <linux/stddef.h>
>  #include <linux/types.h>
>  #include <linux/uidgid.h>
> +#include <net/genetlink.h>
>  #include <uapi/linux/android/binderfs.h>
>  #include "binder_alloc.h"
>  #include "dbitmap.h"
>  
> +/**
> + * struct binder_context - information about a binder domain
> + * @binder_context_mgr_node: the context manager
> + * @context_mgr_node_lock:   the lock protecting the above context manager node
> + * @binder_context_mgr_uid:  the uid of the above context manager
> + * @name:                    the name of the binder device
> + * @report_portid:           the netlink socket to receive binder reports
> + * @report_flags:            the categories of binder transactions that would
> + *                           be reported (see enum binder_report_flag).
> + * @report_seq:              the seq number of the generic netlink report
> + */
>  struct binder_context {
>  	struct binder_node *binder_context_mgr_node;
>  	struct mutex context_mgr_node_lock;
>  	kuid_t binder_context_mgr_uid;
>  	const char *name;
> +	u32 report_portid;
> +	u32 report_flags;
> +	atomic_t report_seq;
>  };
>  
>  /**
>   * struct binder_device - information about a binder device node
> - * @hlist:          list of binder devices (only used for devices requested via
> - *                  CONFIG_ANDROID_BINDER_DEVICES)
> + * @hlist:          list of binder devices

This is the hunk that needs to go on the first 1/2 patch.


>   * @miscdev:        information about a binder character device node
>   * @context:        binder context information
>   * @binderfs_inode: This is the inode of the root dentry of the super block
> @@ -415,6 +429,8 @@ struct binder_ref {
>   * @binderfs_entry:       process-specific binderfs log file
>   * @oneway_spam_detection_enabled: process enabled oneway spam detection
>   *                        or not
> + * @report_flags:         the categories of binder transactions that would
> + *                        be reported (see enum binder_genl_flag).
>   *
>   * Bookkeeping structure for binder processes
>   */
> @@ -453,6 +469,7 @@ struct binder_proc {
>  	spinlock_t outer_lock;
>  	struct dentry *binderfs_entry;
>  	bool oneway_spam_detection_enabled;
> +	u32 report_flags;
>  };
>  
>  /**
> diff --git a/drivers/android/binder_netlink.c b/drivers/android/binder_netlink.c
> new file mode 100644
> index 000000000000..2081b4319268
> --- /dev/null
> +++ b/drivers/android/binder_netlink.c
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
> +/* Do not edit directly, auto-generated from: */
> +/*	Documentation/netlink/specs/binder_netlink.yaml */
> +/* YNL-GEN kernel source */
> +
> +#include <net/netlink.h>
> +#include <net/genetlink.h>
> +
> +#include "binder_netlink.h"
> +
> +#include <uapi/linux/android/binder_netlink.h>
> +
> +/* BINDER_NETLINK_CMD_REPORT_SETUP - do */
> +static const struct nla_policy binder_netlink_report_setup_nl_policy[BINDER_NETLINK_A_CMD_FLAGS + 1] = {
> +	[BINDER_NETLINK_A_CMD_CONTEXT] = { .type = NLA_NUL_STRING, },
> +	[BINDER_NETLINK_A_CMD_PID] = { .type = NLA_U32, },
> +	[BINDER_NETLINK_A_CMD_FLAGS] = NLA_POLICY_MASK(NLA_U32, 0xf),
> +};
> +
> +/* Ops table for binder_netlink */
> +static const struct genl_split_ops binder_netlink_nl_ops[] = {

not: There are several places where you have "netlink_nl" which seems
kind of redundant to me. wdyt? IMO you should drop the "nl" in all of
these cases.

> +	{
> +		.cmd		= BINDER_NETLINK_CMD_REPORT_SETUP,
> +		.doit		= binder_netlink_nl_report_setup_doit,
> +		.policy		= binder_netlink_report_setup_nl_policy,
> +		.maxattr	= BINDER_NETLINK_A_CMD_FLAGS,
> +		.flags		= GENL_CMD_CAP_DO,
> +	},
> +};
> +
> +struct genl_family binder_netlink_nl_family __ro_after_init = {
> +	.name		= BINDER_NETLINK_FAMILY_NAME,
> +	.version	= BINDER_NETLINK_FAMILY_VERSION,
> +	.netnsok	= true,
> +	.parallel_ops	= true,
> +	.module		= THIS_MODULE,
> +	.split_ops	= binder_netlink_nl_ops,
> +	.n_split_ops	= ARRAY_SIZE(binder_netlink_nl_ops),
> +};
> diff --git a/drivers/android/binder_netlink.h b/drivers/android/binder_netlink.h
> new file mode 100644
> index 000000000000..022cff5f7c38
> --- /dev/null
> +++ b/drivers/android/binder_netlink.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
> +/* Do not edit directly, auto-generated from: */
> +/*	Documentation/netlink/specs/binder_netlink.yaml */
> +/* YNL-GEN kernel header */
> +
> +#ifndef _LINUX_BINDER_NETLINK_GEN_H
> +#define _LINUX_BINDER_NETLINK_GEN_H
> +
> +#include <net/netlink.h>
> +#include <net/genetlink.h>
> +
> +#include <uapi/linux/android/binder_netlink.h>
> +
> +int binder_netlink_nl_report_setup_doit(struct sk_buff *skb,
> +					struct genl_info *info);
> +
> +extern struct genl_family binder_netlink_nl_family;
> +
> +#endif /* _LINUX_BINDER_NETLINK_GEN_H */
> diff --git a/drivers/android/binder_trace.h b/drivers/android/binder_trace.h
> index fe38c6fc65d0..8976fb5ee2db 100644
> --- a/drivers/android/binder_trace.h
> +++ b/drivers/android/binder_trace.h
> @@ -423,6 +423,41 @@ TRACE_EVENT(binder_return,
>  			  "unknown")
>  );
>  
> +TRACE_EVENT(binder_netlink_report,
> +	TP_PROTO(const char *name, u32 err, u32 pid, u32 tid, u32 to_pid,
> +		 u32 to_tid, u32 reply, struct binder_transaction_data *tr),
> +	TP_ARGS(name, err, pid, tid, to_pid, to_tid, reply, tr),
> +	TP_STRUCT__entry(
> +		__field(const char *, name)
> +		__field(u32, err)
> +		__field(u32, pid)
> +		__field(u32, tid)
> +		__field(u32, to_pid)
> +		__field(u32, to_tid)
> +		__field(u32, reply)
> +		__field(u32, flags)
> +		__field(u32, code)
> +		__field(binder_size_t, data_size)
> +	),
> +	TP_fast_assign(
> +		__entry->name = name;
> +		__entry->err = err;
> +		__entry->pid = pid;
> +		__entry->tid = tid;
> +		__entry->to_pid = to_pid;
> +		__entry->to_tid = to_tid;
> +		__entry->reply = reply;
> +		__entry->flags = tr->flags;
> +		__entry->code = tr->code;
> +		__entry->data_size = tr->data_size;
> +	),
> +	TP_printk("%s: %d %d:%d -> %d:%d %s flags=0x08%x code=%d %llu",
> +		  __entry->name, __entry->err, __entry->pid, __entry->tid,
> +		  __entry->to_pid, __entry->to_tid,
> +		  __entry->reply ? "reply" : "",
> +		  __entry->flags, __entry->code, __entry->data_size)
> +);
> +
>  #endif /* _BINDER_TRACE_H */
>  
>  #undef TRACE_INCLUDE_PATH
> diff --git a/include/uapi/linux/android/binder_netlink.h b/include/uapi/linux/android/binder_netlink.h
> new file mode 100644
> index 000000000000..2b1460387597
> --- /dev/null
> +++ b/include/uapi/linux/android/binder_netlink.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
> +/* Do not edit directly, auto-generated from: */
> +/*	Documentation/netlink/specs/binder_netlink.yaml */
> +/* YNL-GEN uapi header */
> +
> +#ifndef _UAPI_LINUX_ANDROID_BINDER_NETLINK_H
> +#define _UAPI_LINUX_ANDROID_BINDER_NETLINK_H
> +
> +#define BINDER_NETLINK_FAMILY_NAME	"binder_netlink"
> +#define BINDER_NETLINK_FAMILY_VERSION	1
> +
> +/*
> + * Define what kind of binder transactions should be reported.
> + */
> +enum binder_netlink_flag {
> +	BINDER_NETLINK_FLAG_FAILED = 1,
> +	BINDER_NETLINK_FLAG_ASYNC_FROZEN = 2,
> +	BINDER_NETLINK_FLAG_SPAM = 4,
> +	BINDER_NETLINK_FLAG_OVERRIDE = 8,
> +};
> +
> +enum {
> +	BINDER_NETLINK_A_CMD_CONTEXT = 1,
> +	BINDER_NETLINK_A_CMD_PID,
> +	BINDER_NETLINK_A_CMD_FLAGS,
> +
> +	__BINDER_NETLINK_A_CMD_MAX,
> +	BINDER_NETLINK_A_CMD_MAX = (__BINDER_NETLINK_A_CMD_MAX - 1)
> +};
> +
> +enum {
> +	BINDER_NETLINK_A_REPORT_CONTEXT = 1,
> +	BINDER_NETLINK_A_REPORT_ERR,
> +	BINDER_NETLINK_A_REPORT_FROM_PID,
> +	BINDER_NETLINK_A_REPORT_FROM_TID,
> +	BINDER_NETLINK_A_REPORT_TO_PID,
> +	BINDER_NETLINK_A_REPORT_TO_TID,
> +	BINDER_NETLINK_A_REPORT_REPLY,
> +	BINDER_NETLINK_A_REPORT_FLAGS,
> +	BINDER_NETLINK_A_REPORT_CODE,
> +	BINDER_NETLINK_A_REPORT_DATA_SIZE,
> +
> +	__BINDER_NETLINK_A_REPORT_MAX,
> +	BINDER_NETLINK_A_REPORT_MAX = (__BINDER_NETLINK_A_REPORT_MAX - 1)
> +};
> +
> +enum {
> +	BINDER_NETLINK_CMD_REPORT_SETUP = 1,
> +	BINDER_NETLINK_CMD_REPORT,
> +
> +	__BINDER_NETLINK_CMD_MAX,
> +	BINDER_NETLINK_CMD_MAX = (__BINDER_NETLINK_CMD_MAX - 1)
> +};
> +
> +#endif /* _UAPI_LINUX_ANDROID_BINDER_NETLINK_H */
> -- 
> 2.47.1.613.gc27f4b7a9f-goog
> 

Re: [PATCH net-next v10 2/2] binder: report txn errors via generic netlink

[Li Li]

On Mon, Dec 16, 2024 at 10:12 AM Carlos Llamas <cmllamas@google.com> wrote:
>
> On Thu, Dec 12, 2024 at 02:41:14PM -0800, Li Li wrote:
> > ---
> >  Documentation/admin-guide/binder_genl.rst     | 110 ++++++++
>
> Thanks for renaming to "Binder Netlink" this seems much better IMO.
> Also, I belive the documentation should also be binder_netlink.rst in
> such case?
>

Will also change it.

> > +++ b/Documentation/netlink/specs/binder_netlink.yaml
> > @@ -0,0 +1,108 @@
> > +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
>
> I think you need a Copyright for this. I'm not sure if it would also be
> needed for the Documentation though.
>

Hmm, all other yaml files follow the same code style.

Netlink experts, can you please clarify this?

> > +/**
> > + * binder_find_proc() - set binder report flags
>
> the description of "binder report flags" is no longer accurate here.

Good catch! Will fix that.

>
> > + * @pid:     the target process
> > + */
> > +static struct binder_proc *binder_find_proc(int pid)
> > +{
> > +     struct binder_proc *proc;
> > +
> > +     mutex_lock(&binder_procs_lock);
> > +     hlist_for_each_entry(proc, &binder_procs, proc_node) {
> > +             if (proc->pid == pid) {
> > +                     mutex_unlock(&binder_procs_lock);
> > +                     return proc;
>
> fwiw, the for_each stops when proc is NULL, so you can just break and
> return proc everytime. e.g.:
>
>         mutex_lock(&binder_procs_lock);
>         hlist_for_each_entry(proc, &binder_procs, proc_node) {
>                 if (proc->pid == pid)
>                         break;
>         }
>         mutex_unlock(&binder_procs_lock);
>
>         return proc;
>

Got it. Thx

> >  /**
> >   * struct binder_device - information about a binder device node
> > - * @hlist:          list of binder devices (only used for devices requested via
> > - *                  CONFIG_ANDROID_BINDER_DEVICES)
> > + * @hlist:          list of binder devices
>
> This is the hunk that needs to go on the first 1/2 patch.

Sure.

> >  /**
> > diff --git a/drivers/android/binder_netlink.c b/drivers/android/binder_netlink.c
> > new file mode 100644
> > index 000000000000..2081b4319268
> > --- /dev/null
> > +++ b/drivers/android/binder_netlink.c
> > @@ -0,0 +1,39 @@
> > +// SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
> > +/* Do not edit directly, auto-generated from: */
> > +/*   Documentation/netlink/specs/binder_netlink.yaml */
> > +/* YNL-GEN kernel source */
> > +
> > +#include <net/netlink.h>
> > +#include <net/genetlink.h>
> > +
> > +#include "binder_netlink.h"
> > +
> > +#include <uapi/linux/android/binder_netlink.h>
> > +
> > +/* BINDER_NETLINK_CMD_REPORT_SETUP - do */
> > +static const struct nla_policy binder_netlink_report_setup_nl_policy[BINDER_NETLINK_A_CMD_FLAGS + 1] = {
> > +     [BINDER_NETLINK_A_CMD_CONTEXT] = { .type = NLA_NUL_STRING, },
> > +     [BINDER_NETLINK_A_CMD_PID] = { .type = NLA_U32, },
> > +     [BINDER_NETLINK_A_CMD_FLAGS] = NLA_POLICY_MASK(NLA_U32, 0xf),
> > +};
> > +
> > +/* Ops table for binder_netlink */
> > +static const struct genl_split_ops binder_netlink_nl_ops[] = {
>
> not: There are several places where you have "netlink_nl" which seems
> kind of redundant to me. wdyt? IMO you should drop the "nl" in all of
> these cases.
>

These are automatically generated from the yaml file. So let's just
keep them as is.
But it's a good suggestion to the owner of yaml parser.

Re: [PATCH net-next v10 2/2] binder: report txn errors via generic netlink

[Jakub Kicinski]

On Mon, 16 Dec 2024 10:58:10 -0800 Li Li wrote:
> > not: There are several places where you have "netlink_nl" which seems
> > kind of redundant to me. wdyt? IMO you should drop the "nl" in all of
> > these cases.
> >  
> 
> These are automatically generated from the yaml file. So let's just
> keep them as is.
> But it's a good suggestion to the owner of yaml parser.

I think the unusual naming comes from fact that you call your netlink
family binder_netlink. It's sort of like adding the word sysfs to the
name of a sysfs file. I mean the user visible name, not code
references...

s/binder_netlink/binder/ will clean this up..

Re: [PATCH net-next v10 2/2] binder: report txn errors via generic netlink

[Li Li]

On Mon, Dec 16, 2024, 5:41 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 16 Dec 2024 10:58:10 -0800 Li Li wrote:
> > > not: There are several places where you have "netlink_nl" which seems
> > > kind of redundant to me. wdyt? IMO you should drop the "nl" in all of
> > > these cases.
> > >
> >
> > These are automatically generated from the yaml file. So let's just
> > keep them as is.
> > But it's a good suggestion to the owner of yaml parser.
>
> I think the unusual naming comes from fact that you call your netlink
> family binder_netlink. It's sort of like adding the word sysfs to the
> name of a sysfs file. I mean the user visible name, not code
> references...
>
> s/binder_netlink/binder/ will clean this up..


I did consider that but unfortunately that would result in a
conflicting binder.h in uapi header.

Probably "binder_report" or "bindererr"?

Re: [PATCH net-next v10 2/2] binder: report txn errors via generic netlink

[Jakub Kicinski]

On Mon, 16 Dec 2024 19:53:40 -0800 Li Li wrote:
> > > These are automatically generated from the yaml file. So let's just
> > > keep them as is.
> > > But it's a good suggestion to the owner of yaml parser.  
> >
> > I think the unusual naming comes from fact that you call your netlink
> > family binder_netlink. It's sort of like adding the word sysfs to the
> > name of a sysfs file. I mean the user visible name, not code
> > references...
> >
> > s/binder_netlink/binder/ will clean this up..  
> 
> I did consider that but unfortunately that would result in a
> conflicting binder.h in uapi header.

What exactly is conflicting? The name of the header itself?
You can set the uapi header name using the uapi-header property.