bpf: cgroup: fix null-ptr-deref in __cgroup_bpf_attach

[PATCH] bpf: cgroup: fix null-ptr-deref in __cgroup_bpf_attach

Posted by Liu Jing 3 weeks, 3 days ago

In __cgroup_bpf_attach(), the expression "prog ? : link->link.prog"
dereferences link before validating that link is non-NULL. If both
prog and link are NULL (a contract violation by the caller), this
causes a null pointer dereference.

Add a safe ternary fallback and a NULL check for new_prog before
it is used further.

Signed-off-by: Liu Jing <liujing@cmss.chinamobile.com>
---
 kernel/bpf/cgroup.c | 4 +++-
 1 file changed, 3 insertion(+), 1 deletion(-)

--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -814,7 +814,7 @@
 	struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
 	struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
 	struct bpf_cgroup_storage *old_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
-	struct bpf_prog *new_prog = prog ? : link->link.prog;
+	struct bpf_prog *new_prog = prog ? : (link ? link->link.prog : NULL);
 	enum cgroup_bpf_attach_type atype;
 	u32 old_flags, old_pl_flags;
 	struct bpf_prog_list *pl;
@@ -833,6 +833,8 @@
 		return -EINVAL;
 	if (!!replace_prog != !!(flags & BPF_F_REPLACE))
 		/* replace_prog implies BPF_F_REPLACE, and vice versa */
+		return -EINVAL;
+	if (!new_prog)
 		return -EINVAL;
 
 	atype = bpf_cgroup_atype_find(type, new_prog->aux->attach_btf_id);

--
2.43.0

Re: [PATCH] bpf: cgroup: fix null-ptr-deref in __cgroup_bpf_attach

Posted by bot+bpf-ci@kernel.org 3 weeks, 3 days ago

>     bpf: cgroup: fix null-ptr-deref in __cgroup_bpf_attach
>
>     In __cgroup_bpf_attach(), the expression "prog ? : link->link.prog"
>     dereferences link before validating that link is non-NULL. If both
>     prog and link are NULL (a contract violation by the caller), this
>     causes a null pointer dereference.
>
>     Add a safe ternary fallback and a NULL check for new_prog before
>     it is used further.
>
>     Signed-off-by: Liu Jing <liujing@cmss.chinamobile.com>

This looks like a bug fix. Should this include:

  Fixes: af6eea57437a ("bpf: Implement bpf_link-based cgroup BPF program attachment")


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

Re: [PATCH] bpf: cgroup: fix null-ptr-deref in __cgroup_bpf_attach

Posted by Jiayuan Chen 3 weeks, 3 days ago

on 9/2/26 5:02 PM, Liu Jing wrote:
> In __cgroup_bpf_attach(), the expression "prog ? : link->link.prog"
> dereferences link before validating that link is non-NULL. If both
> prog and link are NULL (a contract violation by the caller), this
> causes a null pointer dereference.


Both being NULL is unreachable, so the new check is dead code.

It's meaningless.


>
> Add a safe ternary fallback and a NULL check for new_prog before
> it is used further.
>
> Signed-off-by: Liu Jing <liujing@cmss.chinamobile.com>
> ---
>   kernel/bpf/cgroup.c | 4 +++-
>   1 file changed, 3 insertion(+), 1 deletion(-)
>
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -814,7 +814,7 @@
>   	struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
>   	struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
>   	struct bpf_cgroup_storage *old_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
> -	struct bpf_prog *new_prog = prog ? : link->link.prog;
> +	struct bpf_prog *new_prog = prog ? : (link ? link->link.prog : NULL);
>   	enum cgroup_bpf_attach_type atype;
>   	u32 old_flags, old_pl_flags;
>   	struct bpf_prog_list *pl;
> @@ -833,6 +833,8 @@
>   		return -EINVAL;
>   	if (!!replace_prog != !!(flags & BPF_F_REPLACE))
>   		/* replace_prog implies BPF_F_REPLACE, and vice versa */
> +		return -EINVAL;
> +	if (!new_prog)
>   		return -EINVAL;
>   
>   	atype = bpf_cgroup_atype_find(type, new_prog->aux->attach_btf_id);
>
> --
> 2.43.0

Re: [PATCH] bpf: cgroup: fix null-ptr-deref in __cgroup_bpf_attach

Posted by Emil Tsalapatis 2 weeks, 3 days ago

+1, this patch implies the error is possible which afaict it isn't.

pw-bot: cr

On Wed, Sep 2, 2026 at 5:41 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
>
> on 9/2/26 5:02 PM, Liu Jing wrote:
> > In __cgroup_bpf_attach(), the expression "prog ? : link->link.prog"
> > dereferences link before validating that link is non-NULL. If both
> > prog and link are NULL (a contract violation by the caller), this
> > causes a null pointer dereference.
>
>
> Both being NULL is unreachable, so the new check is dead code.
>
> It's meaningless.
>
>
> >
> > Add a safe ternary fallback and a NULL check for new_prog before
> > it is used further.
> >
> > Signed-off-by: Liu Jing <liujing@cmss.chinamobile.com>
> > ---
> >   kernel/bpf/cgroup.c | 4 +++-
> >   1 file changed, 3 insertion(+), 1 deletion(-)
> >
> > --- a/kernel/bpf/cgroup.c
> > +++ b/kernel/bpf/cgroup.c
> > @@ -814,7 +814,7 @@
> >       struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
> >       struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
> >       struct bpf_cgroup_storage *old_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
> > -     struct bpf_prog *new_prog = prog ? : link->link.prog;
> > +     struct bpf_prog *new_prog = prog ? : (link ? link->link.prog : NULL);
> >       enum cgroup_bpf_attach_type atype;
> >       u32 old_flags, old_pl_flags;
> >       struct bpf_prog_list *pl;
> > @@ -833,6 +833,8 @@
> >               return -EINVAL;
> >       if (!!replace_prog != !!(flags & BPF_F_REPLACE))
> >               /* replace_prog implies BPF_F_REPLACE, and vice versa */
> > +             return -EINVAL;
> > +     if (!new_prog)
> >               return -EINVAL;
> >
> >       atype = bpf_cgroup_atype_find(type, new_prog->aux->attach_btf_id);
> >
> > --
> > 2.43.0