* [PATCH bpf-next v5 1/2] bpf: Validate program attach type during link update
@ 2026-09-01 1:47 Sanghyun Park
2026-09-01 1:47 ` [PATCH bpf-next v5 2/2] selftests/bpf: Cover attach type checks in " Sanghyun Park
2026-09-05 0:55 ` [PATCH bpf-next v5 1/2] bpf: Validate program attach type during " Emil Tsalapatis
0 siblings, 2 replies; 6+ messages in thread
From: Sanghyun Park @ 2026-09-01 1:47 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, bpf
Cc: Sanghyun Park, Leon Hwang, John Fastabend, Martin KaFai Lau,
Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
Ihor Solodrai, Shuah Khan, Stanislav Fomichev, Pu Lehui,
Suchit Karunakaran, Xu Kuohai, linux-kernel, linux-kselftest
Cgroup link updates compare only the program type. Programs verified
for different hooks can share a type, so a UDP6 sock_addr program can
replace a UDP4 program and write beyond the four-byte ipc.addr field in
the stack-local struct ipcm_cookie. The same gap permits incompatible
LSM_MAC and LSM_CGROUP replacements.
Validate every replacement program against the link attach type before
dispatching to the link-specific update operation. Keep the CGROUP_SKB
CAP_NET_ADMIN check at initial attach so pinned or delegated link FDs can
still be updated after a capability drop.
Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")
Acked-by: Leon Hwang <leon.hwang@linux.dev>
Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
---
v5:
- Fold the link-create permission check call onto one line.
v4: https://lore.kernel.org/r/20260831023918.1857658-1-sanghyun.park.cnu@gmail.com
- Validate every program link update through the shared attach-type helper.
- Fold the LSM attach-flavor check into that helper.
- Keep the CGROUP_SKB permission check in the attach-only wrapper and rename
the wrapper to describe that role.
- Follow the preferred BPF multi-line comment style.
- Add focused regression selftests as patch 2/2.
v3: https://lore.kernel.org/r/20260821084726.3769957-2-sanghyun.park.cnu@gmail.com
- Factor the CGROUP_SKB CAP_NET_ADMIN check into an attach-only helper.
v2: https://lore.kernel.org/r/20260818061021.2551771-2-sanghyun.park.cnu@gmail.com
- Extend validation from cgroup sock_addr programs to all cgroup program
types, including exact LSM attach flavors.
- Preserve legacy CGROUP_SKB ingress/egress replacement compatibility.
- Keep the CGROUP_SKB CAP_NET_ADMIN check on attach, not link update.
v1: https://lore.kernel.org/r/20260805052858.2390918-3-sanghyun.park.cnu@gmail.com
---
kernel/bpf/syscall.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6874ba1424af..2bcd3f3a2a55 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4481,14 +4481,9 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
case BPF_PROG_TYPE_CGROUP_SOCKOPT:
case BPF_PROG_TYPE_SK_LOOKUP:
+ case BPF_PROG_TYPE_LSM:
return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
case BPF_PROG_TYPE_CGROUP_SKB:
- if (!bpf_token_capable(prog->aux->token, CAP_NET_ADMIN))
- /* cg-skb progs can be loaded by unpriv user.
- * check permissions at attach time.
- */
- return -EPERM;
-
ptype = attach_type_to_prog_type(attach_type);
if (prog->type != ptype)
return -EINVAL;
@@ -4542,6 +4537,20 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
}
}
+static int bpf_prog_attach_check_perm(const struct bpf_prog *prog,
+ enum bpf_attach_type attach_type)
+{
+ /*
+ * CGROUP_SKB programs can be loaded by unprivileged users, so check
+ * permissions at attach time.
+ */
+ if (prog->type == BPF_PROG_TYPE_CGROUP_SKB &&
+ !bpf_token_capable(prog->aux->token, CAP_NET_ADMIN))
+ return -EPERM;
+
+ return bpf_prog_attach_check_attach_type(prog, attach_type);
+}
+
static bool is_cgroup_prog_type(enum bpf_prog_type ptype, enum bpf_attach_type atype,
bool check_atype)
{
@@ -4606,7 +4615,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
if (IS_ERR(prog))
return PTR_ERR(prog);
- if (bpf_prog_attach_check_attach_type(prog, attr->attach_type)) {
+ if (bpf_prog_attach_check_perm(prog, attr->attach_type)) {
bpf_prog_put(prog);
return -EINVAL;
}
@@ -5805,8 +5814,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
if (IS_ERR(prog))
return PTR_ERR(prog);
- ret = bpf_prog_attach_check_attach_type(prog,
- attr->link_create.attach_type);
+ ret = bpf_prog_attach_check_perm(prog, attr->link_create.attach_type);
if (ret)
goto out;
@@ -5968,6 +5976,10 @@ static int link_update(union bpf_attr *attr)
goto out_put_progs;
}
+ ret = bpf_prog_attach_check_attach_type(new_prog, link->attach_type);
+ if (ret)
+ goto out_put_progs;
+
if (link->ops->update_prog)
ret = link->ops->update_prog(link, new_prog, old_prog);
else
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next v5 2/2] selftests/bpf: Cover attach type checks in link update
2026-09-01 1:47 [PATCH bpf-next v5 1/2] bpf: Validate program attach type during link update Sanghyun Park
@ 2026-09-01 1:47 ` Sanghyun Park
2026-09-01 2:50 ` bot+bpf-ci
2026-09-05 1:00 ` Emil Tsalapatis
2026-09-05 0:55 ` [PATCH bpf-next v5 1/2] bpf: Validate program attach type during " Emil Tsalapatis
1 sibling, 2 replies; 6+ messages in thread
From: Sanghyun Park @ 2026-09-01 1:47 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, bpf
Cc: Sanghyun Park, Leon Hwang, John Fastabend, Martin KaFai Lau,
Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
Ihor Solodrai, Shuah Khan, Stanislav Fomichev, Pu Lehui,
Suchit Karunakaran, Xu Kuohai, linux-kernel, linux-kselftest
Pin down the two contracts changed by the companion patch. LINK_UPDATE
validates a replacement program against the link's attach type, while
CGROUP_SKB capability checks apply at LINK_CREATE and not LINK_UPDATE.
Cover incompatible UDP4/UDP6 and LSM attach flavors, plus a CGROUP_SKB
link update after dropping CAP_NET_ADMIN and CAP_SYS_ADMIN.
Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
---
v5:
- Isolate attach and update coverage in separate subtests with their own setup.
- Inline the one-use LSM program FD and use a descriptive link FD.
- Explain the create-versus-update contracts covered by the tests.
v4: https://lore.kernel.org/r/20260831023918.1857658-2-sanghyun.park.cnu@gmail.com
---
.../selftests/bpf/prog_tests/cgroup_link.c | 81 ++++++++++++++++++-
.../selftests/bpf/prog_tests/lsm_cgroup.c | 12 +++
.../testing/selftests/bpf/progs/lsm_cgroup.c | 6 ++
.../selftests/bpf/progs/test_cgroup_link.c | 14 +++-
4 files changed, 111 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
index 15093a69510e..3b43c34917cc 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <test_progs.h>
+#include "cap_helpers.h"
#include "cgroup_helpers.h"
#include "testing_helpers.h"
#include "test_cgroup_link.skel.h"
@@ -24,7 +25,77 @@ int ping_and_check(int exp_calls, int exp_alt_calls)
return 0;
}
-void serial_test_cgroup_link(void)
+static void test_cgroup_link_update(void)
+{
+ const __u64 caps = (1ULL << CAP_NET_ADMIN) | (1ULL << CAP_SYS_ADMIN);
+ struct bpf_link *link = NULL;
+ __u64 saved_caps = 0;
+ int cg_fd = -1, err;
+
+ skel = test_cgroup_link__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "skel_open_load"))
+ return;
+
+ err = setup_cgroup_environment();
+ if (!ASSERT_OK(err, "cg_init"))
+ goto cleanup;
+
+ cg_fd = create_and_get_cgroup("/cgroup_link_update");
+ if (!ASSERT_GE(cg_fd, 0, "cg_create"))
+ goto cleanup;
+
+ err = join_cgroup("/cgroup_link_update");
+ if (!ASSERT_OK(err, "cg_join"))
+ goto cleanup;
+
+ link = bpf_program__attach_cgroup(skel->progs.sendmsg4, cg_fd);
+ if (!ASSERT_OK_PTR(link, "attach_sendmsg4"))
+ goto cleanup;
+
+ err = bpf_link__update_program(link, skel->progs.sendmsg6);
+ ASSERT_EQ(err, -EINVAL, "reject_sendmsg6_update");
+ bpf_link__destroy(link);
+ link = NULL;
+
+ err = cap_disable_effective(caps, &saved_caps);
+ if (!ASSERT_OK(err, "drop_caps_for_attach"))
+ goto cleanup;
+
+ link = bpf_program__attach_cgroup(skel->progs.egress, cg_fd);
+ if (!ASSERT_ERR_PTR(link, "attach_without_net_admin"))
+ goto cleanup;
+ err = libbpf_get_error(link);
+ link = NULL;
+ if (!ASSERT_EQ(err, -EPERM, "attach_err"))
+ goto cleanup;
+
+ err = cap_enable_effective(saved_caps & caps, NULL);
+ if (!ASSERT_OK(err, "restore_caps_for_attach"))
+ goto cleanup;
+
+ link = bpf_program__attach_cgroup(skel->progs.egress, cg_fd);
+ if (!ASSERT_OK_PTR(link, "attach_egress"))
+ goto cleanup;
+
+ err = cap_disable_effective(caps, &saved_caps);
+ if (!ASSERT_OK(err, "drop_caps"))
+ goto cleanup;
+
+ err = bpf_link__update_program(link, skel->progs.egress_alt);
+ ASSERT_OK(err, "update_without_net_admin");
+
+cleanup:
+ err = cap_enable_effective(saved_caps & caps, NULL);
+ ASSERT_OK(err, "restore_caps");
+ bpf_link__destroy(link);
+ if (cg_fd >= 0)
+ close(cg_fd);
+ cleanup_cgroup_environment();
+ test_cgroup_link__destroy(skel);
+ skel = NULL;
+}
+
+static void test_cgroup_link_attach(void)
{
struct {
const char *path;
@@ -253,3 +324,11 @@ void serial_test_cgroup_link(void)
}
cleanup_cgroup_environment();
}
+
+void serial_test_cgroup_link(void)
+{
+ if (test__start_subtest("attach"))
+ test_cgroup_link_attach();
+ if (test__start_subtest("update"))
+ test_cgroup_link_update();
+}
diff --git a/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c b/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c
index 41e867467f6c..0dd44dbb9c95 100644
--- a/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c
+++ b/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c
@@ -74,6 +74,7 @@ static void test_lsm_cgroup_functional(void)
int bind_prog_fd = -1;
int bind_link_fd = -1;
int clone_prog_fd = -1;
+ int socket_link_fd = -1;
int err, fd, prio;
socklen_t socklen;
@@ -156,6 +157,17 @@ static void test_lsm_cgroup_functional(void)
ASSERT_EQ(query_prog_cnt(cgroup_fd, "bpf_lsm_socket_bind"), 1, "prog count");
ASSERT_EQ(query_prog_cnt(cgroup_fd, NULL), 4, "total prog count");
+ socket_link_fd =
+ bpf_link_create(bpf_program__fd(skel->progs.socket_first),
+ cgroup_fd, BPF_LSM_CGROUP, NULL);
+ if (!ASSERT_GE(socket_link_fd, 0, "link create socket_first"))
+ goto detach_cgroup;
+ err = bpf_link_update(socket_link_fd,
+ bpf_program__fd(skel->progs.socket_create_lsm),
+ NULL);
+ ASSERT_EQ(err, -EINVAL, "reject lsm_mac link update");
+ close(socket_link_fd);
+
/* Attach another instance of bind program to another cgroup.
* This should trigger the reuse of the trampoline shim (two
* programs attaching to the same btf_id).
diff --git a/tools/testing/selftests/bpf/progs/lsm_cgroup.c b/tools/testing/selftests/bpf/progs/lsm_cgroup.c
index 3bfa479104be..30727945cecc 100644
--- a/tools/testing/selftests/bpf/progs/lsm_cgroup.c
+++ b/tools/testing/selftests/bpf/progs/lsm_cgroup.c
@@ -213,6 +213,12 @@ int BPF_PROG(socket_first, int family, int type, int protocol, int kern)
return 0;
}
+SEC("lsm/socket_create")
+int BPF_PROG(socket_create_lsm, int family, int type, int protocol, int kern)
+{
+ return 0;
+}
+
SEC("lsm_cgroup/socket_create")
int BPF_PROG(socket_second, int family, int type, int protocol, int kern)
{
diff --git a/tools/testing/selftests/bpf/progs/test_cgroup_link.c b/tools/testing/selftests/bpf/progs/test_cgroup_link.c
index 4faba88e45a5..a987744d129c 100644
--- a/tools/testing/selftests/bpf/progs/test_cgroup_link.c
+++ b/tools/testing/selftests/bpf/progs/test_cgroup_link.c
@@ -20,5 +20,17 @@ int egress_alt(struct __sk_buff *skb)
return 1;
}
-char _license[] SEC("license") = "GPL";
+SEC("cgroup/sendmsg4")
+int sendmsg4(struct bpf_sock_addr *ctx)
+{
+ return 1;
+}
+SEC("cgroup/sendmsg6")
+int sendmsg6(struct bpf_sock_addr *ctx)
+{
+ ctx->msg_src_ip6[2] = 0;
+ return 1;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v5 2/2] selftests/bpf: Cover attach type checks in link update
2026-09-01 1:47 ` [PATCH bpf-next v5 2/2] selftests/bpf: Cover attach type checks in " Sanghyun Park
@ 2026-09-01 2:50 ` bot+bpf-ci
2026-09-05 1:00 ` Emil Tsalapatis
1 sibling, 0 replies; 6+ messages in thread
From: bot+bpf-ci @ 2026-09-01 2:50 UTC (permalink / raw)
To: sanghyun.park.cnu, ast, daniel, andrii, eddyz87, memxor, bpf
Cc: sanghyun.park.cnu, leon.hwang, john.fastabend, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai, shuah, sdf, pulehui,
suchitkarunakaran, xukuohai, linux-kernel, linux-kselftest, ast,
andrii, daniel, martin.lau, eddyz87, yonghong.song, mason,
ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 4570 bytes --]
> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
> index 15093a69510eb..3b43c34917ccc 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
>
> #include <test_progs.h>
> +#include "cap_helpers.h"
> #include "cgroup_helpers.h"
> #include "testing_helpers.h"
> #include "test_cgroup_link.skel.h"
[ ... ]
> +static void test_cgroup_link_update(void)
> +{
> + const __u64 caps = (1ULL << CAP_NET_ADMIN) | (1ULL << CAP_SYS_ADMIN);
> + struct bpf_link *link = NULL;
> + __u64 saved_caps = 0;
> + int cg_fd = -1, err;
> +
> + skel = test_cgroup_link__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "skel_open_load"))
> + return;
This isn't a bug, but since this subtest never goes through
ping_and_check(), would a local `struct test_cgroup_link *skel` work
here instead of the file-scope global (which would also make the
trailing `skel = NULL;` unnecessary)?
> +
> + err = setup_cgroup_environment();
> + if (!ASSERT_OK(err, "cg_init"))
> + goto cleanup;
> +
> + cg_fd = create_and_get_cgroup("/cgroup_link_update");
> + if (!ASSERT_GE(cg_fd, 0, "cg_create"))
> + goto cleanup;
> +
> + err = join_cgroup("/cgroup_link_update");
> + if (!ASSERT_OK(err, "cg_join"))
> + goto cleanup;
A subsystem pattern flags this as potentially concerning: the three-call
sequence setup_cgroup_environment() + create_and_get_cgroup(path) +
join_cgroup(path) is exactly what test__join_cgroup(path) provides in
test_progs.c. Using the shared helper would collapse these three blocks
(and their three separate ASSERT/goto arms) into one call plus one
ASSERT_GE(cg_fd, 0, ...). The harness also sets env.test->
need_cgroup_cleanup so teardown happens even if the subtest exits early.
The hand-rolled sequence is functionally equivalent - the three calls
are in the same order, all return values are checked, and the single
cleanup label reaches cleanup_cgroup_environment() on every failure
path. There's also a consistency argument for writing it out:
test_cgroup_link_attach() in this file hand-rolls the same sequence
because it needs four cgroups and cannot use a single-cgroup helper.
Does the test__join_cgroup() approach make sense for this subtest, or is
the hand-rolled version preferred for consistency with the sibling test?
> +
> + link = bpf_program__attach_cgroup(skel->progs.sendmsg4, cg_fd);
> + if (!ASSERT_OK_PTR(link, "attach_sendmsg4"))
> + goto cleanup;
> +
> + err = bpf_link__update_program(link, skel->progs.sendmsg6);
> + ASSERT_EQ(err, -EINVAL, "reject_sendmsg6_update");
[ ... ]
> diff --git a/tools/testing/selftests/bpf/progs/lsm_cgroup.c b/tools/testing/selftests/bpf/progs/lsm_cgroup.c
> index 3bfa479104be8..30727945cecc3 100644
> --- a/tools/testing/selftests/bpf/progs/lsm_cgroup.c
> +++ b/tools/testing/selftests/bpf/progs/lsm_cgroup.c
> @@ -213,6 +213,12 @@ int BPF_PROG(socket_first, int family, int type, int protocol, int kern)
> return 0;
> }
>
> +SEC("lsm/socket_create")
> +int BPF_PROG(socket_create_lsm, int family, int type, int protocol, int kern)
> +{
> + return 0;
> +}
> +
> SEC("lsm_cgroup/socket_create")
> int BPF_PROG(socket_second, int family, int type, int protocol, int kern)
> {
[ ... ]
> diff --git a/tools/testing/selftests/bpf/progs/test_cgroup_link.c b/tools/testing/selftests/bpf/progs/test_cgroup_link.c
> index 4faba88e45a5a..a987744d129cd 100644
> --- a/tools/testing/selftests/bpf/progs/test_cgroup_link.c
> +++ b/tools/testing/selftests/bpf/progs/test_cgroup_link.c
> @@ -20,5 +20,17 @@ int egress_alt(struct __sk_buff *skb)
> return 1;
> }
>
> -char _license[] SEC("license") = "GPL";
> +SEC("cgroup/sendmsg4")
> +int sendmsg4(struct bpf_sock_addr *ctx)
> +{
> + return 1;
> +}
>
> +SEC("cgroup/sendmsg6")
> +int sendmsg6(struct bpf_sock_addr *ctx)
> +{
> + ctx->msg_src_ip6[2] = 0;
> + return 1;
> +}
This isn't a bug, but is the ctx->msg_src_ip6[2] store here load-bearing
for the -EINVAL assertion, or is it standing in for the out-of-bounds
write the companion patch prevents? A short comment (or making sendmsg6
as minimal as sendmsg4) would make the asymmetry between the two new
programs clearer.
> +
> +char _license[] SEC("license") = "GPL";
---
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/33461362664
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v5 1/2] bpf: Validate program attach type during link update
2026-09-01 1:47 [PATCH bpf-next v5 1/2] bpf: Validate program attach type during link update Sanghyun Park
2026-09-01 1:47 ` [PATCH bpf-next v5 2/2] selftests/bpf: Cover attach type checks in " Sanghyun Park
@ 2026-09-05 0:55 ` Emil Tsalapatis
1 sibling, 0 replies; 6+ messages in thread
From: Emil Tsalapatis @ 2026-09-05 0:55 UTC (permalink / raw)
To: Sanghyun Park
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, bpf, Leon Hwang,
John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song,
Jiri Olsa, Ihor Solodrai, Shuah Khan, Stanislav Fomichev,
Pu Lehui, Suchit Karunakaran, Xu Kuohai, linux-kernel,
linux-kselftest
On Mon, Aug 31, 2026 at 9:47 PM Sanghyun Park
<sanghyun.park.cnu@gmail.com> wrote:
>
> Cgroup link updates compare only the program type. Programs verified
> for different hooks can share a type, so a UDP6 sock_addr program can
> replace a UDP4 program and write beyond the four-byte ipc.addr field in
> the stack-local struct ipcm_cookie. The same gap permits incompatible
> LSM_MAC and LSM_CGROUP replacements.
>
> Validate every replacement program against the link attach type before
> dispatching to the link-specific update operation. Keep the CGROUP_SKB
> CAP_NET_ADMIN check at initial attach so pinned or delegated link FDs can
> still be updated after a capability drop.
>
> Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")
> Acked-by: Leon Hwang <leon.hwang@linux.dev>
> Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
> v5:
> - Fold the link-create permission check call onto one line.
> v4: https://lore.kernel.org/r/20260831023918.1857658-1-sanghyun.park.cnu@gmail.com
> - Validate every program link update through the shared attach-type helper.
> - Fold the LSM attach-flavor check into that helper.
> - Keep the CGROUP_SKB permission check in the attach-only wrapper and rename
> the wrapper to describe that role.
> - Follow the preferred BPF multi-line comment style.
> - Add focused regression selftests as patch 2/2.
> v3: https://lore.kernel.org/r/20260821084726.3769957-2-sanghyun.park.cnu@gmail.com
> - Factor the CGROUP_SKB CAP_NET_ADMIN check into an attach-only helper.
> v2: https://lore.kernel.org/r/20260818061021.2551771-2-sanghyun.park.cnu@gmail.com
> - Extend validation from cgroup sock_addr programs to all cgroup program
> types, including exact LSM attach flavors.
> - Preserve legacy CGROUP_SKB ingress/egress replacement compatibility.
> - Keep the CGROUP_SKB CAP_NET_ADMIN check on attach, not link update.
> v1: https://lore.kernel.org/r/20260805052858.2390918-3-sanghyun.park.cnu@gmail.com
> ---
> kernel/bpf/syscall.c | 30 +++++++++++++++++++++---------
> 1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6874ba1424af..2bcd3f3a2a55 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -4481,14 +4481,9 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> case BPF_PROG_TYPE_SK_LOOKUP:
> + case BPF_PROG_TYPE_LSM:
> return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
> case BPF_PROG_TYPE_CGROUP_SKB:
> - if (!bpf_token_capable(prog->aux->token, CAP_NET_ADMIN))
> - /* cg-skb progs can be loaded by unpriv user.
> - * check permissions at attach time.
> - */
> - return -EPERM;
> -
> ptype = attach_type_to_prog_type(attach_type);
> if (prog->type != ptype)
> return -EINVAL;
> @@ -4542,6 +4537,20 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> }
> }
>
> +static int bpf_prog_attach_check_perm(const struct bpf_prog *prog,
> + enum bpf_attach_type attach_type)
> +{
> + /*
> + * CGROUP_SKB programs can be loaded by unprivileged users, so check
> + * permissions at attach time.
> + */
> + if (prog->type == BPF_PROG_TYPE_CGROUP_SKB &&
> + !bpf_token_capable(prog->aux->token, CAP_NET_ADMIN))
> + return -EPERM;
> +
> + return bpf_prog_attach_check_attach_type(prog, attach_type);
> +}
> +
> static bool is_cgroup_prog_type(enum bpf_prog_type ptype, enum bpf_attach_type atype,
> bool check_atype)
> {
> @@ -4606,7 +4615,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
> if (IS_ERR(prog))
> return PTR_ERR(prog);
>
> - if (bpf_prog_attach_check_attach_type(prog, attr->attach_type)) {
> + if (bpf_prog_attach_check_perm(prog, attr->attach_type)) {
> bpf_prog_put(prog);
> return -EINVAL;
> }
> @@ -5805,8 +5814,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
> if (IS_ERR(prog))
> return PTR_ERR(prog);
>
> - ret = bpf_prog_attach_check_attach_type(prog,
> - attr->link_create.attach_type);
> + ret = bpf_prog_attach_check_perm(prog, attr->link_create.attach_type);
> if (ret)
> goto out;
>
> @@ -5968,6 +5976,10 @@ static int link_update(union bpf_attr *attr)
> goto out_put_progs;
> }
>
> + ret = bpf_prog_attach_check_attach_type(new_prog, link->attach_type);
> + if (ret)
> + goto out_put_progs;
> +
> if (link->ops->update_prog)
> ret = link->ops->update_prog(link, new_prog, old_prog);
> else
> --
> 2.48.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v5 2/2] selftests/bpf: Cover attach type checks in link update
2026-09-01 1:47 ` [PATCH bpf-next v5 2/2] selftests/bpf: Cover attach type checks in " Sanghyun Park
2026-09-01 2:50 ` bot+bpf-ci
@ 2026-09-05 1:00 ` Emil Tsalapatis
2026-09-07 1:38 ` Sanghyun Park
1 sibling, 1 reply; 6+ messages in thread
From: Emil Tsalapatis @ 2026-09-05 1:00 UTC (permalink / raw)
To: Sanghyun Park
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, bpf, Leon Hwang,
John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song,
Jiri Olsa, Ihor Solodrai, Shuah Khan, Stanislav Fomichev,
Pu Lehui, Suchit Karunakaran, Xu Kuohai, linux-kernel,
linux-kselftest
On Mon, Aug 31, 2026 at 9:48 PM Sanghyun Park
<sanghyun.park.cnu@gmail.com> wrote:
>
> Pin down the two contracts changed by the companion patch. LINK_UPDATE
> validates a replacement program against the link's attach type, while
> CGROUP_SKB capability checks apply at LINK_CREATE and not LINK_UPDATE.
>
> Cover incompatible UDP4/UDP6 and LSM attach flavors, plus a CGROUP_SKB
> link update after dropping CAP_NET_ADMIN and CAP_SYS_ADMIN.
>
> Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
> ---
> v5:
> - Isolate attach and update coverage in separate subtests with their own setup.
> - Inline the one-use LSM program FD and use a descriptive link FD.
> - Explain the create-versus-update contracts covered by the tests.
> v4: https://lore.kernel.org/r/20260831023918.1857658-2-sanghyun.park.cnu@gmail.com
> ---
> .../selftests/bpf/prog_tests/cgroup_link.c | 81 ++++++++++++++++++-
> .../selftests/bpf/prog_tests/lsm_cgroup.c | 12 +++
> .../testing/selftests/bpf/progs/lsm_cgroup.c | 6 ++
> .../selftests/bpf/progs/test_cgroup_link.c | 14 +++-
> 4 files changed, 111 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
> index 15093a69510e..3b43c34917cc 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
>
> #include <test_progs.h>
> +#include "cap_helpers.h"
> #include "cgroup_helpers.h"
> #include "testing_helpers.h"
> #include "test_cgroup_link.skel.h"
> @@ -24,7 +25,77 @@ int ping_and_check(int exp_calls, int exp_alt_calls)
> return 0;
> }
>
> -void serial_test_cgroup_link(void)
> +static void test_cgroup_link_update(void)
> +{
> + const __u64 caps = (1ULL << CAP_NET_ADMIN) | (1ULL << CAP_SYS_ADMIN);
> + struct bpf_link *link = NULL;
> + __u64 saved_caps = 0;
> + int cg_fd = -1, err;
> +
> + skel = test_cgroup_link__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "skel_open_load"))
> + return;
> +
> + err = setup_cgroup_environment();
> + if (!ASSERT_OK(err, "cg_init"))
> + goto cleanup;
> +
> + cg_fd = create_and_get_cgroup("/cgroup_link_update");
> + if (!ASSERT_GE(cg_fd, 0, "cg_create"))
> + goto cleanup;
> +
> + err = join_cgroup("/cgroup_link_update");
> + if (!ASSERT_OK(err, "cg_join"))
> + goto cleanup;
> +
Wrt bot feedback: This is fine, the bot itself says it's more
consistent within the file to
use the same setup/teardown.
> + link = bpf_program__attach_cgroup(skel->progs.sendmsg4, cg_fd);
> + if (!ASSERT_OK_PTR(link, "attach_sendmsg4"))
> + goto cleanup;
> +
> + err = bpf_link__update_program(link, skel->progs.sendmsg6);
> + ASSERT_EQ(err, -EINVAL, "reject_sendmsg6_update");
> + bpf_link__destroy(link);
> + link = NULL;
> +
> + err = cap_disable_effective(caps, &saved_caps);
> + if (!ASSERT_OK(err, "drop_caps_for_attach"))
> + goto cleanup;
> +
> + link = bpf_program__attach_cgroup(skel->progs.egress, cg_fd);
> + if (!ASSERT_ERR_PTR(link, "attach_without_net_admin"))
> + goto cleanup;
> + err = libbpf_get_error(link);
> + link = NULL;
> + if (!ASSERT_EQ(err, -EPERM, "attach_err"))
> + goto cleanup;
> +
> + err = cap_enable_effective(saved_caps & caps, NULL);
> + if (!ASSERT_OK(err, "restore_caps_for_attach"))
> + goto cleanup;
> +
> + link = bpf_program__attach_cgroup(skel->progs.egress, cg_fd);
> + if (!ASSERT_OK_PTR(link, "attach_egress"))
> + goto cleanup;
> +
> + err = cap_disable_effective(caps, &saved_caps);
> + if (!ASSERT_OK(err, "drop_caps"))
> + goto cleanup;
> +
> + err = bpf_link__update_program(link, skel->progs.egress_alt);
> + ASSERT_OK(err, "update_without_net_admin");
> +
> +cleanup:
> + err = cap_enable_effective(saved_caps & caps, NULL);
> + ASSERT_OK(err, "restore_caps");
> + bpf_link__destroy(link);
> + if (cg_fd >= 0)
> + close(cg_fd);
> + cleanup_cgroup_environment();
> + test_cgroup_link__destroy(skel);
> + skel = NULL;
> +}
> +
> +static void test_cgroup_link_attach(void)
> {
> struct {
> const char *path;
> @@ -253,3 +324,11 @@ void serial_test_cgroup_link(void)
> }
> cleanup_cgroup_environment();
> }
> +
> +void serial_test_cgroup_link(void)
> +{
> + if (test__start_subtest("attach"))
> + test_cgroup_link_attach();
> + if (test__start_subtest("update"))
> + test_cgroup_link_update();
> +}
> diff --git a/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c b/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c
> index 41e867467f6c..0dd44dbb9c95 100644
> --- a/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c
> +++ b/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c
> @@ -74,6 +74,7 @@ static void test_lsm_cgroup_functional(void)
> int bind_prog_fd = -1;
> int bind_link_fd = -1;
> int clone_prog_fd = -1;
> + int socket_link_fd = -1;
> int err, fd, prio;
> socklen_t socklen;
>
> @@ -156,6 +157,17 @@ static void test_lsm_cgroup_functional(void)
> ASSERT_EQ(query_prog_cnt(cgroup_fd, "bpf_lsm_socket_bind"), 1, "prog count");
> ASSERT_EQ(query_prog_cnt(cgroup_fd, NULL), 4, "total prog count");
>
> + socket_link_fd =
> + bpf_link_create(bpf_program__fd(skel->progs.socket_first),
> + cgroup_fd, BPF_LSM_CGROUP, NULL);
> + if (!ASSERT_GE(socket_link_fd, 0, "link create socket_first"))
> + goto detach_cgroup;
> + err = bpf_link_update(socket_link_fd,
> + bpf_program__fd(skel->progs.socket_create_lsm),
> + NULL);
> + ASSERT_EQ(err, -EINVAL, "reject lsm_mac link update");
> + close(socket_link_fd);
> +
> /* Attach another instance of bind program to another cgroup.
> * This should trigger the reuse of the trampoline shim (two
> * programs attaching to the same btf_id).
> diff --git a/tools/testing/selftests/bpf/progs/lsm_cgroup.c b/tools/testing/selftests/bpf/progs/lsm_cgroup.c
> index 3bfa479104be..30727945cecc 100644
> --- a/tools/testing/selftests/bpf/progs/lsm_cgroup.c
> +++ b/tools/testing/selftests/bpf/progs/lsm_cgroup.c
> @@ -213,6 +213,12 @@ int BPF_PROG(socket_first, int family, int type, int protocol, int kern)
> return 0;
> }
>
> +SEC("lsm/socket_create")
> +int BPF_PROG(socket_create_lsm, int family, int type, int protocol, int kern)
> +{
> + return 0;
> +}
> +
> SEC("lsm_cgroup/socket_create")
> int BPF_PROG(socket_second, int family, int type, int protocol, int kern)
> {
> diff --git a/tools/testing/selftests/bpf/progs/test_cgroup_link.c b/tools/testing/selftests/bpf/progs/test_cgroup_link.c
> index 4faba88e45a5..a987744d129c 100644
> --- a/tools/testing/selftests/bpf/progs/test_cgroup_link.c
> +++ b/tools/testing/selftests/bpf/progs/test_cgroup_link.c
> @@ -20,5 +20,17 @@ int egress_alt(struct __sk_buff *skb)
> return 1;
> }
>
> -char _license[] SEC("license") = "GPL";
> +SEC("cgroup/sendmsg4")
> +int sendmsg4(struct bpf_sock_addr *ctx)
> +{
> + return 1;
> +}
>
> +SEC("cgroup/sendmsg6")
> +int sendmsg6(struct bpf_sock_addr *ctx)
> +{
> + ctx->msg_src_ip6[2] = 0;
Bot is right on this, this as it stands doesn't affect the tests. We
should remove it.
That being said, the test itself is fine so please just remove the line and
resend.
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
pw-bot: cr
> + return 1;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.48.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v5 2/2] selftests/bpf: Cover attach type checks in link update
2026-09-05 1:00 ` Emil Tsalapatis
@ 2026-09-07 1:38 ` Sanghyun Park
0 siblings, 0 replies; 6+ messages in thread
From: Sanghyun Park @ 2026-09-07 1:38 UTC (permalink / raw)
To: Emil Tsalapatis
Cc: Sanghyun Park, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, bpf,
Leon Hwang, John Fastabend, Martin KaFai Lau, Song Liu,
Yonghong Song, Jiri Olsa, Ihor Solodrai, Shuah Khan,
Stanislav Fomichev, Pu Lehui, Suchit Karunakaran, Xu Kuohai,
linux-kernel, linux-kselftest
Hi Emil,
Thanks for reviewing both patches. I'll come up with v6 with the things resolved.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-07 1:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-01 1:47 [PATCH bpf-next v5 1/2] bpf: Validate program attach type during link update Sanghyun Park
2026-09-01 1:47 ` [PATCH bpf-next v5 2/2] selftests/bpf: Cover attach type checks in " Sanghyun Park
2026-09-01 2:50 ` bot+bpf-ci
2026-09-05 1:00 ` Emil Tsalapatis
2026-09-07 1:38 ` Sanghyun Park
2026-09-05 0:55 ` [PATCH bpf-next v5 1/2] bpf: Validate program attach type during " Emil Tsalapatis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox