[PATCH bpf-next v3] bpf: Fix stack out-of-bounds write in cgroup link update

[PATCH bpf-next v3] bpf: Fix stack out-of-bounds write in cgroup link update

[Sanghyun Park]

The cgroup link update path checks only the program type. Several cgroup
hooks share a type while using different runtime contexts or verifier
contracts. A UDP6 sock_addr program can therefore replace a UDP4 program
and write beyond the four-byte ipc.addr context into adjacent fields of
the stack-local struct ipcm_cookie. The same omission lets an LSM_MAC
program replace an LSM_CGROUP program despite the incompatible return
semantics.

Validate replacement programs against the link attach type. Use the
existing per-type rules where applicable, and compare LSM
expected_attach_type explicitly because both flavors share
BPF_PROG_TYPE_LSM. Preserve legacy non-enforcing CGROUP_SKB
ingress/egress updates.

CGROUP_SKB programs do not require CAP_NET_ADMIN when loaded. That
permission is checked when the program is attached. Once the link exists,
updates are controlled through its FD, so BPF_LINK_UPDATE does not check
CAP_NET_ADMIN again. Keep this behavior and only validate the attach type
during link update.

Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")
Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
---
v3:
  - 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 | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6874ba1424af05..dfea337ff25ea5 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4483,12 +4483,6 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
 	case BPF_PROG_TYPE_SK_LOOKUP:
 		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 +4536,19 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
 	}
 }
 
+static int bpf_prog_attach_check(const struct bpf_prog *prog,
+				 enum bpf_attach_type attach_type)
+{
+	if (prog->type == BPF_PROG_TYPE_CGROUP_SKB &&
+	    !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;
+
+	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 +4613,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(prog, attr->attach_type)) {
 		bpf_prog_put(prog);
 		return -EINVAL;
 	}
@@ -5805,8 +5812,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(prog, attr->link_create.attach_type);
 	if (ret)
 		goto out;
 
@@ -5931,6 +5937,7 @@ static int link_update(union bpf_attr *attr)
 {
 	struct bpf_prog *old_prog = NULL, *new_prog;
 	struct bpf_link *link;
+	enum bpf_attach_type atype;
 	u32 flags;
 	int ret;
 
@@ -5968,6 +5975,22 @@ static int link_update(union bpf_attr *attr)
 		goto out_put_progs;
 	}
 
+	if (link->type == BPF_LINK_TYPE_CGROUP) {
+		atype = link->attach_type;
+		/*
+		 * BPF_LSM_MAC and BPF_LSM_CGROUP share BPF_PROG_TYPE_LSM, so
+		 * the helper's default prog-type check cannot distinguish them.
+		 */
+		if (new_prog->type == BPF_PROG_TYPE_LSM &&
+		    new_prog->expected_attach_type != atype)
+			ret = -EINVAL;
+		else
+			ret = bpf_prog_attach_check_attach_type(new_prog,
+								atype);
+		if (ret)
+			goto out_put_progs;
+	}
+
 	if (link->ops->update_prog)
 		ret = link->ops->update_prog(link, new_prog, old_prog);
 	else

Re: [PATCH bpf-next v3] bpf: Fix stack out-of-bounds write in cgroup link update

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6874ba1424af..dfea337ff25e 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c

[ ... ]

> @@ -4542,6 +4536,19 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
>  	}
>  }
>
> +static int bpf_prog_attach_check(const struct bpf_prog *prog,
> +				 enum bpf_attach_type attach_type)
> +{
> +	if (prog->type == BPF_PROG_TYPE_CGROUP_SKB &&
> +	    !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;
> +
> +	return bpf_prog_attach_check_attach_type(prog, attach_type);
> +}
> +

This isn't a bug, but would a name that mentions the permission gate
read more clearly next to bpf_prog_attach_check_attach_type()?

The new helper bpf_prog_attach_check() sits near
bpf_prog_attach_check_attach_type() in the same file, and the two names
differ only by a suffix while the security-relevant difference is which
one enforces CAP_NET_ADMIN.

link_update() deliberately calls the longer-named, non-enforcing one
(bpf_prog_attach_check_attach_type), and bpf_prog_attach() plus
link_create() call the shorter, enforcing one (bpf_prog_attach_check).
A name like bpf_prog_attach_check_perm would make the choice at each of
the three call sites self-evident.

[ ... ]


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

Re: [PATCH bpf-next v3] bpf: Fix stack out-of-bounds write in cgroup link update

[Leon Hwang]

On 21/8/26 16:47, Sanghyun Park wrote:
> The cgroup link update path checks only the program type. Several cgroup
> hooks share a type while using different runtime contexts or verifier
> contracts. A UDP6 sock_addr program can therefore replace a UDP4 program
> and write beyond the four-byte ipc.addr context into adjacent fields of
> the stack-local struct ipcm_cookie. The same omission lets an LSM_MAC
> program replace an LSM_CGROUP program despite the incompatible return
> semantics.
> 
> Validate replacement programs against the link attach type. Use the
> existing per-type rules where applicable, and compare LSM
> expected_attach_type explicitly because both flavors share
> BPF_PROG_TYPE_LSM. Preserve legacy non-enforcing CGROUP_SKB
> ingress/egress updates.
> 
> CGROUP_SKB programs do not require CAP_NET_ADMIN when loaded. That
> permission is checked when the program is attached. Once the link exists,
> updates are controlled through its FD, so BPF_LINK_UPDATE does not check
> CAP_NET_ADMIN again. Keep this behavior and only validate the attach type
> during link update.

Any issue of checking CAP_NET_ADMIN for BPF_LINK_UPDATE?

If no, checking CAP_NET_ADMIN for BPF_LINK_UPDATE looks okay.

> 
> Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")
> Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
> ---
> v3:
>   - 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 | 41 ++++++++++++++++++++++++++++++++---------
>  1 file changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6874ba1424af05..dfea337ff25ea5 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -4483,12 +4483,6 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
>  	case BPF_PROG_TYPE_SK_LOOKUP:
+	case BPF_PROG_TYPE_LSM:

Add a 'case' here instead of checking it in link_update()?

And, a selftest is needed to verify the issue and the fix.

Thanks,
Leon

>  		return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
> [...]

Re: [PATCH bpf-next v3] bpf: Fix stack out-of-bounds write in cgroup link update

[Leon Hwang]

On 2026/8/21 16:47, Sanghyun Park wrote:
[...]
>  
> @@ -5968,6 +5975,22 @@ static int link_update(union bpf_attr *attr)
>  		goto out_put_progs;
>  	}
>  
> +	if (link->type == BPF_LINK_TYPE_CGROUP) {

Furthermore, I think this link type check can be dropped. Then,
link_update() will keep the same bpf_prog_attach_check_attach_type()
with link_create().

WDYT?

Thanks,
Leon

> +		atype = link->attach_type;
> +		/*
> +		 * BPF_LSM_MAC and BPF_LSM_CGROUP share BPF_PROG_TYPE_LSM, so
> +		 * the helper's default prog-type check cannot distinguish them.
> +		 */
> +		if (new_prog->type == BPF_PROG_TYPE_LSM &&
> +		    new_prog->expected_attach_type != atype)
> +			ret = -EINVAL;
> +		else
> +			ret = bpf_prog_attach_check_attach_type(new_prog,
> +								atype);
> +		if (ret)
> +			goto out_put_progs;
> +	}
> +
>  	if (link->ops->update_prog)
>  		ret = link->ops->update_prog(link, new_prog, old_prog);
>  	else

Re: [PATCH bpf-next v3] bpf: Fix stack out-of-bounds write in cgroup link update

[Andrii Nakryiko]

On Fri, Aug 21, 2026 at 3:26 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>
> On 21/8/26 16:47, Sanghyun Park wrote:
> > The cgroup link update path checks only the program type. Several cgroup
> > hooks share a type while using different runtime contexts or verifier
> > contracts. A UDP6 sock_addr program can therefore replace a UDP4 program
> > and write beyond the four-byte ipc.addr context into adjacent fields of
> > the stack-local struct ipcm_cookie. The same omission lets an LSM_MAC
> > program replace an LSM_CGROUP program despite the incompatible return
> > semantics.

Patch subject "bpf: Fix stack out-of-bounds write in cgroup link
update" is absolutely out-of-sync with the contents of the patch, can
you please update it to reflect what's going on in a more direct way?

> >
> > Validate replacement programs against the link attach type. Use the
> > existing per-type rules where applicable, and compare LSM
> > expected_attach_type explicitly because both flavors share
> > BPF_PROG_TYPE_LSM. Preserve legacy non-enforcing CGROUP_SKB
> > ingress/egress updates.
> >
> > CGROUP_SKB programs do not require CAP_NET_ADMIN when loaded. That
> > permission is checked when the program is attached. Once the link exists,
> > updates are controlled through its FD, so BPF_LINK_UPDATE does not check
> > CAP_NET_ADMIN again. Keep this behavior and only validate the attach type
> > during link update.
>
> Any issue of checking CAP_NET_ADMIN for BPF_LINK_UPDATE?
>
> If no, checking CAP_NET_ADMIN for BPF_LINK_UPDATE looks okay.
>

Why do we need extra capability checks during LINK_UPDATE if we
already performed the check during LINK_CREATE? If you pass link fd to
some process that doesn't have CAP_NET_ADMIN, but has link fd and
another validated prog fd, they should be able to update the
underlying program without extra checks, IMO.

pw-bot: cr

> >
> > Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")
> > Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
> > ---
> > v3:
> >   - 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 | 41 ++++++++++++++++++++++++++++++++---------
> >  1 file changed, 32 insertions(+), 9 deletions(-)
> >
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 6874ba1424af05..dfea337ff25ea5 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -4483,12 +4483,6 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> >       case BPF_PROG_TYPE_SK_LOOKUP:
> +       case BPF_PROG_TYPE_LSM:
>
> Add a 'case' here instead of checking it in link_update()?
>
> And, a selftest is needed to verify the issue and the fix.
>
> Thanks,
> Leon
>
> >               return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
> > [...]