On Mon, Aug 31, 2026 at 9:48 PM Sanghyun Park
<[email protected]> 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 <[email protected]>
> ---
> 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/[email protected]
> ---
> .../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 <[email protected]>
pw-bot: cr
> + return 1;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.48.1
