Re: [syzbot] [bpf?] general protection fault in bpf_get_local_storage (2)

[Dmitry Vyukov]

On Wed, 31 Dec 2025 at 08:31, Liang Jie <buaajxlj@xxxxxxx> wrote:
>
> On Tue, 30 Dec 2025 22:18:00 -0800, Amery Hung <ameryhung@xxxxxxxxx> wrote:
> > >
> > > #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3f0e9c8cefa9
> > >
> > > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> > > index 69988af44b37..2bc27ece5cc5 100644
> > > --- a/kernel/bpf/cgroup.c
> > > +++ b/kernel/bpf/cgroup.c
> > > @@ -1768,6 +1768,9 @@ BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
> > >         ctx = container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
> > >         storage = ctx->prog_item->cgroup_storage[stype];
> > >
> > > +       if (unlikely(!storage))
> > > +               return (unsigned long)NULL;
> > > +
> > >         if (stype == BPF_CGROUP_STORAGE_SHARED)
> > >                 ptr = &READ_ONCE(storage->buf)->data[0];
> > >
> > >
> >
> > Hi Liang,
> >
> > I don't think we can do this here due to backward compatibility. The
> > return type of the helper is RET_PTR_TO_MAP_VALUE. Your proposed fix
> > would require adding a PTR_MAYBE_NULL and existing BPF programs would
> > no longer pass the verifier.
> >
> > Did you look into why the storage pointer is NULL in the first place?
> >
> > BTW, there is also another similar report and a work-in-progress fix
> > [1]. Do you think this is a separate issue from that?
>
> Hi Amery,
>
> Thanks for pointing this out.
>
> Sorry, I initially missed your earlier WIP fix. Looking at it now, this
> does seem to be addressing the same class of issues where
> bpf_get_local_storage() can observe a NULL storage pointer.
>
> I'm interested in this area and will take a closer look to see whether
> your patch also covers the syzbot report.

Hi Liang,

Have you had any progress on this?

syzbot performed AI debugging and generated a patch for this:
https://syzbot.org/ai_job?id=8b1e4143-642b-4b3e-8be9-194c7a66e24a

It came to a similar conclusion regarding storage allocation:

"""
The root cause is a bug in __cgroup_bpf_replace() which handles
BPF_LINK_UPDATE for cgroup links. When replacing a program that does not
use cgroup storage with one that does, __cgroup_bpf_replace() fails to
allocate the new cgroup storage.
"""

But the patch is very convoluted and is likely incorrect (though,
passed testing on this reproducer).

Let's try testing the other patch on this repro.

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
v7.1-rc3
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index c8ae6ab31651..e249ea98f55d 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2403,8 +2403,7 @@ static bool __bpf_prog_map_compatible(struct bpf_map *map,
 				break;
 			cookie = aux->cgroup_storage[i] ?
 				 aux->cgroup_storage[i]->cookie : 0;
-			ret = map->owner->storage_cookie[i] == cookie ||
-			      !cookie;
+			ret = map->owner->storage_cookie[i] == cookie;
 		}
 		if (ret &&
 		    map->owner->attach_func_proto != aux->attach_func_proto) {