[PATCH bpf-next v6 1/2] bpf: Validate program attach type during link update

[PATCH bpf-next v6 1/2] bpf: Validate program attach type during link update

[Sanghyun Park]

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>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
---
v6:
  - No code changes. Add Reviewed-by tag.
v5: https://lore.kernel.org/r/20260901014713.3502900-2-sanghyun.park.cnu@gmail.com
  - 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

[PATCH bpf-next v6 2/2] selftests/bpf: Cover attach type checks in link update

[Sanghyun Park]

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.

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
---
v6:
  - Remove the unused msg_src_ip6 store from sendmsg6().
  - Add Reviewed-by tag.
v5: https://lore.kernel.org/r/20260901014713.3502900-4-sanghyun.park.cnu@gmail.com
  - 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
---
 .../testing/selftests/bpf/prog_tests/cgroup_link.c | 81 +++++++++++++++++++++-
 .../testing/selftests/bpf/prog_tests/lsm_cgroup.c  | 12 ++++
 tools/testing/selftests/bpf/progs/lsm_cgroup.c     |  6 ++
 .../testing/selftests/bpf/progs/test_cgroup_link.c | 13 +++-
 4 files changed, 110 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..107429638f54 100644
--- a/tools/testing/selftests/bpf/progs/test_cgroup_link.c
+++ b/tools/testing/selftests/bpf/progs/test_cgroup_link.c
@@ -20,5 +20,16 @@ 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)
+{
+	return 1;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.48.1

Re: [PATCH bpf-next v6 1/2] bpf: Validate program attach type during link update

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index c7bc9ba9b331..b394a37eecb4 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;

Does adding BPF_PROG_TYPE_LSM to this arm also relax the check on the
BPF_LINK_CREATE path?

Before this patch, BPF_PROG_TYPE_LSM fell into the default arm:

	default:
		ptype = attach_type_to_prog_type(attach_type);
		if (ptype == BPF_PROG_TYPE_UNSPEC || ptype != prog->type)
			return -EINVAL;
		return 0;

Since attach_type_to_prog_type() returns BPF_PROG_TYPE_LSM only for
BPF_LSM_MAC and BPF_LSM_CGROUP (kernel/bpf/syscall.c:4457, 4463), an LSM
program could only ever be accepted for those two attach types.

The new arm drops that constraint and accepts any attach_type as long as it
equals prog->expected_attach_type. The other members of that arm are safe
because bpf_prog_load_check_attach() restricts their expected_attach_type
to a set that all maps back to the same prog type. BPF_PROG_TYPE_LSM is the
one type in that arm with no such restriction: bpf_prog_load_check_attach()
(kernel/bpf/syscall.c:2766 onwards) has no LSM case and falls into
`default: return 0`, so prog->expected_attach_type is entirely
userspace-controlled for LSM programs.

An LSM program can be loaded with expected_attach_type == BPF_TRACE_ITER
and attach_btf_id pointing at a bpf_iter_* function, because
check_attach_btf_id() takes the ITER shortcut before the LSM-specific
validation (kernel/bpf/verifier.c:20810-20819):

	if (prog->expected_attach_type == BPF_TRACE_RAW_TP) {
		prog->aux->attach_btf_trace = true;
		return 0;
	} else if (prog->expected_attach_type == BPF_TRACE_ITER) {
		return bpf_iter_prog_supported(prog);
	}

	if (prog->type == BPF_PROG_TYPE_LSM) {
		ret = bpf_lsm_verify_prog(&env->log, prog);

bpf_check_attach_target()'s `case BPF_TRACE_ITER:` (kernel/bpf/verifier.c:
20551) and bpf_iter_prog_supported() (kernel/bpf/bpf_iter.c:337) both
switch on expected_attach_type only and never look at prog->type, so the
load succeeds and bpf_lsm_verify_prog() never runs.

Post-patch, BPF_LINK_CREATE with attach_type == BPF_TRACE_ITER then
succeeds for that program. link_create() fetches the prog with plain
bpf_prog_get() (no type filter, kernel/bpf/syscall.c:5813), so
bpf_prog_attach_check_perm() is the only prog_type/attach_type gate; it now
returns 0, and the `case BPF_PROG_TYPE_LSM: case BPF_PROG_TYPE_TRACING:`
arm dispatches to bpf_iter_link_attach() (kernel/bpf/syscall.c:5847), which
also performs no prog->type check. The result is a live BPF_LINK_TYPE_ITER
link holding a BPF_PROG_TYPE_LSM program. Pre-patch this returned -EINVAL.

BPF_PROG_ATTACH is not affected, because bpf_prog_attach() still calls
bpf_prog_get_type(attr->attach_bpf_fd,
attach_type_to_prog_type(attr->attach_type)) before the check. BPF_TRACE_ITER
is also the only newly-permitted attach type that lacks a downstream
prog->type guard: RAW_TP, FENTRY, FEXIT, FSESSION and MODIFY_RETURN all
funnel into bpf_tracing_prog_attach(), which rejects
`prog->expected_attach_type != BPF_LSM_MAC` for LSM programs
(kernel/bpf/syscall.c:3663), and the *_MULTI types cannot be loaded for
LSM at all.

Could this be tightened to keep the mapping constraint for LSM?  For
example, handle BPF_PROG_TYPE_LSM like BPF_PROG_TYPE_CGROUP_SKB does:
verify attach_type_to_prog_type(attach_type) == prog->type first, then
compare expected_attach_type.

>  	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;

[ ... ]


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

Re: [PATCH bpf-next v6 1/2] bpf: Validate program attach type during link update

[Leon Hwang]

On 2026/9/7 10:36, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index c7bc9ba9b331..b394a37eecb4 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;
> 
> Does adding BPF_PROG_TYPE_LSM to this arm also relax the check on the
> BPF_LINK_CREATE path?
> 
> Before this patch, BPF_PROG_TYPE_LSM fell into the default arm:
> 
> 	default:
> 		ptype = attach_type_to_prog_type(attach_type);
> 		if (ptype == BPF_PROG_TYPE_UNSPEC || ptype != prog->type)
> 			return -EINVAL;
> 		return 0;
> 
> Since attach_type_to_prog_type() returns BPF_PROG_TYPE_LSM only for
> BPF_LSM_MAC and BPF_LSM_CGROUP (kernel/bpf/syscall.c:4457, 4463), an LSM
> program could only ever be accepted for those two attach types.
> 
> The new arm drops that constraint and accepts any attach_type as long as it
> equals prog->expected_attach_type. The other members of that arm are safe
> because bpf_prog_load_check_attach() restricts their expected_attach_type
> to a set that all maps back to the same prog type. BPF_PROG_TYPE_LSM is the
> one type in that arm with no such restriction: bpf_prog_load_check_attach()
> (kernel/bpf/syscall.c:2766 onwards) has no LSM case and falls into
> `default: return 0`, so prog->expected_attach_type is entirely
> userspace-controlled for LSM programs.


Good catch.

Since there's no expected_attach_type check for lsm prog, a lsm prog can
have BPF_TRACE_ITER expected_attach_type, and creates a lsm link via
BPF_LINK_CREATE with this patch. Such lsm prog cannot create a lsm link
via BPF_LINK_CREATE without this patch.

The new case BPF_PROG_TYPE_LSM in bpf_prog_attach_check_attach_type()
relaxes the prog->type check when adding expected_attach_type check.

I think we can add expected_attach_type check for lsm prog in
bpf_prog_load_check_attach(). See below diff. The diff will restrict a
lsm prog with these two expected_attach_type, BPF_LSM_MAC and
BPF_LSM_CGROUP.

Thanks,
Leon

---

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6874ba1424af..4b56e82ff3b9 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2830,6 +2830,16 @@ bpf_prog_load_check_attach(enum bpf_prog_type
prog_type,
 		if (expected_attach_type == BPF_NETFILTER)
 			return 0;
 		return -EINVAL;
+	case BPF_PROG_TYPE_LSM:
+		switch (expected_attach_type) {
+		case BPF_LSM_MAC:
+		case BPF_LSM_CGROUP:
+			return 0;
+		default:
+			return -EINVAL;
+		}
 	case BPF_PROG_TYPE_SYSCALL:
 	case BPF_PROG_TYPE_EXT:
 		if (expected_attach_type)

> [...]

Re: [PATCH bpf-next v6 1/2] bpf: Validate program attach type during link update

[Emil Tsalapatis]

On Mon, Sep 7, 2026 at 3:08 PM Leon Hwang <leon.hwang@linux.dev> wrote:
>
> On 2026/9/7 10:36, bot+bpf-ci@kernel.org wrote:
> >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> >> index c7bc9ba9b331..b394a37eecb4 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;
> >
> > Does adding BPF_PROG_TYPE_LSM to this arm also relax the check on the
> > BPF_LINK_CREATE path?
> >
> > Before this patch, BPF_PROG_TYPE_LSM fell into the default arm:
> >
> >       default:
> >               ptype = attach_type_to_prog_type(attach_type);
> >               if (ptype == BPF_PROG_TYPE_UNSPEC || ptype != prog->type)
> >                       return -EINVAL;
> >               return 0;
> >
> > Since attach_type_to_prog_type() returns BPF_PROG_TYPE_LSM only for
> > BPF_LSM_MAC and BPF_LSM_CGROUP (kernel/bpf/syscall.c:4457, 4463), an LSM
> > program could only ever be accepted for those two attach types.
> >
> > The new arm drops that constraint and accepts any attach_type as long as it
> > equals prog->expected_attach_type. The other members of that arm are safe
> > because bpf_prog_load_check_attach() restricts their expected_attach_type
> > to a set that all maps back to the same prog type. BPF_PROG_TYPE_LSM is the
> > one type in that arm with no such restriction: bpf_prog_load_check_attach()
> > (kernel/bpf/syscall.c:2766 onwards) has no LSM case and falls into
> > `default: return 0`, so prog->expected_attach_type is entirely
> > userspace-controlled for LSM programs.
>
>
> Good catch.
>
> Since there's no expected_attach_type check for lsm prog, a lsm prog can
> have BPF_TRACE_ITER expected_attach_type, and creates a lsm link via
> BPF_LINK_CREATE with this patch. Such lsm prog cannot create a lsm link
> via BPF_LINK_CREATE without this patch.
>
> The new case BPF_PROG_TYPE_LSM in bpf_prog_attach_check_attach_type()
> relaxes the prog->type check when adding expected_attach_type check.
>
> I think we can add expected_attach_type check for lsm prog in
> bpf_prog_load_check_attach(). See below diff. The diff will restrict a
> lsm prog with these two expected_attach_type, BPF_LSM_MAC and
> BPF_LSM_CGROUP.
>

+1, skipping LSM progs in bpf_prog_load_check_attach() is counterintuitive
and the reason why we missed this edge case in the first place. It's
better to add
the missing checks than adjust the refactoring.

pw-bot: cr

> Thanks,
> Leon
>
> ---
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6874ba1424af..4b56e82ff3b9 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2830,6 +2830,16 @@ bpf_prog_load_check_attach(enum bpf_prog_type
> prog_type,
>                 if (expected_attach_type == BPF_NETFILTER)
>                         return 0;
>                 return -EINVAL;
> +       case BPF_PROG_TYPE_LSM:
> +               switch (expected_attach_type) {
> +               case BPF_LSM_MAC:
> +               case BPF_LSM_CGROUP:
> +                       return 0;
> +               default:
> +                       return -EINVAL;
> +               }
>         case BPF_PROG_TYPE_SYSCALL:
>         case BPF_PROG_TYPE_EXT:
>                 if (expected_attach_type)
>
> > [...]
>