Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [PATCH bpf-next v4 1/2] bpf: Validate program attach type during link update
@ 2026-08-31  2:39 Sanghyun Park
  2026-08-31  2:39 ` [PATCH bpf-next v4 2/2] selftests/bpf: Cover attach type checks in " Sanghyun Park
  2026-08-31 14:48 ` [PATCH bpf-next v4 1/2] bpf: Validate program attach type during " Leon Hwang
  0 siblings, 2 replies; 5+ messages in thread
From: Sanghyun Park @ 2026-08-31  2:39 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,
	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")
Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
---
v4:
  - 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 | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6874ba1424af..84823149dffa 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,8 @@ 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 +5977,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] 5+ messages in thread

* [PATCH bpf-next v4 2/2] selftests/bpf: Cover attach type checks in link update
  2026-08-31  2:39 [PATCH bpf-next v4 1/2] bpf: Validate program attach type during link update Sanghyun Park
@ 2026-08-31  2:39 ` Sanghyun Park
  2026-08-31  3:49   ` bot+bpf-ci
  2026-08-31 14:50   ` Leon Hwang
  2026-08-31 14:48 ` [PATCH bpf-next v4 1/2] bpf: Validate program attach type during " Leon Hwang
  1 sibling, 2 replies; 5+ messages in thread
From: Sanghyun Park @ 2026-08-31  2:39 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,
	linux-kernel, linux-kselftest

Add regression coverage for link update compatibility. Verify that a UDP6
sock_addr program cannot replace a UDP4 program and that an LSM_MAC
program cannot replace an LSM_CGROUP program.

Also verify that CGROUP_SKB attachment still requires CAP_NET_ADMIN, while
an existing link can be updated after dropping CAP_NET_ADMIN and
CAP_SYS_ADMIN.

Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
---
 .../selftests/bpf/prog_tests/cgroup_link.c    | 51 +++++++++++++++++++
 .../selftests/bpf/prog_tests/lsm_cgroup.c     | 10 ++++
 .../testing/selftests/bpf/progs/lsm_cgroup.c  |  6 +++
 .../selftests/bpf/progs/test_cgroup_link.c    | 14 ++++-
 4 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
index 15093a69510e..b3219314596e 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,6 +25,55 @@ int ping_and_check(int exp_calls, int exp_alt_calls)
 	return 0;
 }
 
+static void test_cgroup_link_update(int cg_fd)
+{
+	const __u64 caps = (1ULL << CAP_NET_ADMIN) | (1ULL << CAP_SYS_ADMIN);
+	struct bpf_link *link = NULL;
+	__u64 saved_caps = 0;
+	int err;
+
+	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);
+}
+
 void serial_test_cgroup_link(void)
 {
 	struct {
@@ -61,6 +111,7 @@ void serial_test_cgroup_link(void)
 	err = join_cgroup(cgs[last_cg].path);
 	if (CHECK(err, "cg_join", "fail: %d\n", err))
 		goto cleanup;
+	test_cgroup_link_update(cgs[last_cg].fd);
 
 	for (i = 0; i < cg_nr; i++) {
 		links[i] = bpf_program__attach_cgroup(skel->progs.egress,
diff --git a/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c b/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c
index 41e867467f6c..85b0af4f486a 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 mac_prog_fd;
 	int err, fd, prio;
 	socklen_t socklen;
 
@@ -156,6 +157,15 @@ 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");
 
+	fd = bpf_link_create(bpf_program__fd(skel->progs.socket_first),
+			     cgroup_fd, BPF_LSM_CGROUP, NULL);
+	if (!ASSERT_GE(fd, 0, "link create socket_first"))
+		goto detach_cgroup;
+	mac_prog_fd = bpf_program__fd(skel->progs.socket_create_lsm);
+	err = bpf_link_update(fd, mac_prog_fd, NULL);
+	ASSERT_EQ(err, -EINVAL, "reject lsm_mac link update");
+	close(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] 5+ messages in thread

* Re: [PATCH bpf-next v4 2/2] selftests/bpf: Cover attach type checks in link update
  2026-08-31  2:39 ` [PATCH bpf-next v4 2/2] selftests/bpf: Cover attach type checks in " Sanghyun Park
@ 2026-08-31  3:49   ` bot+bpf-ci
  2026-08-31 14:50   ` Leon Hwang
  1 sibling, 0 replies; 5+ messages in thread
From: bot+bpf-ci @ 2026-08-31  3:49 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,
	linux-kernel, linux-kselftest, ast, andrii, daniel, martin.lau,
	eddyz87, yonghong.song, mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 2582 bytes --]

> Add regression coverage for link update compatibility. Verify that a UDP6
> sock_addr program cannot replace a UDP4 program and that an LSM_MAC
> program cannot replace an LSM_CGROUP program.
> Also verify that CGROUP_SKB attachment still requires CAP_NET_ADMIN, while
> an existing link can be updated after dropping CAP_NET_ADMIN and
> CAP_SYS_ADMIN.

This isn't a bug, but could the changelog say why this coverage is needed,
rather than restating what each assertion does? The tests appear to pin
down attach-type validation in link update (which didn't exist before the
companion patch) and the permission-check split for CGROUP_SKB (capability
at LINK_CREATE but not 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 41e867467f6c6..85b0af4f486af 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 mac_prog_fd;
>  	int err, fd, prio;
>  	socklen_t socklen;
>
> @@ -156,6 +157,15 @@ 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");
>
> +	fd = bpf_link_create(bpf_program__fd(skel->progs.socket_first),
> +			     cgroup_fd, BPF_LSM_CGROUP, NULL);
> +	if (!ASSERT_GE(fd, 0, "link create socket_first"))
> +		goto detach_cgroup;
> +	mac_prog_fd = bpf_program__fd(skel->progs.socket_create_lsm);
> +	err = bpf_link_update(fd, mac_prog_fd, NULL);
> +	ASSERT_EQ(err, -EINVAL, "reject lsm_mac link update");
> +	close(fd);
> +

This isn't a bug, but would it read more consistently to inline
bpf_program__fd(skel->progs.socket_create_lsm) into the bpf_link_update()
call? Every other '*_prog_fd' local in this function's declaration block is
initialised to -1, while mac_prog_fd is uninitialised and used exactly once
on the line after assignment; the bpf_link_create() call twelve lines above
already inlines the equivalent expression. Also, the new link fd is held in
the generic 'fd' variable, whereas the link created earlier uses the
descriptive 'bind_link_fd'.


---
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/33352119890

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next v4 1/2] bpf: Validate program attach type during link update
  2026-08-31  2:39 [PATCH bpf-next v4 1/2] bpf: Validate program attach type during link update Sanghyun Park
  2026-08-31  2:39 ` [PATCH bpf-next v4 2/2] selftests/bpf: Cover attach type checks in " Sanghyun Park
@ 2026-08-31 14:48 ` Leon Hwang
  1 sibling, 0 replies; 5+ messages in thread
From: Leon Hwang @ 2026-08-31 14:48 UTC (permalink / raw)
  To: Sanghyun Park, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, bpf
  Cc: John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jiri Olsa, Emil Tsalapatis, Ihor Solodrai, Shuah Khan,
	Stanislav Fomichev, Pu Lehui, linux-kernel, linux-kselftest

On 2026/8/31 10:39, Sanghyun Park 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")
> Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>

One nit below, others lgtm:

Acked-by: Leon Hwang <leon.hwang@linux.dev>

> ---
> v4:
>   - 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 | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6874ba1424af..84823149dffa 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,8 @@ 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);

NIT: fold into one line. 100 chars are allowed per line.

Thanks,
Leon

>  	if (ret)
>  		goto out;
>  
> @@ -5968,6 +5977,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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next v4 2/2] selftests/bpf: Cover attach type checks in link update
  2026-08-31  2:39 ` [PATCH bpf-next v4 2/2] selftests/bpf: Cover attach type checks in " Sanghyun Park
  2026-08-31  3:49   ` bot+bpf-ci
@ 2026-08-31 14:50   ` Leon Hwang
  1 sibling, 0 replies; 5+ messages in thread
From: Leon Hwang @ 2026-08-31 14:50 UTC (permalink / raw)
  To: Sanghyun Park, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, bpf
  Cc: John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jiri Olsa, Emil Tsalapatis, Ihor Solodrai, Shuah Khan,
	Stanislav Fomichev, Pu Lehui, linux-kernel, linux-kselftest

On 2026/8/31 10:39, Sanghyun Park wrote:
> Add regression coverage for link update compatibility. Verify that a UDP6
> sock_addr program cannot replace a UDP4 program and that an LSM_MAC
> program cannot replace an LSM_CGROUP program.
> 
> Also verify that CGROUP_SKB attachment still requires CAP_NET_ADMIN, while
> an existing link can be updated after dropping CAP_NET_ADMIN and
> CAP_SYS_ADMIN.
> 
> Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
> ---
>  .../selftests/bpf/prog_tests/cgroup_link.c    | 51 +++++++++++++++++++
>  .../selftests/bpf/prog_tests/lsm_cgroup.c     | 10 ++++
>  .../testing/selftests/bpf/progs/lsm_cgroup.c  |  6 +++
>  .../selftests/bpf/progs/test_cgroup_link.c    | 14 ++++-
>  4 files changed, 80 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
> index 15093a69510e..b3219314596e 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,6 +25,55 @@ int ping_and_check(int exp_calls, int exp_alt_calls)
>  	return 0;
>  }
>  
> +static void test_cgroup_link_update(int cg_fd)
> +{
> +	const __u64 caps = (1ULL << CAP_NET_ADMIN) | (1ULL << CAP_SYS_ADMIN);
> +	struct bpf_link *link = NULL;
> +	__u64 saved_caps = 0;
> +	int err;
> +
> +	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);
> +}
> +
>  void serial_test_cgroup_link(void)
>  {
>  	struct {
> @@ -61,6 +111,7 @@ void serial_test_cgroup_link(void)
>  	err = join_cgroup(cgs[last_cg].path);
>  	if (CHECK(err, "cg_join", "fail: %d\n", err))
>  		goto cleanup;
> +	test_cgroup_link_update(cgs[last_cg].fd);

imo, it is not a good idea to embed test_cgroup_link_update() here.

Better to add it as a subtest with its own env setup/cleanup:

static void test_cgroup_link_attach(void)
{
	/* previous serial_test_cgroup_link() function body */
}

static void test_cgroup_link_update(void)
{
	/* setup its own skel and cg env */

	/* test link_update */

	/* cleanup the skel and cg env */
}

void serial_test_cgroup_link(void)
{
	if (test__start_subtest("attach"))
		test_cgroup_link_attach();
	if (test__start_subtest("update"))
		test_cgroup_link_update();
}

>  
>  	for (i = 0; i < cg_nr; i++) {
>  		links[i] = bpf_program__attach_cgroup(skel->progs.egress,
> diff --git a/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c b/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c
> index 41e867467f6c..85b0af4f486a 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 mac_prog_fd;
>  	int err, fd, prio;
>  	socklen_t socklen;
>  
> @@ -156,6 +157,15 @@ 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");
>  
> +	fd = bpf_link_create(bpf_program__fd(skel->progs.socket_first),
> +			     cgroup_fd, BPF_LSM_CGROUP, NULL);
> +	if (!ASSERT_GE(fd, 0, "link create socket_first"))
> +		goto detach_cgroup;
> +	mac_prog_fd = bpf_program__fd(skel->progs.socket_create_lsm);
> +	err = bpf_link_update(fd, mac_prog_fd, NULL);

Rewrite to bpf_link_update(fd,
bpf_program__fd(skel->progs.socket_create_lsm), NULL).

Then, the 'mac_prog_fd' can be dropped.

Thanks,
Leon

> +	ASSERT_EQ(err, -EINVAL, "reject lsm_mac link update");> +	close(fd);
> [...]


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-31 14:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31  2:39 [PATCH bpf-next v4 1/2] bpf: Validate program attach type during link update Sanghyun Park
2026-08-31  2:39 ` [PATCH bpf-next v4 2/2] selftests/bpf: Cover attach type checks in " Sanghyun Park
2026-08-31  3:49   ` bot+bpf-ci
2026-08-31 14:50   ` Leon Hwang
2026-08-31 14:48 ` [PATCH bpf-next v4 1/2] bpf: Validate program attach type during " Leon Hwang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox