Skip to content

Commit b573cf6

Browse files
author
Alexei Starovoitov
committed
Merge branch 'bpf-align-syscall-writeback-behavior-with-user-declared-size'
Yuyang Huang says: ==================== bpf: Align syscall writeback behavior with user-declared size This series fixes an out-of-bounds write vulnerability in BPF_PROG_QUERY while maintaining backward compatibility for older userspace applications. BPF_PROG_QUERY unconditionally writes back the 'query.revision' field to userspace. If userspace passes a smaller 'bpf_attr' structure (e.g. 40 bytes, which was the cgroup query layout before 'query.revision' was added), the kernel performs an out-of-bounds write. We address this by propagating the user-provided 'uattr_size' down to the cgroup query handlers and conditionally skipping the write-back of 'query.revision' if the buffer is too small. This allows legacy cgroup queries to succeed safely. tcx and netkit queries are left unchanged since they were introduced in the same merge window as 'query.revision' and have no legacy callers. Finally, we add a selftest to verify these boundary behaviors. Changes since v2: - Propagate uattr_size to __cgroup_bpf_query() and conditionally write revision (instead of unconditionally rejecting smaller sizes in front-gate). - Update BPF selftests to verify that cgroup queries succeed with OLD_QUERY_SIZE without writing revision, and succeed with FULL_QUERY_SIZE. - Remove early size checks in the front-gate to keep the patch minimal. Changes since v1: - Simplify the kernel fix to checking the size only in bpf_prog_query(). - Revert all other subsystem query plumbing changes. - Update BPF selftest to target BPF_CGROUP_INET_INGRESS cgroup query, and add verification for attr size boundaries. ==================== Link: https://patch.msgid.link/20260531075600.4058207-1-yuyanghuang@google.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
2 parents 9a720e0 + 5add3a4 commit b573cf6

4 files changed

Lines changed: 82 additions & 11 deletions

File tree

include/linux/bpf-cgroup.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ int cgroup_bpf_prog_detach(const union bpf_attr *attr,
421421
enum bpf_prog_type ptype);
422422
int cgroup_bpf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
423423
int cgroup_bpf_prog_query(const union bpf_attr *attr,
424-
union bpf_attr __user *uattr);
424+
union bpf_attr __user *uattr, u32 uattr_size);
425425

426426
const struct bpf_func_proto *
427427
cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog);
@@ -452,7 +452,8 @@ static inline int cgroup_bpf_link_attach(const union bpf_attr *attr,
452452
}
453453

454454
static inline int cgroup_bpf_prog_query(const union bpf_attr *attr,
455-
union bpf_attr __user *uattr)
455+
union bpf_attr __user *uattr,
456+
u32 uattr_size)
456457
{
457458
return -EINVAL;
458459
}

kernel/bpf/cgroup.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,7 +1208,7 @@ static int cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
12081208

12091209
/* Must be called with cgroup_mutex held to avoid races. */
12101210
static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
1211-
union bpf_attr __user *uattr)
1211+
union bpf_attr __user *uattr, u32 uattr_size)
12121212
{
12131213
__u32 __user *prog_attach_flags = u64_to_user_ptr(attr->query.prog_attach_flags);
12141214
bool effective_query = attr->query.query_flags & BPF_F_QUERY_EFFECTIVE;
@@ -1259,7 +1259,8 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
12591259
return -EFAULT;
12601260
if (!effective_query && from_atype == to_atype)
12611261
revision = cgrp->bpf.revisions[from_atype];
1262-
if (copy_to_user(&uattr->query.revision, &revision, sizeof(revision)))
1262+
if (uattr_size >= offsetofend(union bpf_attr, query.revision) &&
1263+
copy_to_user(&uattr->query.revision, &revision, sizeof(revision)))
12631264
return -EFAULT;
12641265
if (attr->query.prog_cnt == 0 || !prog_ids || !total_cnt)
12651266
/* return early if user requested only program count + flags */
@@ -1312,12 +1313,12 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
13121313
}
13131314

13141315
static int cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
1315-
union bpf_attr __user *uattr)
1316+
union bpf_attr __user *uattr, u32 uattr_size)
13161317
{
13171318
int ret;
13181319

13191320
cgroup_lock();
1320-
ret = __cgroup_bpf_query(cgrp, attr, uattr);
1321+
ret = __cgroup_bpf_query(cgrp, attr, uattr, uattr_size);
13211322
cgroup_unlock();
13221323
return ret;
13231324
}
@@ -1520,7 +1521,7 @@ int cgroup_bpf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
15201521
}
15211522

15221523
int cgroup_bpf_prog_query(const union bpf_attr *attr,
1523-
union bpf_attr __user *uattr)
1524+
union bpf_attr __user *uattr, u32 uattr_size)
15241525
{
15251526
struct cgroup *cgrp;
15261527
int ret;
@@ -1529,7 +1530,7 @@ int cgroup_bpf_prog_query(const union bpf_attr *attr,
15291530
if (IS_ERR(cgrp))
15301531
return PTR_ERR(cgrp);
15311532

1532-
ret = cgroup_bpf_query(cgrp, attr, uattr);
1533+
ret = cgroup_bpf_query(cgrp, attr, uattr, uattr_size);
15331534

15341535
cgroup_put(cgrp);
15351536
return ret;

kernel/bpf/syscall.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4719,7 +4719,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
47194719
#define BPF_PROG_QUERY_LAST_FIELD query.revision
47204720

47214721
static int bpf_prog_query(const union bpf_attr *attr,
4722-
union bpf_attr __user *uattr)
4722+
union bpf_attr __user *uattr, u32 uattr_size)
47234723
{
47244724
if (!bpf_net_capable())
47254725
return -EPERM;
@@ -4758,7 +4758,7 @@ static int bpf_prog_query(const union bpf_attr *attr,
47584758
case BPF_CGROUP_GETSOCKOPT:
47594759
case BPF_CGROUP_SETSOCKOPT:
47604760
case BPF_LSM_CGROUP:
4761-
return cgroup_bpf_prog_query(attr, uattr);
4761+
return cgroup_bpf_prog_query(attr, uattr, uattr_size);
47624762
case BPF_LIRC_MODE2:
47634763
return lirc_prog_query(attr, uattr);
47644764
case BPF_FLOW_DISSECTOR:
@@ -6376,7 +6376,7 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
63766376
err = bpf_prog_detach(&attr);
63776377
break;
63786378
case BPF_PROG_QUERY:
6379-
err = bpf_prog_query(&attr, uattr.user);
6379+
err = bpf_prog_query(&attr, uattr.user, size);
63806380
break;
63816381
case BPF_PROG_TEST_RUN:
63826382
err = bpf_prog_test_run(&attr, uattr.user);
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
/* Copyright (c) 2026 Google LLC */
3+
#include <linux/bpf.h>
4+
#include <unistd.h>
5+
#include <sys/syscall.h>
6+
#include <test_progs.h>
7+
#include <cgroup_helpers.h>
8+
#include "cgroup_skb_direct_packet_access.skel.h"
9+
10+
#define OLD_QUERY_SIZE offsetofend(union bpf_attr, query.prog_cnt)
11+
#define FULL_QUERY_SIZE offsetofend(union bpf_attr, query.revision)
12+
13+
static void test_query_size_boundaries(void)
14+
{
15+
struct cgroup_skb_direct_packet_access *skel;
16+
struct bpf_link *link = NULL;
17+
union bpf_attr attr;
18+
int cg_fd = -1;
19+
int err;
20+
21+
skel = cgroup_skb_direct_packet_access__open_and_load();
22+
if (!ASSERT_OK_PTR(skel, "skel_load"))
23+
return;
24+
25+
cg_fd = test__join_cgroup("/attr_size_cg");
26+
if (!ASSERT_GE(cg_fd, 0, "join_cgroup"))
27+
goto cleanup;
28+
29+
link = bpf_program__attach_cgroup(skel->progs.direct_packet_access,
30+
cg_fd);
31+
if (!ASSERT_OK_PTR(link, "cg_attach"))
32+
goto cleanup;
33+
34+
memset(&attr, 0, sizeof(attr));
35+
attr.query.target_fd = cg_fd;
36+
attr.query.attach_type = BPF_CGROUP_INET_INGRESS;
37+
attr.query.revision = 0xdeadbeefdeadbeefULL;
38+
39+
err = syscall(__NR_bpf, BPF_PROG_QUERY, &attr, OLD_QUERY_SIZE);
40+
if (ASSERT_OK(err, "query_old_size")) {
41+
ASSERT_EQ(attr.query.prog_cnt, 1, "prog_cnt_written_old");
42+
ASSERT_EQ(attr.query.revision, 0xdeadbeefdeadbeefULL,
43+
"revision_not_written_old");
44+
}
45+
46+
memset(&attr, 0, sizeof(attr));
47+
attr.query.target_fd = cg_fd;
48+
attr.query.attach_type = BPF_CGROUP_INET_INGRESS;
49+
50+
err = syscall(__NR_bpf, BPF_PROG_QUERY, &attr, FULL_QUERY_SIZE);
51+
if (!ASSERT_OK(err, "query_full_size"))
52+
goto cleanup;
53+
54+
ASSERT_EQ(attr.query.prog_cnt, 1, "prog_cnt_written");
55+
ASSERT_GT(attr.query.revision, 0, "revision_written");
56+
57+
cleanup:
58+
if (link)
59+
bpf_link__destroy(link);
60+
if (cg_fd >= 0)
61+
close(cg_fd);
62+
cgroup_skb_direct_packet_access__destroy(skel);
63+
}
64+
65+
void test_bpf_attr_size(void)
66+
{
67+
if (test__start_subtest("query_size_boundaries"))
68+
test_query_size_boundaries();
69+
}

0 commit comments

Comments
 (0)