Skip to contentSkip to conversation

bpf: Check tail zero of bpf_map_info and bpf_prog_info

PATCH Superseded Leon Hwang 4 participants 10 messages Jun 3 – Jun 4

Subsystems (4)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Leon Hwang

author
#0 lore raw

cover bpf: Check tail zero of bpf_map_info and bpf_prog_info

Check the tail bytes of bpf_map_info and bpf_prog_info due to padding when getting map info and prog info via BPF_OBJ_GET_INFO_BY_FD, which was discussed in the thread
"bpf: Check tail zero of bpf_common_attr using offsetofend" [1].

Links:
[1] https://lore.kernel.org/bpf/[email protected]/

Leon Hwang (3):
bpf: Check tail zero of bpf_map_info
bpf: Check tail zero of bpf_prog_info selftests/bpf: Add tests to verify checking padding bytes for

    bpf_[map,prog]_info
 kernel/bpf/syscall.c                          | 10 ++--
 .../selftests/bpf/prog_tests/bpf_attr_size.c  | 55 +++++++++++++++++++
 2 files changed, 61 insertions(+), 4 deletions(-)
Signature · 1 line
2.54.0
author
#1 lore raw

PATCH 1/3 bpf: Check tail zero of bpf_map_info

Since there're 4 bytes padding at the end of struct bpf_map_info, they won't be checked by bpf_check_uarg_tail_zero().

pahole -C bpf_map_info ./vmlinux

struct bpf_map_info {
	...
	__u64                      hash __attribute__((__aligned__(8))); /*    88     8 */
	__u32                      hash_size;            /*    96     4 */

	/* size: 104, cachelines: 2, members: 18 */
	/* padding: 4 */
	/* forced alignments: 1 */
	/* last cacheline: 40 bytes */
} __attribute__((__aligned__(8)));

If a future kernel extension adds a new 4-byte field, older userspace programs allocating this structure on the stack might inadvertently pass uninitialized stack garbage into the new field, permanently breaking backward compatibility. -- sashiko [1]

Fix it by changing sizeof(info) to
offsetofend(struct bpf_map_info, hash_size).

[1] https://lore.kernel.org/bpf/[email protected]/

Fixes ea2e6467ac36 ("bpf: Return hashes of maps in BPF_OBJ_GET_INFO_BY_FD")
---
 kernel/bpf/syscall.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
kernel/bpf/syscall.c +3 -2
index 625a4366fe6d..1007fb7c87e9 100644
@@ -5400,10 +5400,11 @@ static int bpf_map_get_info_by_fd(struct file *file,
 {
 	struct bpf_map_info __user *uinfo = u64_to_user_ptr(attr->info.info);
 	struct bpf_map_info info;
-	u32 info_len = attr->info.info_len;
+	u32 info_len = attr->info.info_len, len;
 	int err;
 
-	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), sizeof(info), info_len);
+	len = offsetofend(struct bpf_map_info, hash_size);
+	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), len, info_len);
 	if (err)
 		return err;
 	info_len = min_t(u32, sizeof(info), info_len);
Signature · 1 line
2.54.0
Acked-by on v1 1/3
#2 lore raw

Re: [PATCH bpf-next 1/3] bpf: Check tail zero of bpf_map_info

Quoted 20 lines · Leon Hwang

Since there're 4 bytes padding at the end of struct bpf_map_info, they won't be checked by bpf_check_uarg_tail_zero().

pahole -C bpf_map_info ./vmlinux

struct bpf_map_info {
	...
	__u64                      hash __attribute__((__aligned__(8))); /*    88     8 */
	__u32                      hash_size;            /*    96     4 */

	/* size: 104, cachelines: 2, members: 18 */
	/* padding: 4 */
	/* forced alignments: 1 */
	/* last cacheline: 40 bytes */
} __attribute__((__aligned__(8)));

If a future kernel extension adds a new 4-byte field, older userspace programs allocating this structure on the stack might inadvertently pass uninitialized stack garbage into the new field, permanently breaking backward compatibility. -- sashiko [1]

Quoted 8 lines

Fix it by changing sizeof(info) to
offsetofend(struct bpf_map_info, hash_size).

[1] https://lore.kernel.org/bpf/[email protected]/

Fixes ea2e6467ac36 ("bpf: Return hashes of maps in BPF_OBJ_GET_INFO_BY_FD")

---

Quoted 21 lines
 kernel/bpf/syscall.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
kernel/bpf/syscall.c +3 -2
index 625a4366fe6d..1007fb7c87e9 100644
@@ -5400,10 +5400,11 @@ static int bpf_map_get_info_by_fd(struct file *file,
 {
 	struct bpf_map_info __user *uinfo = u64_to_user_ptr(attr->info.info);
 	struct bpf_map_info info;
-	u32 info_len = attr->info.info_len;
+	u32 info_len = attr->info.info_len, len;
 	int err;
 
-	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), sizeof(info), info_len);
+	len = offsetofend(struct bpf_map_info, hash_size);
+	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), len, info_len);
 	if (err)
 		return err;
 	info_len = min_t(u32, sizeof(info), info_len);
author
#3 lore raw

PATCH 2/3 bpf: Check tail zero of bpf_prog_info

In reply to Leon Hwang

Since there're 4 bytes padding at the end of struct bpf_prog_info, they won't be checked by bpf_check_uarg_tail_zero().

pahole -C bpf_prog_info ./vmlinux

struct bpf_prog_info {
	...
	__u32                      attach_btf_obj_id;    /*   220     4 */
	__u32                      attach_btf_id;        /*   224     4 */

	/* size: 232, cachelines: 4, members: 38 */
	/* sum members: 224 */
	/* sum bitfield members: 1 bits, bit holes: 1, sum bit holes: 31 bits */
	/* padding: 4 */
	/* forced alignments: 9 */
	/* last cacheline: 40 bytes */
} __attribute__((__aligned__(8)));

If a future kernel extension adds a new 4-byte field, older userspace programs allocating this structure on the stack might inadvertently pass uninitialized stack garbage into the new field, permanently breaking backward compatibility. -- sashiko [1]

Fix it by changing sizeof(info) to
offsetofend(struct bpf_prog_info, attach_btf_id).

[1] https://lore.kernel.org/bpf/[email protected]/

Fixes b79c9fc9551b ("bpf: implement BPF_PROG_QUERY for BPF_LSM_CGROUP")
---
 kernel/bpf/syscall.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
kernel/bpf/syscall.c +3 -2
index 1007fb7c87e9..0c86a70d31c1 100644
@@ -5115,10 +5115,11 @@ static int bpf_prog_get_info_by_fd(struct file *file,
 	u32 info_len = attr->info.info_len;
 	struct bpf_prog_kstats stats;
 	char __user *uinsns;
-	u32 ulen;
+	u32 ulen, len;
 	int err;
 
-	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), sizeof(info), info_len);
+	len = offsetofend(struct bpf_prog_info, attach_btf_id);
+	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), len, info_len);
 	if (err)
 		return err;
 	info_len = min_t(u32, sizeof(info), info_len);
Signature · 1 line
2.54.0
#4 lore raw

Re: [PATCH bpf-next 2/3] bpf: Check tail zero of bpf_prog_info

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] UAPI regression and fundamentally flawed forward-compatibility logic due to strict zero-checks…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] UAPI regression and fundamentally flawed forward-compatibility logic due to strict zero-checks on previously ignored struct padding. --

commit 7758939eea6a099c4db0364bf334e0034c2864fc
Author: Leon Hwang <[email protected]>

bpf: Check tail zero of bpf_prog_info

This commit changes the size passed to bpf_check_uarg_tail_zero() to exclude the trailing padding of struct bpf_prog_info. The goal is to prevent older userspace programs from passing uninitialized stack garbage into a future 4-byte field.

Quoted 5 lines
kernel/bpf/syscall.c +0 -0
index 1007fb7c87e91..0c86a70d31c17 100644

kernel/bpf/syscall.c static int bpf_prog_get_info_by_fd(struct file *file, · line 5122

Quoted 10 lines
kernel/bpf/syscall.c +3 -2
@@ -5115,10 +5115,11 @@ static int bpf_prog_get_info_by_fd(struct file *file,
 	u32 info_len = attr->info.info_len;
 	struct bpf_prog_kstats stats;
 	char __user *uinsns;
-	u32 ulen;
+	u32 ulen, len;
 	int err;
 
-	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), sizeof(info), info_len);
+	len = offsetofend(struct bpf_prog_info, attach_btf_id);
+	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), len, info_len);

[Severity: Critical]
Does this change break the ABI for existing deployed binaries?

Previously, expected_size was 232 (sizeof(info)). If an existing binary passes info_len = 232 with uninitialized stack garbage in the 4-byte padding, bpf_check_uarg_tail_zero() would see actual_size <= expected_size (232 <= 232) and immediately return 0.

By reducing expected_size to 228, actual_size > expected_size (232 > 228), so bpf_check_uarg_tail_zero() will actively check the 4 padding bytes and return -E2BIG if they contain garbage. This rejects previously working applications.

Additionally, does this actually protect against the stated future issue?

If a future kernel adds a new 4-byte field, the struct size stays 232, and the new kernel's expected_size will be 232. At that point, those old buggy binaries passing info_len = 232 will once again hit the fast path (actual_size <= expected_size), skipping the check and allowing the uninitialized padding to be consumed as the new field.

Quoted 3 lines
kernel/bpf/syscall.c +0 -0
 	if (err)
 		return err;
 	info_len = min_t(u32, sizeof(info), info_len);
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
author
#5 lore raw

Re: [PATCH bpf-next 2/3] bpf: Check tail zero of bpf_prog_info

On 3/6/26 23:14, [email protected] wrote:
[...]

Quoted 29 lines
Quoted 5 lines
kernel/bpf/syscall.c +0 -0
index 1007fb7c87e91..0c86a70d31c17 100644

kernel/bpf/syscall.c static int bpf_prog_get_info_by_fd(struct file *file, · line 5122

Quoted 10 lines
kernel/bpf/syscall.c +3 -2
@@ -5115,10 +5115,11 @@ static int bpf_prog_get_info_by_fd(struct file *file,
 	u32 info_len = attr->info.info_len;
 	struct bpf_prog_kstats stats;
 	char __user *uinsns;
-	u32 ulen;
+	u32 ulen, len;
 	int err;
 
-	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), sizeof(info), info_len);
+	len = offsetofend(struct bpf_prog_info, attach_btf_id);
+	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), len, info_len);

[Severity: Critical]
Does this change break the ABI for existing deployed binaries?

Previously, expected_size was 232 (sizeof(info)). If an existing binary passes info_len = 232 with uninitialized stack garbage in the 4-byte padding, bpf_check_uarg_tail_zero() would see actual_size <= expected_size (232 <= 232) and immediately return 0.

By reducing expected_size to 228, actual_size > expected_size (232 > 228), so bpf_check_uarg_tail_zero() will actively check the 4 padding bytes and return -E2BIG if they contain garbage. This rejects previously working applications.

Quoted 8 lines

Additionally, does this actually protect against the stated future issue?

If a future kernel adds a new 4-byte field, the struct size stays 232, and the new kernel's expected_size will be 232. At that point, those old buggy binaries passing info_len = 232 will once again hit the fast path (actual_size <= expected_size), skipping the check and allowing the uninitialized padding to be consumed as the new field.

For this case, forward compatibility is more important than backward compatibility. Thereafter, those old buggy binaries should fix themselves instead.

Thanks,
Leon

Acked-by on v1 2/3
#6 lore raw

Re: [PATCH bpf-next 2/3] bpf: Check tail zero of bpf_prog_info

In reply to Leon Hwang
Quoted 22 lines · Leon Hwang

Since there're 4 bytes padding at the end of struct bpf_prog_info, they won't be checked by bpf_check_uarg_tail_zero().

pahole -C bpf_prog_info ./vmlinux

struct bpf_prog_info {
	...
	__u32                      attach_btf_obj_id;    /*   220     4 */
	__u32                      attach_btf_id;        /*   224     4 */

	/* size: 232, cachelines: 4, members: 38 */
	/* sum members: 224 */
	/* sum bitfield members: 1 bits, bit holes: 1, sum bit holes: 31 bits */
	/* padding: 4 */
	/* forced alignments: 9 */
	/* last cacheline: 40 bytes */
} __attribute__((__aligned__(8)));

If a future kernel extension adds a new 4-byte field, older userspace programs allocating this structure on the stack might inadvertently pass uninitialized stack garbage into the new field, permanently breaking backward compatibility. -- sashiko [1]

Quoted 8 lines

Fix it by changing sizeof(info) to
offsetofend(struct bpf_prog_info, attach_btf_id).

[1] https://lore.kernel.org/bpf/[email protected]/

Fixes b79c9fc9551b ("bpf: implement BPF_PROG_QUERY for BPF_LSM_CGROUP")

---

Quoted 21 lines
 kernel/bpf/syscall.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
kernel/bpf/syscall.c +3 -2
index 1007fb7c87e9..0c86a70d31c1 100644
@@ -5115,10 +5115,11 @@ static int bpf_prog_get_info_by_fd(struct file *file,
 	u32 info_len = attr->info.info_len;
 	struct bpf_prog_kstats stats;
 	char __user *uinsns;
-	u32 ulen;
+	u32 ulen, len;
 	int err;
 
-	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), sizeof(info), info_len);
+	len = offsetofend(struct bpf_prog_info, attach_btf_id);
+	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), len, info_len);
 	if (err)
 		return err;
 	info_len = min_t(u32, sizeof(info), info_len);
#7 lore raw

Re: [PATCH bpf-next 2/3] bpf: Check tail zero of bpf_prog_info

In reply to Leon Hwang
Is this the right commit for the Fixes: tag?
Quoted 4 lines

Fix it by changing sizeof(info) to
offsetofend(struct bpf_prog_info, attach_btf_id).

Fixes b79c9fc9551b ("bpf: implement BPF_PROG_QUERY for BPF_LSM_CGROUP")

Is this the right commit for the Fixes: tag?

The change avoids the trailing padding that exists because struct bpf_prog_info has 4 bytes of padding at its end, so sizeof(info) (232) differs from offsetofend(struct bpf_prog_info, attach_btf_id) (228).

That trailing padding first appeared when __u32 verified_insns was appended after the last __u64 member (recursion_misses). Before that, the struct ended on an 8-byte-aligned member and had no trailing padding.

b79c9fc9551b only appended two further __u32 fields (attach_btf_obj_id, attach_btf_id), while the 4-byte trailing padding already existed and was unchanged.

Would this be more accurate?

Fixes aba64c7da983 ("bpf: Add verified_insns to bpf_prog_info and fdinfo")

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26893539599

author
#8 lore raw

Re: [PATCH bpf-next 2/3] bpf: Check tail zero of bpf_prog_info

Quoted 16 lines · On 3/6/26 23:37, [email protected] wrote:
Quoted 4 lines

Fix it by changing sizeof(info) to
offsetofend(struct bpf_prog_info, attach_btf_id).

Fixes b79c9fc9551b ("bpf: implement BPF_PROG_QUERY for BPF_LSM_CGROUP")

Is this the right commit for the Fixes: tag?

The change avoids the trailing padding that exists because struct bpf_prog_info has 4 bytes of padding at its end, so sizeof(info) (232) differs from offsetofend(struct bpf_prog_info, attach_btf_id) (228).

That trailing padding first appeared when __u32 verified_insns was appended after the last __u64 member (recursion_misses). Before that, the struct ended on an 8-byte-aligned member and had no trailing padding.

Quoted 9 lines

b79c9fc9551b only appended two further __u32 fields (attach_btf_obj_id, attach_btf_id), while the 4-byte trailing padding already existed and was unchanged.

Would this be more accurate?

Fixes aba64c7da983 ("bpf: Add verified_insns to bpf_prog_info and fdinfo")

Agreed.

Will update the Fixes tag in the next revision.

Thanks,
Leon

author
#9 lore raw

PATCH 3/3 selftests/bpf: Add tests to verify checking padding bytes for bpf_[map,prog]_info

In reply to Leon Hwang

Add two tests to verify that the tail padding 4 bytes of struct bpf_map_info and bpf_prog_info are checked in syscall.c using bpf_check_uarg_tail_zero().

---
 .../selftests/bpf/prog_tests/bpf_attr_size.c  | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c +55 -0
index 32159dc64da8..87842c4347a6 100644
@@ -62,8 +62,63 @@ static void test_query_size_boundaries(void)
 	cgroup_skb_direct_packet_access__destroy(skel);
 }
 
+static void test_map_info_tail_zero(void)
+{
+	LIBBPF_OPTS(bpf_map_create_opts, map_opts);
+	struct bpf_map_info_fake {
+		__u8 info[offsetofend(struct bpf_map_info, hash_size)];
+		__u32 pad;
+	} info = {
+		.pad = 1,
+	};
+	int map_fd, err;
+	__u32 info_len;
+
+	map_fd = bpf_map_create(BPF_MAP_TYPE_ARRAY, "arr", sizeof(int), 1, 1, &map_opts);
+	if (!ASSERT_GE(map_fd, 0, "bpf_map_create"))
+		return;
+
+	info_len = sizeof(info);
+	err = bpf_obj_get_info_by_fd(map_fd, &info, &info_len);
+	ASSERT_EQ(err, -E2BIG, "bpf_obj_get_info_by_fd");
+
+	close(map_fd);
+}
+
+static void test_prog_info_tail_zero(void)
+{
+	LIBBPF_OPTS(bpf_prog_load_opts, prog_opts);
+	struct bpf_insn insns[] = {
+		BPF_MOV64_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+	};
+	struct bpf_prog_info_fake {
+		__u8 info[offsetofend(struct bpf_prog_info, attach_btf_id)];
+		__u32 pad;
+	} info = {
+		.pad = 1,
+	};
+	int prog_fd, err;
+	__u32 info_len;
+
+	prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, "test_prog", "GPL", insns,
+				ARRAY_SIZE(insns), &prog_opts);
+	if (!ASSERT_GE(prog_fd, 0, "bpf_prog_load"))
+		return;
+
+	info_len = sizeof(info);
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	ASSERT_EQ(err, -E2BIG, "bpf_obj_get_info_by_fd");
+
+	close(prog_fd);
+}
+
 void test_bpf_attr_size(void)
 {
 	if (test__start_subtest("query_size_boundaries"))
 		test_query_size_boundaries();
+	if (test__start_subtest("map_info_tail_zero"))
+		test_map_info_tail_zero();
+	if (test__start_subtest("prog_info_tail_zero"))
+		test_prog_info_tail_zero();
 }
Signature · 1 line
2.54.0