Skip to contentSkip to conversation

taskstats: fix cgroupstats invalid fd handling and add selftests

PATCH Applied Yiyang Chen 4 participants 7 messages Jul 10 – Jul 27

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover 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.

Yiyang Chen (2):
taskstats: return -EBADF when cgroupstats receives an invalid fd selftests/acct: add cgroupstats functional test

 kernel/taskstats.c                         |   2 +-
 tools/testing/selftests/acct/Makefile      |   1 +
 tools/testing/selftests/acct/cgroupstats.c | 354 +++++++++++++++++++++
 3 files changed, 356 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/acct/cgroupstats.c
Signature · 1 line
2.43.0
#1 lore raw

PATCH 1/2 taskstats: return -EBADF when cgroupstats receives an invalid fd

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.

---
 kernel/taskstats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/taskstats.c +1 -1
index 2cd0172d0516..8d115670a3ca 100644
@@ -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));
 
Signature · 1 line
2.43.0
Acked-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] taskstats: return -EBADF when cgroupstats receives an invalid fd

Quoted 22 lines · Yiyang Chen

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.

---
 kernel/taskstats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/taskstats.c +0 -0
index 2cd0172d0516..8d115670a3ca 100644

kernel/taskstats.c static int cgroupstats_user_cmd(struct sk_buff *skb, struct genl_info *info) · line 429

Quoted 8 lines
kernel/taskstats.c +1 -1
@@ -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));
 

Thanks!

#3 lore raw

PATCH 2/2 selftests/acct: add cgroupstats functional test

In reply to Yiyang Chen

Add a self-contained test for the CGROUPSTATS_CMD_GET genl command that covers three cases:

- querying a cgroup v1 hierarchy and verifying the response contains non-zero task counts
- rejecting an invalid fd without NLM_F_ACK
- rejecting an invalid fd with NLM_F_ACK

---
 tools/testing/selftests/acct/Makefile      |   1 +
 tools/testing/selftests/acct/cgroupstats.c | 354 +++++++++++++++++++++
 2 files changed, 355 insertions(+)
 create mode 100644 tools/testing/selftests/acct/cgroupstats.c
tools/testing/selftests/acct/Makefile +1 -0
index 083cab5ddb72..db88d65f5581 100644
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 TEST_GEN_PROGS := acct_syscall
 TEST_GEN_PROGS += taskstats_fill_stats_tgid
+TEST_GEN_PROGS += cgroupstats
 
 CFLAGS += -Wall
 LDLIBS += -lpthread
tools/testing/selftests/acct/cgroupstats.c new file +354 -0

354 lines not on this page · raw message

Signature · 1 line
2.43.0
#4 lore raw
In reply to Yiyang Chen
Quoted 10 lines · On Sat, 11 Jul 2026 04:07:35 +0800 Yiyang Chen <[email protected]> wrote:

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.

Quoted 4 lines

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.

lgtm, thanks.

It's a little odd to group two unrelated(?) changes in one series like this. Oh well, let's not redo things for this.

Should the netlink_open() in taskstats_fill_stats_tgid.c also be setting that timeout? If so, we can share a single implementation - a little cleanup.

There's a lot of code duplication in selftests/ :(

#5 lore raw
Quoted 18 lines · Andrew Morton
Quoted 10 lines · On Sat, 11 Jul 2026 04:07:35 +0800 Yiyang Chen <[email protected]> wrote:

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.

Quoted 4 lines

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.

Quoted 4 lines

lgtm, thanks.

It's a little odd to group two unrelated(?) changes in one series like this. Oh well, let's not redo things for this.

I've put the fix and the corresponding regression test into a patch series, but since there
didn't seem to be a test suite for cgroupstats before, I also added a test case for a typical
scenario (test 1).

Quoted 6 lines

Should the netlink_open() in taskstats_fill_stats_tgid.c also be setting that timeout? If so, we can share a single implementation - a little cleanup.

There's a lot of code duplication in selftests/ :(

Thanks for you advise:)
I'll do some cleanup stuff in the follow-up that extracts the duplicated helpers into common headers for taskstats selftests