Re: [PATCH v4 1/2] bpf: Tighten cgroup storage cookie checks for prog arrays

[Yonghong Song <yonghong.song@linux.dev>]

On 6/5/26 4:16 PM, Daniel Borkmann wrote:
> On 6/4/26 6:56 PM, Yonghong Song wrote:
>> On 6/4/26 12:03 AM, Lin Ma wrote:
>>> The recent KCTF-reported cgroup local storage issue assigned
>>> CVE-2025-38502 was fixed by commit abad3d0bad72 ("bpf: Fix oob access
>>> in cgroup local storage").
>>>
>>> However, the previous fixes are still incomplete. The current 
>>> prog-array
>>> compatibility check treats a program with no cgroup storage as
>>> compatible with any stored storage cookie. This allows a storage-less
>>> program to bridge a tail-call chain between an entry program and a
>>> storage-using callee even though runtime cgroup local storage still
>>> follows the caller context.
>>>
>>> Require exact per-type storage_cookie equality when checking prog-array
>>> compatibility. This blocks zero-storage bridge programs from joining a
>>> prog-array owned by a storage-using program and closes the residual
>>> A -> B(no storage) -> C(storage) path.
>>>
>>> This also aligns with Amery Hung's earlier NULL-storage tail-call 
>>> fix by
>>> requiring storage use to match consistently across prog-array users.
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: abad3d0bad72 ("bpf: Fix oob access in cgroup local storage")
>>> Tested-by: Amery Hung <ameryhung@gmail.com>
>>> Signed-off-by: Lin Ma <malin89@huawei.com>
>>> Signed-off-by: Rongzhen Cui <cuirongzhen@huawei.com>
>>> Signed-off-by: Jingguo Tan <tanjingguo@huawei.com>
>>
>> Acked-by: Yonghong Song <yonghong.song@linux.dev>
>
> Sorry for jumping in late, but I would have an alternative suggestion 
> below:
>
>>> v1: 
>>> https://lore.kernel.org/bpf/20260601095158.1186318-1-malin89@huawei.com/ 
>>>
>>> v1 -> v2:
>>>   - refine the commit message and mention the relation to Amery Hung's
>>>     NULL-storage tail-call fix
>>>   - add patch 2/2 selftests for tail-call cgroup storage prog-array
>>>     checks
>>> v2: 
>>> https://lore.kernel.org/bpf/31927f33-9db0-4a39-b38a-72b33487979e@linux.dev/T/#t
>>> v2 -> v3:
>>>   - use abad3d0bad72 as the Fixes tag
>>> v3: 
>>> https://lore.kernel.org/bpf/7b3e1adae5c92153e991ac406b2d334609c36d866b5bf81e4465cf63bde0a79c@mail.kernel.org/T/#t
>>> v3 -> v4:
>>>   - no changes
>>> ---
>>>   kernel/bpf/core.c | 8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>>> index 6aa2a8b24030..f0b61b10f30e 100644
>>> --- a/kernel/bpf/core.c
>>> +++ b/kernel/bpf/core.c
>>> @@ -2470,8 +2470,12 @@ 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;
>>> +            /*
>>> +             * Tail calls keep using the caller cgroup storage
>>> +             * context, so prog-array members must use the same
>>> +             * storage cookie.
>>> +             */
>>> +            ret = map->owner->storage_cookie[i] == cookie;
>
>
> This would basically break if the leaf program does not use cgroup 
> storage.. can
> you please try out the below diff ... rationale: this would allow A -> 
> B(no storage)
> but reject the A -> B(no storage) -> C(storage) case:
>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index a656a8572bdb..649cce41e13f 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2481,7 +2481,7 @@ static bool __bpf_prog_map_compatible(struct 
> bpf_map *map,
>                         cookie = aux->cgroup_storage[i] ?
> aux->cgroup_storage[i]->cookie : 0;
>                         ret = map->owner->storage_cookie[i] == cookie ||
> -                             !cookie;
> +                             (!cookie && !aux->tail_call_reachable);
>                 }
>                 if (ret &&
>                      map->owner->attach_func_proto != aux->attach_func_proto) {

I just tried. All tests (including patch v4) passed. Your patch indeed
preserves the leaf prog (last one in the chain). Thanks!