From b0860c1df2dc3123171146ac5f1e3321289835b4 Mon Sep 17 00:00:00 2001 From: Yiyang Chen Date: Sat, 11 Jul 2026 04:07:36 +0800 Subject: taskstats: return -EBADF when cgroupstats receives an invalid fd Patch series "taskstats: fix cgroupstats invalid fd handling and add selftests". This series fixes an issue where cgroupstats mishandles invalid file descriptors, and introduces a functional kselftest to prevent regressions. When an invalid file descriptor is passed via CGROUPSTATS_CMD_ATTR_FD, cgroupstats_user_cmd() returns 0 instead of an error code. This leads to two broken behaviors depending on netlink flags: - Callers without NLM_F_ACK block indefinitely on recv(). - Callers with NLM_F_ACK receive a misleading success ACK (errno == 0) but no actual statistics payload. The first patch addresses this by returning -EBADF when the fd cannot be resolved. The second patch adds a comprehensive kselftest covering both the valid cgroup v1 query and the invalid fd paths (with and without NLM_F_ACK) to ensure the fixes work as intended. This patch (of 2): cgroupstats_user_cmd() returns 0 without sending a reply or a netlink error when the fd passed via CGROUPSTATS_CMD_ATTR_FD does not resolve to an open file in the caller's table. As a result: - clients that did not set NLM_F_ACK block on recv() indefinitely waiting for a CGROUPSTATS_CMD_NEW message that is never emitted; - clients that set NLM_F_ACK receive a misleading "success" ACK (errno == 0) with no statistics payload. Return -EBADF instead so the netlink layer propagates the error to userspace as expected. Link: https://lore.kernel.org/cover.1783713230.git.cyyzero16@gmail.com Link: https://lore.kernel.org/b4fd9e288e4a48efebaf41b4ffcdb204b06675c4.1783713230.git.cyyzero16@gmail.com Signed-off-by: Yiyang Chen Acked-by: Balbir Singh Cc: Dr. Thomas Orgis Signed-off-by: Andrew Morton --- kernel/taskstats.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'kernel/taskstats.c') diff --git a/kernel/taskstats.c b/kernel/taskstats.c index 2cd0172d0516..8d115670a3ca 100644 --- a/kernel/taskstats.c +++ b/kernel/taskstats.c @@ -423,7 +423,7 @@ static int cgroupstats_user_cmd(struct sk_buff *skb, struct genl_info *info) fd = nla_get_u32(info->attrs[CGROUPSTATS_CMD_ATTR_FD]); CLASS(fd, f)(fd); if (fd_empty(f)) - return 0; + return -EBADF; size = nla_total_size(sizeof(struct cgroupstats)); -- cgit v1.2.3 From 1f58a5335cdd14b3fb5f2a5d3763dee1f5cba1d3 Mon Sep 17 00:00:00 2001 From: Bradley Morgan Date: Thu, 23 Jul 2026 21:09:22 +0000 Subject: taskstats: fix cpumask parsing cutting off the last character parse() hands nla_strscpy() len as dstsize, and nla_strscpy() copies at most dstsize - 1 bytes. When the attr payload comes in without a trailing NUL, srclen == len >= dstsize and the last character of the cpumask string gets cut off. Register "0-15" and you are silently listening on "0-1", exit data for the rest never shows up. The bug only bites when the sender doesn't NUL terminate the payload; senders that include the NUL were always fine (srclen gets decremented for the trailing NUL, so srclen < dstsize). Thats probably why this survived 20 years. And the policy is NLA_STRING, not NLA_NUL_STRING, so a payload without the trailing NUL is legit input here. Skip the kmalloc/nla_strscpy dance entirely and use nla_strdup(), which already allocates srclen + 1 and terminates. The nla_len() bounds checks stay as they were. Link: https://lore.kernel.org/EC49FE41-7F5F-41E0-A07A-ABEB8ECA514D@grrlz.net Fixes: f9fd8914c1ac ("[PATCH] per-task delay accounting taskstats interface: control exit data through cpumasks") Signed-off-by: Bradley Morgan Reported-by: Oleg Deomi Closes: https://lore.kernel.org/CAByWkfZ6b1=3H9pwkz-dDQOs9cZaF-HYQ6b9Yb0=Hq2r1Vv_Pw@mail.gmail.com Reviewed-by: Andrew Morton Cc: Balbir Singh Cc: Signed-off-by: Andrew Morton --- kernel/taskstats.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'kernel/taskstats.c') diff --git a/kernel/taskstats.c b/kernel/taskstats.c index 8d115670a3ca..74f89b632c0a 100644 --- a/kernel/taskstats.c +++ b/kernel/taskstats.c @@ -368,10 +368,9 @@ static int parse(struct nlattr *na, struct cpumask *mask) return -E2BIG; if (len < 1) return -EINVAL; - data = kmalloc(len, GFP_KERNEL); + data = nla_strdup(na, GFP_KERNEL); if (!data) return -ENOMEM; - nla_strscpy(data, na, len); ret = cpulist_parse(data, mask); kfree(data); return ret; -- cgit v1.2.3 From 36ffbe1d5e99ebe17bea60e65e053dbcd89bc04c Mon Sep 17 00:00:00 2001 From: Bradley Morgan Date: Tue, 28 Jul 2026 20:21:03 +0000 Subject: taskstats: drop the dead NULL attribute check in parse() Patch series "taskstats: tidy up the cpumask command path". Two small cleanups from reading kernel/taskstats.c. No functional change in either one. This patch (of 2): taskstats_user_cmd() only calls the cpumask handlers after checking the same info->attrs[] entry, so parse() never sees a NULL attribute. Drop the check and its odd "return 1", which no caller tested for anyway. No functional change. Link: https://lore.kernel.org/20260728202104.17839-1-include@grrlz.net Link: https://lore.kernel.org/20260728202104.17839-2-include@grrlz.net Signed-off-by: Bradley Morgan Cc: Balbir Singh Signed-off-by: Andrew Morton --- kernel/taskstats.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'kernel/taskstats.c') diff --git a/kernel/taskstats.c b/kernel/taskstats.c index 74f89b632c0a..7af47511152d 100644 --- a/kernel/taskstats.c +++ b/kernel/taskstats.c @@ -361,8 +361,6 @@ static int parse(struct nlattr *na, struct cpumask *mask) int len; int ret; - if (na == NULL) - return 1; len = nla_len(na); if (len > TASKSTATS_CPUMASK_MAXLEN) return -E2BIG; -- cgit v1.2.3 From c4904c4ae33c5f76c10b03a2283e47d8a6736941 Mon Sep 17 00:00:00 2001 From: Bradley Morgan Date: Tue, 28 Jul 2026 20:21:04 +0000 Subject: taskstats: fold the two cpumask handlers into one cmd_attr_register_cpumask() and cmd_attr_deregister_cpumask() differed only in which attribute they parsed and which action they passed on, so take both as arguments. __free(free_cpumask_var) then removes the goto. No functional change. Link: https://lore.kernel.org/20260728202104.17839-3-include@grrlz.net Signed-off-by: Bradley Morgan Cc: Balbir Singh Signed-off-by: Andrew Morton --- kernel/taskstats.c | 38 ++++++++++++-------------------------- 1 file changed, 12 insertions(+), 26 deletions(-) (limited to 'kernel/taskstats.c') diff --git a/kernel/taskstats.c b/kernel/taskstats.c index 7af47511152d..f31df72f0e9d 100644 --- a/kernel/taskstats.c +++ b/kernel/taskstats.c @@ -448,36 +448,18 @@ static int cgroupstats_user_cmd(struct sk_buff *skb, struct genl_info *info) return send_reply(rep_skb, info); } -static int cmd_attr_register_cpumask(struct genl_info *info) +static int cmd_attr_cpumask(struct genl_info *info, int attr, + enum actions action) { - cpumask_var_t mask; + cpumask_var_t mask __free(free_cpumask_var) = CPUMASK_VAR_NULL; int rc; if (!alloc_cpumask_var(&mask, GFP_KERNEL)) return -ENOMEM; - rc = parse(info->attrs[TASKSTATS_CMD_ATTR_REGISTER_CPUMASK], mask); + rc = parse(info->attrs[attr], mask); if (rc < 0) - goto out; - rc = add_del_listener(info->snd_portid, mask, REGISTER); -out: - free_cpumask_var(mask); - return rc; -} - -static int cmd_attr_deregister_cpumask(struct genl_info *info) -{ - cpumask_var_t mask; - int rc; - - if (!alloc_cpumask_var(&mask, GFP_KERNEL)) - return -ENOMEM; - rc = parse(info->attrs[TASKSTATS_CMD_ATTR_DEREGISTER_CPUMASK], mask); - if (rc < 0) - goto out; - rc = add_del_listener(info->snd_portid, mask, DEREGISTER); -out: - free_cpumask_var(mask); - return rc; + return rc; + return add_del_listener(info->snd_portid, mask, action); } static size_t taskstats_packet_size(void) @@ -552,9 +534,13 @@ err: static int taskstats_user_cmd(struct sk_buff *skb, struct genl_info *info) { if (info->attrs[TASKSTATS_CMD_ATTR_REGISTER_CPUMASK]) - return cmd_attr_register_cpumask(info); + return cmd_attr_cpumask(info, + TASKSTATS_CMD_ATTR_REGISTER_CPUMASK, + REGISTER); else if (info->attrs[TASKSTATS_CMD_ATTR_DEREGISTER_CPUMASK]) - return cmd_attr_deregister_cpumask(info); + return cmd_attr_cpumask(info, + TASKSTATS_CMD_ATTR_DEREGISTER_CPUMASK, + DEREGISTER); else if (info->attrs[TASKSTATS_CMD_ATTR_PID]) return cmd_attr_pid(info); else if (info->attrs[TASKSTATS_CMD_ATTR_TGID]) -- cgit v1.2.3