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@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxx
> >   - 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@xxxxxxxxx
> > ---
> >  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;
> > [...]