* [PATCH v3 0/1] binder: report txn errors via generic netlink (genl)
@ 2024-10-21 18:28 Li Li
2024-10-21 18:28 ` [PATCH v3 1/1] binder: report txn errors via generic netlink Li Li
2024-10-21 18:28 ` [PATCH v3 1/1] report binder " Li Li
0 siblings, 2 replies; 8+ messages in thread
From: Li Li @ 2024-10-21 18:28 UTC (permalink / raw)
To: dualli, corbet, davem, edumazet, kuba, pabeni, donald.hunter,
gregkh, arve, tkjos, maco, joel, brauner, cmllamas, surenb, arnd,
masahiroy, linux-kernel, linux-doc, netdev, hridya, smoreland
Cc: kernel-team
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 first 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 second 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/
This version replaces the handcrafted netlink source code with the
netlink protocal specs in YAML. It also fixes the documentation issues.
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
Li Li (1):
report binder txn errors via generic netlink
Documentation/admin-guide/binder_genl.rst | 92 ++++++
Documentation/admin-guide/index.rst | 1 +
Documentation/netlink/specs/binder_genl.yaml | 59 ++++
drivers/android/Kconfig | 1 +
drivers/android/Makefile | 2 +-
drivers/android/binder.c | 287 ++++++++++++++++++-
drivers/android/binder_genl.c | 38 +++
drivers/android/binder_genl.h | 18 ++
drivers/android/binder_internal.h | 22 ++
drivers/android/binder_trace.h | 37 +++
drivers/android/binderfs.c | 4 +
include/uapi/linux/android/binder.h | 31 ++
include/uapi/linux/android/binder_genl.h | 37 +++
13 files changed, 625 insertions(+), 4 deletions(-)
create mode 100644 Documentation/admin-guide/binder_genl.rst
create mode 100644 Documentation/netlink/specs/binder_genl.yaml
create mode 100644 drivers/android/binder_genl.c
create mode 100644 drivers/android/binder_genl.h
create mode 100644 include/uapi/linux/android/binder_genl.h
--
2.47.0.105.g07ac214952-goog
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/1] binder: report txn errors via generic netlink
2024-10-21 18:28 [PATCH v3 0/1] binder: report txn errors via generic netlink (genl) Li Li
@ 2024-10-21 18:28 ` Li Li
2024-10-21 18:35 ` Li Li
2024-10-23 16:47 ` kernel test robot
2024-10-21 18:28 ` [PATCH v3 1/1] report binder " Li Li
1 sibling, 2 replies; 8+ messages in thread
From: Li Li @ 2024-10-21 18:28 UTC (permalink / raw)
To: dualli, corbet, davem, edumazet, kuba, pabeni, donald.hunter,
gregkh, arve, tkjos, maco, joel, brauner, cmllamas, surenb, arnd,
masahiroy, linux-kernel, linux-doc, netdev, hridya, smoreland
Cc: kernel-team
From: Li Li <dualli@google.com>
Frozen tasks can't process binder transactions, so sync binder
transactions will fail with BR_FROZEN_REPLY and async binder
transactions will be queued in the kernel async binder buffer.
As these queued async transactions accumulates over time, the async
buffer will eventually be running out, denying all new transactions
after that with BR_FAILED_REPLY.
In addition to the above cases, different kinds of binder error codes
might be returned to the sender. However, the core Linux, or Android,
system administration process never knows what's actually happening.
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 transactions.
To prevent making the already bloated binder.c even bigger, a new source
file binder_genl.c is created to host those generic netlink code.
Signed-off-by: Li Li <dualli@google.com>
---
Documentation/admin-guide/binder_genl.rst | 71 ++++++
Documentation/admin-guide/index.rst | 1 +
drivers/android/Kconfig | 1 +
drivers/android/Makefile | 2 +-
drivers/android/binder.c | 82 ++++++-
drivers/android/binder_genl.c | 249 ++++++++++++++++++++++
drivers/android/binder_internal.h | 31 +++
drivers/android/binder_trace.h | 37 ++++
drivers/android/binderfs.c | 4 +
include/uapi/linux/android/binder.h | 132 ++++++++++++
10 files changed, 606 insertions(+), 4 deletions(-)
create mode 100644 Documentation/admin-guide/binder_genl.rst
create mode 100644 drivers/android/binder_genl.c
diff --git a/Documentation/admin-guide/binder_genl.rst b/Documentation/admin-guide/binder_genl.rst
new file mode 100644
index 000000000000..923a67bbf8ec
--- /dev/null
+++ b/Documentation/admin-guide/binder_genl.rst
@@ -0,0 +1,71 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===========================================================
+Generic Netlink for the Android Binder Driver (Binder Genl)
+===========================================================
+
+The Generic Netlink subsystem in the Linux kernel provides a generic way for
+the Linux kernel to communicate to the user space applications. In the kernel
+binder driver, it is used to report various kinds of binder transactions to
+user space administratioin process. The binder driver allows multiple binder
+devices and their correspondign binder contexts. Each binder context has a
+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.
+
+Using Binder Genl
+-----------------
+
+The Binder Genl can be used in the same way as any other generic netlink
+drivers. The user space application uses a raw netlink socket to send commands
+to and receive packets from the kernel driver.
+
+NOTE: if the user applications that talks to the Binder Genl driver exits,
+the kernel driver will automatically reset the configuration to the default
+and stop sending more reports to prevent leaking memory.
+
+Usage example (user space pseudo code):
+
+::
+
+ // 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 genl
+ send(fd, CTRL_CMD_GETFAMILY, CTRL_ATTR_FAMILY_NAME, "binder");
+ void *data = recv(CTRL_CMD_NEWFAMILY);
+ __u16 id = nla(data)[CTRL_ATTR_FAMILY_ID];
+
+ // enable per-context binder report
+ send(fd, id, BINDER_GENL_SET_REPORT, 0, BINDER_REPORT_ALL);
+
+ // confirm the per-context configuration
+ void *data = recv(fd, BINDER_GENL_CMD_REPLY);
+ __u32 pid = nla(data)[BINDER_GENL_ATTR_PID];
+ __u32 flags = nla(data)[BINDER_GENL_ATTR_FLAGS];
+
+ // set optional per-process report, overriding the per-context one
+ send(fd, id, BINDER_GENL_SET_REPORT, getpid(),
+ BINDER_REPORT_FAILED | BINDER_REPORT_OVERRIDE);
+
+ // confirm the optional per-process configuration
+ void *data = recv(fd, BINDER_GENL_CMD_REPLY);
+ __u32 pid = nla(data)[BINDER_GENL_ATTR_PID];
+ __u32 flags = nla(data)[BINDER_GENL_ATTR_FLAGS];
+
+ // wait and read all binder reports
+ while (running) {
+ void *data = recv(fd, BINDER_GENL_CMD_REPORT);
+ struct binder_report report = nla(data)[BINDER_GENL_ATTR_REPORT];
+
+ // process struct binder_report
+ do_something(&report);
+ }
+
+ // clean up
+ send(fd, id, BINDER_GENL_SET_REPORT, 0, 0);
+ send(fd, id, BINDER_GENL_SET_REPORT, 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/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..d818447fbc4c 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_genl.o
obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 978740537a1a..c99d6c6ff13b 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3678,10 +3678,25 @@ 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_genl_report_enabled(proc, BINDER_REPORT_SPAM)) {
+ struct binder_report report;
+
+ report.err = BR_ONEWAY_SPAM_SUSPECT;
+ report.from_pid = proc->pid;
+ report.from_tid = thread->pid;
+ report.to_pid = target_proc ? target_proc->pid : 0;
+ report.to_tid = target_thread ? target_thread->pid : 0;
+ report.reply = reply;
+ report.flags = tr->flags;
+ report.code = tr->code;
+ report.data_size = tr->data_size;
+ binder_genl_send_report(context, &report, sizeof(report));
+ }
+ } else {
tcomplete->type = BINDER_WORK_TRANSACTION_COMPLETE;
+ }
t->work.type = BINDER_WORK_TRANSACTION;
if (reply) {
@@ -3731,8 +3746,24 @@ 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_genl_report_enabled(proc, BINDER_REPORT_DELAYED)) {
+ struct binder_report report;
+
+ report.err = return_error;
+ report.from_pid = proc->pid;
+ report.from_tid = thread->pid;
+ report.to_pid = target_proc ? target_proc->pid : 0;
+ report.to_tid = target_thread ? target_thread->pid : 0;
+ report.reply = reply;
+ report.flags = tr->flags;
+ report.code = tr->code;
+ report.data_size = tr->data_size;
+ binder_genl_send_report(context, &report,
+ sizeof(report));
+ }
+ }
binder_enqueue_thread_work(thread, tcomplete);
if (return_error &&
return_error != BR_TRANSACTION_PENDING_FROZEN)
@@ -3794,6 +3825,21 @@ static void binder_transaction(struct binder_proc *proc,
binder_dec_node_tmpref(target_node);
}
+ if (binder_genl_report_enabled(proc, BINDER_REPORT_FAILED)) {
+ struct binder_report report;
+
+ report.err = return_error;
+ report.from_pid = proc->pid;
+ report.from_tid = thread->pid;
+ report.to_pid = target_proc ? target_proc->pid : 0;
+ report.to_tid = target_thread ? target_thread->pid : 0;
+ report.reply = reply;
+ report.flags = tr->flags;
+ report.code = tr->code;
+ report.data_size = tr->data_size;
+ binder_genl_send_report(context, &report, sizeof(report));
+ }
+
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" :
@@ -6114,6 +6160,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;
}
@@ -6311,6 +6362,26 @@ binder_defer_work(struct binder_proc *proc, enum binder_deferred_state defer)
mutex_unlock(&binder_deferred_lock);
}
+/**
+ * binder_find_proc() - set binder report flags
+ * @pid: the target process
+ */
+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;
+}
+
static void print_binder_transaction_ilocked(struct seq_file *m,
struct binder_proc *proc,
const char *prefix,
@@ -6920,6 +6991,11 @@ static int __init init_binder_device(const char *name)
hlist_add_head(&binder_device->hlist, &binder_devices);
+ binder_device->context.report_seq = (atomic_t)ATOMIC_INIT(0);
+ ret = binder_genl_init(&binder_device->context.genl_family, name);
+ if (ret < 0)
+ kfree(binder_device);
+
return ret;
}
diff --git a/drivers/android/binder_genl.c b/drivers/android/binder_genl.c
new file mode 100644
index 000000000000..8f82f039bc17
--- /dev/null
+++ b/drivers/android/binder_genl.c
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* binder_genl.c
+ *
+ * Android IPC Subsystem
+ *
+ * Copyright (C) 2024 Google, Inc.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/skbuff.h>
+#include <linux/string.h>
+#include <net/sock.h>
+#include <uapi/linux/android/binder.h>
+
+#include "binder_internal.h"
+#include "binder_trace.h"
+
+/*
+ * The default multicast group
+ */
+static const struct genl_multicast_group binder_genl_mcgrps[] = {
+ { .name = "binder_genl", },
+};
+
+/*
+ * The policy to verify the type of the binder genl data
+ */
+static const struct nla_policy binder_report_policy[BINDER_GENL_ATTR_MAX + 1] = {
+ [BINDER_GENL_ATTR_PID] = { .type = NLA_U32 },
+ [BINDER_GENL_ATTR_FLAGS] = { .type = NLA_U32 },
+};
+
+/**
+ * binder_genl_cmd_doit() - .doit handler for BINDER_GENL_CMD_SET_REPORT
+ * @skb: the metadata struct passed from netlink driver
+ * @info: the generic netlink struct passed from netlink driver
+ *
+ * Implements the .doit function to process binder genl commands.
+ */
+static int binder_genl_cmd_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ int len;
+ int portid;
+ u32 pid;
+ u32 flags;
+ void *hdr;
+ struct binder_context *context;
+
+ /* Both attributes are required for BINDER_GENL_CMD_SET_REPORT */
+ if (!info->attrs[BINDER_GENL_ATTR_PID] || !info->attrs[BINDER_GENL_ATTR_FLAGS]) {
+ pr_err("Attributes not set\n");
+ return -EINVAL;
+ }
+
+ portid = nlmsg_hdr(skb)->nlmsg_pid;
+ pid = nla_get_u32(info->attrs[BINDER_GENL_ATTR_PID]);
+ flags = nla_get_u32(info->attrs[BINDER_GENL_ATTR_FLAGS]);
+ context = container_of(info->family, struct binder_context,
+ genl_family);
+
+ if (context->report_portid && context->report_portid != portid) {
+ pr_err("No permission to set report flags from %u\n", portid);
+ return -EPERM;
+ }
+
+ if (binder_genl_set_report(context, pid, flags) < 0) {
+ pr_err("Failed to set report flags %u for %u\n", flags, pid);
+ return -EINVAL;
+ }
+
+ len = nla_total_size(sizeof(pid)) + nla_total_size(sizeof(flags));
+ skb = genlmsg_new(len, GFP_KERNEL);
+ if (!skb) {
+ pr_err("Failed to alloc binder genl reply message\n");
+ return -ENOMEM;
+ }
+
+ hdr = genlmsg_put_reply(skb, info, info->family, 0,
+ BINDER_GENL_CMD_REPLY);
+ if (!hdr)
+ goto free_skb;
+
+ if (nla_put_u32(skb, BINDER_GENL_ATTR_PID, pid))
+ goto cancel_skb;
+
+ if (nla_put_u32(skb, BINDER_GENL_ATTR_FLAGS, flags))
+ goto cancel_skb;
+
+ genlmsg_end(skb, hdr);
+
+ if (genlmsg_reply(skb, info)) {
+ pr_err("Failed to send binder genl reply message\n");
+ return -EFAULT;
+ }
+
+ if (!context->report_portid)
+ context->report_portid = portid;
+
+ return 0;
+
+cancel_skb:
+ pr_err("Failed to add genl header to reply message\n");
+ genlmsg_cancel(skb, hdr);
+
+free_skb:
+ pr_err("Failed to add genl attribute to reply message\n");
+ nlmsg_free(skb);
+ return -EMSGSIZE;
+}
+
+/*
+ * binder_genl_ops - the small version of generic netlink operations
+ */
+static struct genl_small_ops binder_genl_ops[] = {
+ {
+ .cmd = BINDER_GENL_CMD_SET_REPORT,
+ .doit = binder_genl_cmd_doit,
+ }
+};
+
+/**
+ * binder_genl_init() - initialize binder generic netlink
+ * @family: the generic netlink family
+ * @name: the binder device name
+ *
+ * Registers the binder generic netlink family.
+ */
+int binder_genl_init(struct genl_family *family, const char *name)
+{
+ int ret;
+
+ strscpy(family->name, name, GENL_NAMSIZ);
+ family->version = BINDER_GENL_VERSION;
+ family->maxattr = BINDER_GENL_ATTR_MAX;
+ family->policy = binder_report_policy;
+ family->small_ops = binder_genl_ops;
+ family->n_small_ops = ARRAY_SIZE(binder_genl_ops);
+ family->mcgrps = binder_genl_mcgrps;
+ family->n_mcgrps = ARRAY_SIZE(binder_genl_mcgrps);
+ ret = genl_register_family(family);
+ if (ret) {
+ pr_err("Failed to register binder genl: %s\n", name);
+ return ret;
+ }
+
+ return 0;
+}
+
+/**
+ * binder_genl_set_report() - set binder report flags
+ * @proc: the binder_proc calling the ioctl
+ * @pid: the target process
+ * @flags: the flags to set
+ *
+ * If pid is 0, the flags are applied to the whole binder context.
+ * Otherwise, the flags are applied to the specific process only.
+ */
+int binder_genl_set_report(struct binder_context *context, u32 pid, u32 flags)
+{
+ struct binder_proc *proc;
+
+ if (flags != (flags & (BINDER_REPORT_ALL | BINDER_REPORT_OVERRIDE))) {
+ pr_err("Invalid binder report flags: %u\n", flags);
+ return -EINVAL;
+ }
+
+ 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) {
+ pr_err("Invalid binder report pid %u\n", pid);
+ return -EINVAL;
+ }
+
+ proc->report_flags = flags;
+ }
+
+ return 0;
+}
+
+/**
+ * binder_genl_report_enabled() - check if binder genl reports are enabled
+ * @proc: the binder_proc to check
+ * @mask: the categories of binder genl reports
+ *
+ * Returns true if certain binder genl reports are enabled for this binder
+ * proc (when per-process overriding takes effect) or context.
+ */
+inline bool binder_genl_report_enabled(struct binder_proc *proc, u32 mask)
+{
+ struct binder_context *context = proc->context;
+
+ if (!context->report_portid)
+ return false;
+
+ if (proc->report_flags & BINDER_REPORT_OVERRIDE)
+ return (proc->report_flags & mask) != 0;
+ else
+ return (context->report_flags & mask) != 0;
+}
+
+/**
+ * binder_genl_send_report() - send one binder genl report
+ * @context: the binder context
+ * @report: the binder genl report to send
+ * @len: the length of the report data
+ *
+ * Packs the report data into a BINDER_GENL_ATTR_REPORT packet and send it.
+ */
+void binder_genl_send_report(struct binder_context *context,
+ struct binder_report *report, int len)
+{
+ int ret;
+ struct sk_buff *skb;
+ void *hdr;
+
+ trace_binder_send_report(context->name, report, len);
+
+ skb = genlmsg_new(nla_total_size(len), GFP_KERNEL);
+ if (!skb) {
+ pr_err("Failed to alloc binder genl message\n");
+ return;
+ }
+
+ hdr = genlmsg_put(skb, 0, atomic_inc_return(&context->report_seq),
+ &context->genl_family, 0, BINDER_GENL_CMD_REPORT);
+ if (!hdr) {
+ pr_err("Failed to set binder genl header\n");
+ kfree_skb(skb);
+ return;
+ }
+
+ if (nla_put(skb, BINDER_GENL_ATTR_REPORT, len, report)) {
+ genlmsg_cancel(skb, hdr);
+ nlmsg_free(skb);
+ return;
+ }
+
+ genlmsg_end(skb, hdr);
+
+ ret = genlmsg_unicast(&init_net, skb, context->report_portid);
+ if (ret < 0)
+ pr_err("Failed to send binder genl message to %d: %d\n",
+ context->report_portid, ret);
+}
diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
index f8d6be682f23..7264d8bedf90 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -12,15 +12,32 @@
#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
+ * @genl_family: the generic netlink family
+ * @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;
+ struct genl_family genl_family;
+ u32 report_portid;
+ u32 report_flags;
+ atomic_t report_seq;
};
/**
@@ -415,6 +432,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_report_flag).
*
* Bookkeeping structure for binder processes
*/
@@ -453,6 +472,7 @@ struct binder_proc {
spinlock_t outer_lock;
struct dentry *binderfs_entry;
bool oneway_spam_detection_enabled;
+ u32 report_flags;
};
/**
@@ -582,4 +602,15 @@ struct binder_object {
};
};
+struct binder_proc *binder_find_proc(int pid);
+
+int binder_genl_init(struct genl_family *family, const char *name);
+
+int binder_genl_set_report(struct binder_context *context, u32 pid, u32 flags);
+
+bool binder_genl_report_enabled(struct binder_proc *proc, u32 mask);
+
+void binder_genl_send_report(struct binder_context *context,
+ struct binder_report *report, int len);
+
#endif /* _LINUX_BINDER_INTERNAL_H */
diff --git a/drivers/android/binder_trace.h b/drivers/android/binder_trace.h
index fe38c6fc65d0..16e0a7efbe3a 100644
--- a/drivers/android/binder_trace.h
+++ b/drivers/android/binder_trace.h
@@ -423,6 +423,43 @@ TRACE_EVENT(binder_return,
"unknown")
);
+TRACE_EVENT(binder_send_report,
+ TP_PROTO(const char *name, struct binder_report *report, int len),
+ TP_ARGS(name, report, len),
+ TP_STRUCT__entry(
+ __field(const char *, name)
+ __field(uint32_t, err)
+ __field(uint32_t, from_pid)
+ __field(uint32_t, from_tid)
+ __field(uint32_t, to_pid)
+ __field(uint32_t, to_tid)
+ __field(uint32_t, reply)
+ __field(uint32_t, flags)
+ __field(uint32_t, code)
+ __field(binder_size_t, data_size)
+ __field(uint32_t, len)
+ ),
+ TP_fast_assign(
+ __entry->name = name;
+ __entry->err = report->err;
+ __entry->from_pid = report->from_pid;
+ __entry->from_tid = report->from_tid;
+ __entry->to_pid = report->to_pid;
+ __entry->to_tid = report->to_tid;
+ __entry->reply = report->reply;
+ __entry->flags = report->flags;
+ __entry->code = report->code;
+ __entry->data_size = report->data_size;
+ __entry->len = len;
+ ),
+ TP_printk("%s: %d %d:%d -> %d:%d %s flags=0x08%x code=%d %llu %d",
+ __entry->name, __entry->err, __entry->from_pid,
+ __entry->from_tid, __entry->to_pid, __entry->to_tid,
+ __entry->reply ? "reply" : "",
+ __entry->flags, __entry->code, __entry->data_size,
+ __entry->len)
+);
+
#endif /* _BINDER_TRACE_H */
#undef TRACE_INCLUDE_PATH
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index ad1fa7abc323..b2c5b04bf2ab 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -207,6 +207,10 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
fsnotify_create(root->d_inode, dentry);
inode_unlock(d_inode(root));
+ ret = binder_genl_init(&device->context.genl_family, name);
+ if (ret < 0)
+ goto err;
+
return 0;
err:
diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h
index 1fd92021a573..19c49a2a0301 100644
--- a/include/uapi/linux/android/binder.h
+++ b/include/uapi/linux/android/binder.h
@@ -588,5 +588,137 @@ enum binder_driver_command_protocol {
*/
};
+/*
+ * Name of binder generic netlink
+ */
+#define BINDER_GENL_FAMILY_NAME "binder"
+
+/*
+ * Version of binder generic netlink
+ */
+#define BINDER_GENL_VERSION 1
+
+/*
+ * Used with BINDER_ENABLE_REPORT, defining what kind of binder transactions
+ * should be reported to user space administration process.
+ */
+enum binder_report_flag {
+ /*
+ * Report failed binder transactions,
+ * when the sender gets BR_{FAILED|FROZEN|DEAD}_REPLY
+ */
+ BINDER_REPORT_FAILED = 0x1,
+
+ /*
+ * Report delayed async binder transactions due to frozen target,
+ * when the sender gets BR_TRANSACTION_PENDING_FROZEN.
+ */
+ BINDER_REPORT_DELAYED = 0x2,
+
+ /*
+ * Report spamming binder transactions,
+ * when the sender gets BR_ONEWAY_SPAM_SUSPECT.
+ */
+ BINDER_REPORT_SPAM = 0x4,
+
+ /*
+ * Report all of the above
+ */
+ BINDER_REPORT_ALL = BINDER_REPORT_FAILED
+ | BINDER_REPORT_DELAYED
+ | BINDER_REPORT_SPAM,
+
+ /*
+ * Enable the per-process flag, which overrides the global one.
+ */
+ BINDER_REPORT_OVERRIDE = 0x8000000,
+};
+
+/*
+ * struct binder_report - reports an abnormal binder transaction
+ * @err: copy of binder_driver_return_protocol returned to the sender
+ * @from_pid: sender pid of the corresponding binder transaction
+ * @from_tid: sender tid of the corresponding binder transaction
+ * @to_pid: target pid of the corresponding binder transaction
+ * @to_tid: target tid of the corresponding binder transaction
+ * @reply: 1 means the txn is a reply, 0 otherwise
+ * @flags: copy of binder_transaction_data->flags
+ * @code: copy of binder_transaction_data->code
+ * @data_size: copy of binder_transaction_data->data_size
+ *
+ * When a binder transaction fails to reach the target process or is not
+ * delivered on time, an error command BR_XXX is returned from the kernel
+ * binder driver to the user space sender. This is considered an abnormal
+ * binder transaction. The most important information about this abnormal
+ * binder transaction will be packed into this binder_report struct and sent
+ * to the registered user space administration process via generic netlink.
+ */
+struct binder_report {
+ __u32 err;
+ __u32 from_pid;
+ __u32 from_tid;
+ __u32 to_pid;
+ __u32 to_tid;
+ __u32 reply;
+ __u32 flags;
+ __u32 code;
+ binder_size_t data_size;
+};
+
+/*
+ * The supported attributes of binder generic netlink.
+ */
+enum binder_genl_attr {
+ BINDER_GENL_ATTR_UNSPEC = 0,
+
+ /*
+ * The attribute for BINDER_GENL_CMD_SET_REPORT and its REPLY.
+ * Contains payload u32 pid.
+ */
+ BINDER_GENL_ATTR_PID = 1,
+
+ /*
+ * The attribute for BINDER_GENL_CMD_SET_REPORT and its REPLY.
+ * Contains payload u32 report_flags;.
+ */
+ BINDER_GENL_ATTR_FLAGS = 2,
+
+ /*
+ * The attribute for BINDER_GENL_CMD_REPORT.
+ * Contains payload struct binder_report.
+ */
+ BINDER_GENL_ATTR_REPORT = 3,
+
+ BINDER_GENL_ATTR_MAX = BINDER_GENL_ATTR_REPORT,
+};
+
+/*
+ * The supported commands of binder generic netlink.
+ */
+enum binder_genl_cmd {
+ BINDER_GENL_CMD_UNSPEC = 0,
+
+ /*
+ * The user space administrator process uses this command to set what
+ * kinds of binder transactions are reported via generic netlink.
+ */
+ BINDER_GENL_CMD_SET_REPORT = 1,
+
+ /*
+ * After receiving BINDER_GENL_SET_REPORT from the user space
+ * administrator process, the kernel binder driver uses this command
+ * to acknowledge the request.
+ */
+ BINDER_GENL_CMD_REPLY = 2,
+
+ /*
+ * After enabling binder report, the kernel binder driver uses this
+ * command to send the requested reports to the user space.
+ */
+ BINDER_GENL_CMD_REPORT = 3,
+
+ BINDER_GENL_CMD_MAX = BINDER_GENL_CMD_REPORT,
+};
+
#endif /* _UAPI_LINUX_BINDER_H */
--
2.47.0.105.g07ac214952-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 1/1] report binder txn errors via generic netlink
2024-10-21 18:28 [PATCH v3 0/1] binder: report txn errors via generic netlink (genl) Li Li
2024-10-21 18:28 ` [PATCH v3 1/1] binder: report txn errors via generic netlink Li Li
@ 2024-10-21 18:28 ` Li Li
2024-10-26 8:04 ` Dan Carpenter
1 sibling, 1 reply; 8+ messages in thread
From: Li Li @ 2024-10-21 18:28 UTC (permalink / raw)
To: dualli, corbet, davem, edumazet, kuba, pabeni, donald.hunter,
gregkh, arve, tkjos, maco, joel, brauner, cmllamas, surenb, arnd,
masahiroy, linux-kernel, linux-doc, netdev, hridya, smoreland
Cc: kernel-team
From: Li Li <dualli@google.com>
Frozen tasks can't process binder transactions, so sync binder
transactions will fail with BR_FROZEN_REPLY and async binder
transactions will be queued in the kernel async binder buffer.
As these queued async transactions accumulates over time, the async
buffer will eventually be running out, denying all new transactions
after that with BR_FAILED_REPLY.
In addition to the above cases, different kinds of binder error codes
might be returned to the sender. However, the core Linux, or Android,
system administration process never knows what's actually happening.
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 transactions.
The new binder genl sources and headers are automatically generated from
the corresponding YAML spec. Don't modify them directly.
Signed-off-by: Li Li <dualli@google.com>
---
Documentation/admin-guide/binder_genl.rst | 92 ++++++
Documentation/admin-guide/index.rst | 1 +
Documentation/netlink/specs/binder_genl.yaml | 59 ++++
drivers/android/Kconfig | 1 +
drivers/android/Makefile | 2 +-
drivers/android/binder.c | 287 ++++++++++++++++++-
drivers/android/binder_genl.c | 38 +++
drivers/android/binder_genl.h | 18 ++
drivers/android/binder_internal.h | 22 ++
drivers/android/binder_trace.h | 37 +++
drivers/android/binderfs.c | 4 +
include/uapi/linux/android/binder.h | 31 ++
include/uapi/linux/android/binder_genl.h | 37 +++
13 files changed, 625 insertions(+), 4 deletions(-)
create mode 100644 Documentation/admin-guide/binder_genl.rst
create mode 100644 Documentation/netlink/specs/binder_genl.yaml
create mode 100644 drivers/android/binder_genl.c
create mode 100644 drivers/android/binder_genl.h
create mode 100644 include/uapi/linux/android/binder_genl.h
diff --git a/Documentation/admin-guide/binder_genl.rst b/Documentation/admin-guide/binder_genl.rst
new file mode 100644
index 000000000000..48a0ceab6552
--- /dev/null
+++ b/Documentation/admin-guide/binder_genl.rst
@@ -0,0 +1,92 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===========================================================
+Generic Netlink for the Android Binder Driver (Binder Genl)
+===========================================================
+
+The Generic Netlink subsystem in the Linux kernel provides a generic way for
+the Linux kernel to communicate to the user space applications. In the kernel
+binder driver, it is used to report various kinds of binder transactions to
+user space administration process. The binder driver allows multiple binder
+devices and their corresponding binder contexts. Each binder context has a
+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 use the "set" command to request what kinds
+of binder transactions should be reported by the kernel binder driver. The
+kernel binder driver use "reply" command to acknowledge the request. The
+"set" command also register the current user space process to receive the
+reports. When the user space process exits, the previous request will be
+reset to prevent any potential leaks.
+
+Currently the binder driver can report binder trasnactiosn that "failed"
+to reach the target process, or that are "delayed" due to the target process
+being frozen by cgroup freezer, or that are considered "spam" according to
+existing logic in binder_alloc.c.
+
+When the specified binder transactions happened, the binder driver uses the
+"report" command to send a generic netlink message to the registered process,
+containing the payload struct binder_report.
+
+More details about the flags, attributes and operations can be found at the
+the doc sections in Documentations/netlink/specs/binder_genl.yaml and the
+kernel-doc comments of the new source code in binder.{h|c}.
+
+Using Binder Genl
+-----------------
+
+The Binder Genl can be used in the same way as any other generic netlink
+drivers. The user space application uses a raw netlink socket to send commands
+to and receive packets from the kernel driver.
+
+NOTE: if the user applications that talks to the Binder Genl driver exits,
+the kernel driver will automatically reset the configuration to the default
+and stop sending more reports to prevent leaking memory.
+
+Usage example (user space pseudo code):
+
+::
+
+ // 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 genl
+ send(fd, CTRL_CMD_GETFAMILY, CTRL_ATTR_FAMILY_NAME, "binder");
+ void *data = recv(CTRL_CMD_NEWFAMILY);
+ __u16 id = nla(data)[CTRL_ATTR_FAMILY_ID];
+
+ // enable per-context binder report
+ send(fd, id, BINDER_GENL_CMD_SET, 0, BINDER_GENL_FLAG_FAILED |
+ BINDER_GENL_FLAG_DELAYED);
+
+ // confirm the per-context configuration
+ void *data = recv(fd, BINDER_GENL_CMD_REPLY);
+ __u32 pid = nla(data)[BINDER_GENL_ATTR_PID];
+ __u32 flags = nla(data)[BINDER_GENL_ATTR_FLAGS];
+
+ // set optional per-process report, overriding the per-context one
+ send(fd, id, BINDER_GENL_CMD_SET, getpid(),
+ BINDER_GENL_FLAG_SPAM | BINDER_REPORT_OVERRIDE);
+
+ // confirm the optional per-process configuration
+ void *data = recv(fd, BINDER_GENL_CMD_REPLY);
+ __u32 pid = nla(data)[BINDER_GENL_A_ATTR_PID];
+ __u32 flags = nla(data)[BINDER_GENL_A_ATTR_FLAGS];
+
+ // wait and read all binder reports
+ while (running) {
+ void *data = recv(fd, BINDER_GENL_CMD_REPORT);
+ struct binder_report report = nla(data)[BINDER_GENL_A_ATTR_REPORT];
+
+ // process struct binder_report
+ do_something(&report);
+ }
+
+ // clean up
+ send(fd, id, BINDER_GENL_CMD_SET, 0, 0);
+ send(fd, id, BINDER_GENL_CMD_SET, 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_genl.yaml b/Documentation/netlink/specs/binder_genl.yaml
new file mode 100644
index 000000000000..8479580a2856
--- /dev/null
+++ b/Documentation/netlink/specs/binder_genl.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+
+name: binder_genl
+protocol: genetlink
+
+doc: Netlink protocol to report binder transaction errors and warnings.
+
+uapi-header: linux/android/binder_genl.h
+
+definitions:
+ -
+ type: flags
+ name: flag
+ doc: Used with "set" and "reply" command below, defining what kind \
+ of binder transactions should be reported to the user space \
+ administration process.
+ value-start: 0
+ entries: [ failed, delayed, spam, override ]
+
+attribute-sets:
+ -
+ name: attr
+ doc: The supported attributes of binder genl.
+ attributes:
+ -
+ name: pid
+ type: u32
+ doc: Used by "set" and "reply" command below, indicating the \
+ binder proc or context to set the flags.
+ -
+ name: flags
+ type: u32
+ doc: Used by "set" and "reply" command below, indicating the \
+ flags to set.
+ -
+ name: report
+ type: binary
+ doc: Used by "report" command below, containing struct binder_report.
+
+operations:
+ list:
+ -
+ name: set
+ doc: Set flags from user space, requesting what kinds of binder \
+ transactions to report.
+ attribute-set: attr
+
+ do:
+ request: ¶ms
+ attributes:
+ - pid
+ - flags
+ reply: *params
+ -
+ name: reply
+ doc: Acknowledge the above "set" request, echoing the same params.
+ -
+ name: report
+ doc: Send the requested binder transaction reports to user space.
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..d818447fbc4c 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_genl.o
obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 978740537a1a..26941338c965 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -72,6 +72,7 @@
#include <linux/cacheflush.h>
+#include "binder_genl.h"
#include "binder_internal.h"
#include "binder_trace.h"
@@ -2984,6 +2985,130 @@ 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_genl_set_report() - set binder report flags
+ * @proc: the binder_proc calling the ioctl
+ * @pid: the target process
+ * @flags: the flags to set
+ *
+ * If pid is 0, the flags are applied to the whole binder context.
+ * Otherwise, the flags are applied to the specific process only.
+ */
+static int binder_genl_set_report(struct binder_context *context, u32 pid, u32 flags)
+{
+ struct binder_proc *proc;
+
+ if (flags != (flags & (BINDER_GENL_FLAG_OVERRIDE
+ | BINDER_GENL_FLAG_FAILED
+ | BINDER_GENL_FLAG_DELAYED
+ | BINDER_GENL_FLAG_SPAM))) {
+ pr_err("Invalid binder report flags: %u\n", flags);
+ return -EINVAL;
+ }
+
+ 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) {
+ pr_err("Invalid binder report pid %u\n", pid);
+ return -EINVAL;
+ }
+
+ proc->report_flags = flags;
+ }
+
+ return 0;
+}
+
+/**
+ * binder_genl_report_enabled() - check if binder genl reports are enabled
+ * @proc: the binder_proc to check
+ * @mask: the categories of binder genl reports
+ *
+ * Returns true if certain binder genl reports are enabled for this binder
+ * proc (when per-process overriding takes effect) or context.
+ */
+static bool binder_genl_report_enabled(struct binder_proc *proc, u32 mask)
+{
+ struct binder_context *context = proc->context;
+
+ if (!context->report_portid)
+ return false;
+
+ if (proc->report_flags & BINDER_GENL_FLAG_OVERRIDE)
+ return (proc->report_flags & mask) != 0;
+ else
+ return (context->report_flags & mask) != 0;
+}
+
+/**
+ * binder_genl_send_report() - send one binder genl report
+ * @context: the binder context
+ * @report: the binder genl report to send
+ * @len: the length of the report data
+ *
+ * Packs the report data into a BINDER_GENL_A_ATTR_REPORT packet and send it.
+ */
+static void binder_genl_send_report(struct binder_context *context,
+ struct binder_report *report, int len)
+{
+ int ret;
+ struct sk_buff *skb;
+ void *hdr;
+
+ trace_binder_send_report(context->name, report, len);
+
+ skb = genlmsg_new(nla_total_size(len), GFP_KERNEL);
+ if (!skb) {
+ pr_err("Failed to alloc binder genl message\n");
+ return;
+ }
+
+ hdr = genlmsg_put(skb, 0, atomic_inc_return(&context->report_seq),
+ &context->genl_family, 0, BINDER_GENL_CMD_REPORT);
+ if (!hdr) {
+ pr_err("Failed to set binder genl header\n");
+ kfree_skb(skb);
+ return;
+ }
+
+ if (nla_put(skb, BINDER_GENL_A_ATTR_REPORT, len, report)) {
+ genlmsg_cancel(skb, hdr);
+ nlmsg_free(skb);
+ return;
+ }
+
+ genlmsg_end(skb, hdr);
+
+ ret = genlmsg_unicast(&init_net, skb, context->report_portid);
+ if (ret < 0)
+ pr_err("Failed to send binder genl message to %d: %d\n",
+ context->report_portid, ret);
+}
+
static void binder_transaction(struct binder_proc *proc,
struct binder_thread *thread,
struct binder_transaction_data *tr, int reply,
@@ -3678,10 +3803,25 @@ 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_genl_report_enabled(proc, BINDER_GENL_FLAG_SPAM)) {
+ struct binder_report report;
+
+ report.err = BR_ONEWAY_SPAM_SUSPECT;
+ report.from_pid = proc->pid;
+ report.from_tid = thread->pid;
+ report.to_pid = target_proc ? target_proc->pid : 0;
+ report.to_tid = target_thread ? target_thread->pid : 0;
+ report.reply = reply;
+ report.flags = tr->flags;
+ report.code = tr->code;
+ report.data_size = tr->data_size;
+ binder_genl_send_report(context, &report, sizeof(report));
+ }
+ } else {
tcomplete->type = BINDER_WORK_TRANSACTION_COMPLETE;
+ }
t->work.type = BINDER_WORK_TRANSACTION;
if (reply) {
@@ -3731,8 +3871,24 @@ 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_genl_report_enabled(proc, BINDER_GENL_FLAG_DELAYED)) {
+ struct binder_report report;
+
+ report.err = return_error;
+ report.from_pid = proc->pid;
+ report.from_tid = thread->pid;
+ report.to_pid = target_proc ? target_proc->pid : 0;
+ report.to_tid = target_thread ? target_thread->pid : 0;
+ report.reply = reply;
+ report.flags = tr->flags;
+ report.code = tr->code;
+ report.data_size = tr->data_size;
+ binder_genl_send_report(context, &report,
+ sizeof(report));
+ }
+ }
binder_enqueue_thread_work(thread, tcomplete);
if (return_error &&
return_error != BR_TRANSACTION_PENDING_FROZEN)
@@ -3794,6 +3950,21 @@ static void binder_transaction(struct binder_proc *proc,
binder_dec_node_tmpref(target_node);
}
+ if (binder_genl_report_enabled(proc, BINDER_GENL_FLAG_FAILED)) {
+ struct binder_report report;
+
+ report.err = return_error;
+ report.from_pid = proc->pid;
+ report.from_tid = thread->pid;
+ report.to_pid = target_proc ? target_proc->pid : 0;
+ report.to_tid = target_thread ? target_thread->pid : 0;
+ report.reply = reply;
+ report.flags = tr->flags;
+ report.code = tr->code;
+ report.data_size = tr->data_size;
+ binder_genl_send_report(context, &report, sizeof(report));
+ }
+
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" :
@@ -6114,6 +6285,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;
}
@@ -6311,6 +6487,84 @@ binder_defer_work(struct binder_proc *proc, enum binder_deferred_state defer)
mutex_unlock(&binder_deferred_lock);
}
+/**
+ * binder_genl_nl_set_doit() - .doit handler for BINDER_GENL_CMD_SET
+ * @skb: the metadata struct passed from netlink driver
+ * @info: the generic netlink struct passed from netlink driver
+ *
+ * Implements the .doit function to process binder genl commands.
+ */
+int binder_genl_nl_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ int len;
+ int portid;
+ u32 pid;
+ u32 flags;
+ void *hdr;
+ struct binder_context *context;
+
+ /* Both attributes are required for BINDER_GENL_CMD_SET */
+ if (!info->attrs[BINDER_GENL_A_ATTR_PID] || !info->attrs[BINDER_GENL_A_ATTR_FLAGS]) {
+ pr_err("Attributes not set\n");
+ return -EINVAL;
+ }
+
+ portid = nlmsg_hdr(skb)->nlmsg_pid;
+ pid = nla_get_u32(info->attrs[BINDER_GENL_A_ATTR_PID]);
+ flags = nla_get_u32(info->attrs[BINDER_GENL_A_ATTR_FLAGS]);
+ context = container_of(info->family, struct binder_context,
+ genl_family);
+
+ if (context->report_portid && context->report_portid != portid) {
+ pr_err("No permission to set report flags from %u\n", portid);
+ return -EPERM;
+ }
+
+ if (binder_genl_set_report(context, pid, flags) < 0) {
+ pr_err("Failed to set report flags %u for %u\n", flags, pid);
+ return -EINVAL;
+ }
+
+ len = nla_total_size(sizeof(pid)) + nla_total_size(sizeof(flags));
+ skb = genlmsg_new(len, GFP_KERNEL);
+ if (!skb) {
+ pr_err("Failed to alloc binder genl reply message\n");
+ return -ENOMEM;
+ }
+
+ hdr = genlmsg_put_reply(skb, info, info->family, 0,
+ BINDER_GENL_CMD_REPLY);
+ if (!hdr)
+ goto free_skb;
+
+ if (nla_put_u32(skb, BINDER_GENL_A_ATTR_PID, pid))
+ goto cancel_skb;
+
+ if (nla_put_u32(skb, BINDER_GENL_A_ATTR_FLAGS, flags))
+ goto cancel_skb;
+
+ genlmsg_end(skb, hdr);
+
+ if (genlmsg_reply(skb, info)) {
+ pr_err("Failed to send binder genl reply message\n");
+ return -EFAULT;
+ }
+
+ if (!context->report_portid)
+ context->report_portid = portid;
+
+ return 0;
+
+cancel_skb:
+ pr_err("Failed to add genl header to reply message\n");
+ genlmsg_cancel(skb, hdr);
+
+free_skb:
+ pr_err("Failed to add genl attribute to reply message\n");
+ nlmsg_free(skb);
+ return -EMSGSIZE;
+}
+
static void print_binder_transaction_ilocked(struct seq_file *m,
struct binder_proc *proc,
const char *prefix,
@@ -6894,6 +7148,28 @@ const struct binder_debugfs_entry binder_debugfs_entries[] = {
{} /* terminator */
};
+/**
+ * binder_genl_init() - initialize binder generic netlink
+ * @family: the generic netlink family
+ * @name: the binder device name
+ *
+ * Registers the binder generic netlink family.
+ */
+int binder_genl_init(struct genl_family *family, const char *name)
+{
+ int ret;
+
+ memcpy(family, &binder_genl_nl_family, sizeof(*family));
+ strscpy(family->name, name, GENL_NAMSIZ);
+ ret = genl_register_family(family);
+ if (ret) {
+ pr_err("Failed to register binder genl: %s\n", name);
+ return ret;
+ }
+
+ return 0;
+}
+
static int __init init_binder_device(const char *name)
{
int ret;
@@ -6920,6 +7196,11 @@ static int __init init_binder_device(const char *name)
hlist_add_head(&binder_device->hlist, &binder_devices);
+ binder_device->context.report_seq = (atomic_t)ATOMIC_INIT(0);
+ ret = binder_genl_init(&binder_device->context.genl_family, name);
+ if (ret < 0)
+ kfree(binder_device);
+
return ret;
}
diff --git a/drivers/android/binder_genl.c b/drivers/android/binder_genl.c
new file mode 100644
index 000000000000..c9b9bc5cdfb6
--- /dev/null
+++ b/drivers/android/binder_genl.c
@@ -0,0 +1,38 @@
+// 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_genl.yaml */
+/* YNL-GEN kernel source */
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include "binder_genl.h"
+
+#include <uapi/linux/android/binder_genl.h>
+
+/* BINDER_GENL_CMD_SET - do */
+static const struct nla_policy binder_genl_set_nl_policy[BINDER_GENL_A_ATTR_FLAGS + 1] = {
+ [BINDER_GENL_A_ATTR_PID] = { .type = NLA_U32, },
+ [BINDER_GENL_A_ATTR_FLAGS] = { .type = NLA_U32, },
+};
+
+/* Ops table for binder_genl */
+static const struct genl_split_ops binder_genl_nl_ops[] = {
+ {
+ .cmd = BINDER_GENL_CMD_SET,
+ .doit = binder_genl_nl_set_doit,
+ .policy = binder_genl_set_nl_policy,
+ .maxattr = BINDER_GENL_A_ATTR_FLAGS,
+ .flags = GENL_CMD_CAP_DO,
+ },
+};
+
+struct genl_family binder_genl_nl_family __ro_after_init = {
+ .name = BINDER_GENL_FAMILY_NAME,
+ .version = BINDER_GENL_FAMILY_VERSION,
+ .netnsok = true,
+ .parallel_ops = true,
+ .module = THIS_MODULE,
+ .split_ops = binder_genl_nl_ops,
+ .n_split_ops = ARRAY_SIZE(binder_genl_nl_ops),
+};
diff --git a/drivers/android/binder_genl.h b/drivers/android/binder_genl.h
new file mode 100644
index 000000000000..9d68c155b7c4
--- /dev/null
+++ b/drivers/android/binder_genl.h
@@ -0,0 +1,18 @@
+/* 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_genl.yaml */
+/* YNL-GEN kernel header */
+
+#ifndef _LINUX_BINDER_GENL_GEN_H
+#define _LINUX_BINDER_GENL_GEN_H
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include <uapi/linux/android/binder_genl.h>
+
+int binder_genl_nl_set_doit(struct sk_buff *skb, struct genl_info *info);
+
+extern struct genl_family binder_genl_nl_family;
+
+#endif /* _LINUX_BINDER_GENL_GEN_H */
diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
index f8d6be682f23..4abc8e3940c3 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -12,15 +12,32 @@
#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
+ * @genl_family: the generic netlink family
+ * @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;
+ struct genl_family genl_family;
+ u32 report_portid;
+ u32 report_flags;
+ atomic_t report_seq;
};
/**
@@ -415,6 +432,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_report_flag).
*
* Bookkeeping structure for binder processes
*/
@@ -453,6 +472,7 @@ struct binder_proc {
spinlock_t outer_lock;
struct dentry *binderfs_entry;
bool oneway_spam_detection_enabled;
+ u32 report_flags;
};
/**
@@ -582,4 +602,6 @@ struct binder_object {
};
};
+int binder_genl_init(struct genl_family *family, const char *name);
+
#endif /* _LINUX_BINDER_INTERNAL_H */
diff --git a/drivers/android/binder_trace.h b/drivers/android/binder_trace.h
index fe38c6fc65d0..16e0a7efbe3a 100644
--- a/drivers/android/binder_trace.h
+++ b/drivers/android/binder_trace.h
@@ -423,6 +423,43 @@ TRACE_EVENT(binder_return,
"unknown")
);
+TRACE_EVENT(binder_send_report,
+ TP_PROTO(const char *name, struct binder_report *report, int len),
+ TP_ARGS(name, report, len),
+ TP_STRUCT__entry(
+ __field(const char *, name)
+ __field(uint32_t, err)
+ __field(uint32_t, from_pid)
+ __field(uint32_t, from_tid)
+ __field(uint32_t, to_pid)
+ __field(uint32_t, to_tid)
+ __field(uint32_t, reply)
+ __field(uint32_t, flags)
+ __field(uint32_t, code)
+ __field(binder_size_t, data_size)
+ __field(uint32_t, len)
+ ),
+ TP_fast_assign(
+ __entry->name = name;
+ __entry->err = report->err;
+ __entry->from_pid = report->from_pid;
+ __entry->from_tid = report->from_tid;
+ __entry->to_pid = report->to_pid;
+ __entry->to_tid = report->to_tid;
+ __entry->reply = report->reply;
+ __entry->flags = report->flags;
+ __entry->code = report->code;
+ __entry->data_size = report->data_size;
+ __entry->len = len;
+ ),
+ TP_printk("%s: %d %d:%d -> %d:%d %s flags=0x08%x code=%d %llu %d",
+ __entry->name, __entry->err, __entry->from_pid,
+ __entry->from_tid, __entry->to_pid, __entry->to_tid,
+ __entry->reply ? "reply" : "",
+ __entry->flags, __entry->code, __entry->data_size,
+ __entry->len)
+);
+
#endif /* _BINDER_TRACE_H */
#undef TRACE_INCLUDE_PATH
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index ad1fa7abc323..b2c5b04bf2ab 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -207,6 +207,10 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
fsnotify_create(root->d_inode, dentry);
inode_unlock(d_inode(root));
+ ret = binder_genl_init(&device->context.genl_family, name);
+ if (ret < 0)
+ goto err;
+
return 0;
err:
diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h
index 1fd92021a573..d95776448db9 100644
--- a/include/uapi/linux/android/binder.h
+++ b/include/uapi/linux/android/binder.h
@@ -588,5 +588,36 @@ enum binder_driver_command_protocol {
*/
};
+/*
+ * struct binder_report - reports an abnormal binder transaction
+ * @err: copy of binder_driver_return_protocol returned to the sender
+ * @from_pid: sender pid of the corresponding binder transaction
+ * @from_tid: sender tid of the corresponding binder transaction
+ * @to_pid: target pid of the corresponding binder transaction
+ * @to_tid: target tid of the corresponding binder transaction
+ * @reply: 1 means the txn is a reply, 0 otherwise
+ * @flags: copy of binder_transaction_data->flags
+ * @code: copy of binder_transaction_data->code
+ * @data_size: copy of binder_transaction_data->data_size
+ *
+ * When a binder transaction fails to reach the target process or is not
+ * delivered on time, an error command BR_XXX is returned from the kernel
+ * binder driver to the user space sender. This is considered an abnormal
+ * binder transaction. The most important information about this abnormal
+ * binder transaction will be packed into this binder_report struct and sent
+ * to the registered user space administration process via generic netlink.
+ */
+struct binder_report {
+ __u32 err;
+ __u32 from_pid;
+ __u32 from_tid;
+ __u32 to_pid;
+ __u32 to_tid;
+ __u32 reply;
+ __u32 flags;
+ __u32 code;
+ binder_size_t data_size;
+};
+
#endif /* _UAPI_LINUX_BINDER_H */
diff --git a/include/uapi/linux/android/binder_genl.h b/include/uapi/linux/android/binder_genl.h
new file mode 100644
index 000000000000..ef5289133be5
--- /dev/null
+++ b/include/uapi/linux/android/binder_genl.h
@@ -0,0 +1,37 @@
+/* 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_genl.yaml */
+/* YNL-GEN uapi header */
+
+#ifndef _UAPI_LINUX_BINDER_GENL_H
+#define _UAPI_LINUX_BINDER_GENL_H
+
+#define BINDER_GENL_FAMILY_NAME "binder_genl"
+#define BINDER_GENL_FAMILY_VERSION 1
+
+enum binder_genl_flag {
+ BINDER_GENL_FLAG_FAILED = 1,
+ BINDER_GENL_FLAG_DELAYED = 2,
+ BINDER_GENL_FLAG_SPAM = 4,
+ BINDER_GENL_FLAG_OVERRIDE = 8,
+};
+
+enum {
+ BINDER_GENL_A_ATTR_PID = 1,
+ BINDER_GENL_A_ATTR_FLAGS,
+ BINDER_GENL_A_ATTR_REPORT,
+
+ __BINDER_GENL_A_ATTR_MAX,
+ BINDER_GENL_A_ATTR_MAX = (__BINDER_GENL_A_ATTR_MAX - 1)
+};
+
+enum {
+ BINDER_GENL_CMD_SET = 1,
+ BINDER_GENL_CMD_REPLY,
+ BINDER_GENL_CMD_REPORT,
+
+ __BINDER_GENL_CMD_MAX,
+ BINDER_GENL_CMD_MAX = (__BINDER_GENL_CMD_MAX - 1)
+};
+
+#endif /* _UAPI_LINUX_BINDER_GENL_H */
--
2.47.0.105.g07ac214952-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] binder: report txn errors via generic netlink
2024-10-21 18:28 ` [PATCH v3 1/1] binder: report txn errors via generic netlink Li Li
@ 2024-10-21 18:35 ` Li Li
2024-10-21 18:56 ` Greg KH
2024-10-23 16:47 ` kernel test robot
1 sibling, 1 reply; 8+ messages in thread
From: Li Li @ 2024-10-21 18:35 UTC (permalink / raw)
To: dualli, corbet, davem, edumazet, kuba, pabeni, donald.hunter,
gregkh, arve, tkjos, maco, joel, brauner, cmllamas, surenb, arnd,
masahiroy, linux-kernel, linux-doc, netdev, hridya, smoreland
Sorry, please ignore this outdated and duplicated patch [1/1]. The
correct one is
https://lore.kernel.org/lkml/20241021182821.1259487-1-dualli@chromium.org/T/#m5f8d7ed4333ab4dc7f08932c01bb413e540e007a
On Mon, Oct 21, 2024 at 11:28 AM Li Li <dualli@chromium.org> wrote:
>
> From: Li Li <dualli@google.com>
>
> --
> 2.47.0.105.g07ac214952-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] binder: report txn errors via generic netlink
2024-10-21 18:35 ` Li Li
@ 2024-10-21 18:56 ` Greg KH
2024-10-21 19:23 ` Li Li
0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2024-10-21 18:56 UTC (permalink / raw)
To: Li Li
Cc: corbet, davem, edumazet, kuba, pabeni, donald.hunter, arve, tkjos,
maco, joel, brauner, cmllamas, surenb, arnd, masahiroy,
linux-kernel, linux-doc, netdev, hridya, smoreland
On Mon, Oct 21, 2024 at 11:35:57AM -0700, Li Li wrote:
> Sorry, please ignore this outdated and duplicated patch [1/1]. The
> correct one is
>
> https://lore.kernel.org/lkml/20241021182821.1259487-1-dualli@chromium.org/T/#m5f8d7ed4333ab4dc7f08932c01bb413e540e007a
Please send a v4 in a day or so when it's fixed up, as our tools can't
figure this out (and neither can I manually...)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] binder: report txn errors via generic netlink
2024-10-21 18:56 ` Greg KH
@ 2024-10-21 19:23 ` Li Li
0 siblings, 0 replies; 8+ messages in thread
From: Li Li @ 2024-10-21 19:23 UTC (permalink / raw)
To: Greg KH
Cc: corbet, davem, edumazet, kuba, pabeni, donald.hunter, arve, tkjos,
maco, joel, brauner, cmllamas, surenb, arnd, masahiroy,
linux-kernel, linux-doc, netdev, hridya, smoreland
On Mon, Oct 21, 2024 at 11:56 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Oct 21, 2024 at 11:35:57AM -0700, Li Li wrote:
> > Sorry, please ignore this outdated and duplicated patch [1/1]. The
> > correct one is
> >
> > https://lore.kernel.org/lkml/20241021182821.1259487-1-dualli@chromium.org/T/#m5f8d7ed4333ab4dc7f08932c01bb413e540e007a
>
> Please send a v4 in a day or so when it's fixed up, as our tools can't
> figure this out (and neither can I manually...)
>
Fixed and sent v4 (link below). Thank you very much!
https://lore.kernel.org/lkml/20241021191233.1334897-1-dualli@chromium.org/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] binder: report txn errors via generic netlink
2024-10-21 18:28 ` [PATCH v3 1/1] binder: report txn errors via generic netlink Li Li
2024-10-21 18:35 ` Li Li
@ 2024-10-23 16:47 ` kernel test robot
1 sibling, 0 replies; 8+ messages in thread
From: kernel test robot @ 2024-10-23 16:47 UTC (permalink / raw)
To: Li Li, dualli, corbet, davem, edumazet, kuba, pabeni,
donald.hunter, gregkh, arve, tkjos, maco, joel, brauner, cmllamas,
surenb, arnd, masahiroy, linux-kernel, linux-doc, netdev, hridya,
smoreland
Cc: oe-kbuild-all, kernel-team
Hi Li,
kernel test robot noticed the following build warnings:
[auto build test WARNING on staging/staging-testing]
[also build test WARNING on staging/staging-next staging/staging-linus linus/master v6.12-rc4 next-20241023]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Li-Li/binder-report-txn-errors-via-generic-netlink/20241022-022923
base: staging/staging-testing
patch link: https://lore.kernel.org/r/20241021182821.1259487-2-dualli%40chromium.org
patch subject: [PATCH v3 1/1] binder: report txn errors via generic netlink
config: arc-allmodconfig (https://download.01.org/0day-ci/archive/20241024/202410240012.MJJTBFCx-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241024/202410240012.MJJTBFCx-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410240012.MJJTBFCx-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/android/binder_genl.c:160: warning: Function parameter or struct member 'context' not described in 'binder_genl_set_report'
>> drivers/android/binder_genl.c:160: warning: Excess function parameter 'proc' description in 'binder_genl_set_report'
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for GET_FREE_REGION
Depends on [n]: SPARSEMEM [=n]
Selected by [m]:
- RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
vim +160 drivers/android/binder_genl.c
149
150 /**
151 * binder_genl_set_report() - set binder report flags
152 * @proc: the binder_proc calling the ioctl
153 * @pid: the target process
154 * @flags: the flags to set
155 *
156 * If pid is 0, the flags are applied to the whole binder context.
157 * Otherwise, the flags are applied to the specific process only.
158 */
159 int binder_genl_set_report(struct binder_context *context, u32 pid, u32 flags)
> 160 {
161 struct binder_proc *proc;
162
163 if (flags != (flags & (BINDER_REPORT_ALL | BINDER_REPORT_OVERRIDE))) {
164 pr_err("Invalid binder report flags: %u\n", flags);
165 return -EINVAL;
166 }
167
168 if (!pid) {
169 /* Set the global flags for the whole binder context */
170 context->report_flags = flags;
171 } else {
172 /* Set the per-process flags */
173 proc = binder_find_proc(pid);
174 if (!proc) {
175 pr_err("Invalid binder report pid %u\n", pid);
176 return -EINVAL;
177 }
178
179 proc->report_flags = flags;
180 }
181
182 return 0;
183 }
184
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] report binder txn errors via generic netlink
2024-10-21 18:28 ` [PATCH v3 1/1] report binder " Li Li
@ 2024-10-26 8:04 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2024-10-26 8:04 UTC (permalink / raw)
To: oe-kbuild, Li Li, dualli, corbet, davem, edumazet, kuba, pabeni,
donald.hunter, gregkh, arve, tkjos, maco, joel, brauner, cmllamas,
surenb, arnd, masahiroy, linux-kernel, linux-doc, netdev, hridya,
smoreland
Cc: lkp, oe-kbuild-all, kernel-team
Hi Li,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Li-Li/report-binder-txn-errors-via-generic-netlink/20241022-032903
base: staging/staging-testing
patch link: https://lore.kernel.org/r/20241021182821.1259487-3-dualli%40chromium.org
patch subject: [PATCH v3 1/1] report binder txn errors via generic netlink
config: x86_64-randconfig-161-20241024 (https://download.01.org/0day-ci/archive/20241026/202410260847.NDByoPQo-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202410260847.NDByoPQo-lkp@intel.com/
New smatch warnings:
drivers/android/binder.c:7204 init_binder_device() warn: '&binder_device->miscdev' from misc_register() not released on lines: 7204.
vim +7204 drivers/android/binder.c
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7173 static int __init init_binder_device(const char *name)
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7174 {
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7175 int ret;
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7176 struct binder_device *binder_device;
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7177
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7178 binder_device = kzalloc(sizeof(*binder_device), GFP_KERNEL);
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7179 if (!binder_device)
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7180 return -ENOMEM;
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7181
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7182 binder_device->miscdev.fops = &binder_fops;
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7183 binder_device->miscdev.minor = MISC_DYNAMIC_MINOR;
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7184 binder_device->miscdev.name = name;
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7185
f0fe2c0f050d31 Christian Brauner 2020-03-03 7186 refcount_set(&binder_device->ref, 1);
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7187 binder_device->context.binder_context_mgr_uid = INVALID_UID;
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7188 binder_device->context.name = name;
c44b1231ff1170 Todd Kjos 2017-06-29 7189 mutex_init(&binder_device->context.context_mgr_node_lock);
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7190
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7191 ret = misc_register(&binder_device->miscdev);
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7192 if (ret < 0) {
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7193 kfree(binder_device);
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7194 return ret;
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7195 }
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7196
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7197 hlist_add_head(&binder_device->hlist, &binder_devices);
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7198
39854ac9458c74 Li Li 2024-10-21 7199 binder_device->context.report_seq = (atomic_t)ATOMIC_INIT(0);
39854ac9458c74 Li Li 2024-10-21 7200 ret = binder_genl_init(&binder_device->context.genl_family, name);
39854ac9458c74 Li Li 2024-10-21 7201 if (ret < 0)
Needs a misc_unregister() as well to prevent a use after free.
39854ac9458c74 Li Li 2024-10-21 7202 kfree(binder_device);
39854ac9458c74 Li Li 2024-10-21 7203
ac4812c5ffbb88 Martijn Coenen 2017-02-03 @7204 return ret;
ac4812c5ffbb88 Martijn Coenen 2017-02-03 7205 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-10-26 8:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-21 18:28 [PATCH v3 0/1] binder: report txn errors via generic netlink (genl) Li Li
2024-10-21 18:28 ` [PATCH v3 1/1] binder: report txn errors via generic netlink Li Li
2024-10-21 18:35 ` Li Li
2024-10-21 18:56 ` Greg KH
2024-10-21 19:23 ` Li Li
2024-10-23 16:47 ` kernel test robot
2024-10-21 18:28 ` [PATCH v3 1/1] report binder " Li Li
2024-10-26 8:04 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).