[PATCH bpf v1 1/1] bpf: Fix sk_local_storage diag dumping uninitialized special fields

[PATCH bpf v1 1/1] bpf: Fix sk_local_storage diag dumping uninitialized special fields

[Amery Hung]

Call check_and_init_map_value() after the copy_map_value() to zero out
special field regions. diag_get() copies sk_local_storage map values
into a netlink message using copy_map_value{_locked}(), which
intentionally skip special fields. However, the destination buffer from
nla_reserve_64bit() is not zeroed and the skipped regions contain
uninitialized skb data can be sent to userspace.

Fixes: 1ed4d92458a9 ("bpf: INET_DIAG support in bpf_sk_storage")
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 net/core/bpf_sk_storage.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
index 14eb7812bda4..b50d26a542ed 100644
--- a/net/core/bpf_sk_storage.c
+++ b/net/core/bpf_sk_storage.c
@@ -558,6 +558,7 @@ static int diag_get(struct bpf_local_storage_data *sdata, struct sk_buff *skb)
 				      sdata->data, true);
 	else
 		copy_map_value(&smap->map, nla_data(nla_value), sdata->data);
+	check_and_init_map_value(&smap->map, nla_data(nla_value));
 
 	nla_nest_end(skb, nla_stg);
 	return 0;
-- 
2.52.0

Re: [PATCH bpf v1 1/1] bpf: Fix sk_local_storage diag dumping uninitialized special fields

[Mykyta Yatsenko]

On 4/23/26 11:23 PM, Amery Hung wrote:
> Call check_and_init_map_value() after the copy_map_value() to zero out
> special field regions. diag_get() copies sk_local_storage map values
> into a netlink message using copy_map_value{_locked}(), which
> intentionally skip special fields. However, the destination buffer from
> nla_reserve_64bit() is not zeroed and the skipped regions contain
> uninitialized skb data can be sent to userspace.
> 
> Fixes: 1ed4d92458a9 ("bpf: INET_DIAG support in bpf_sk_storage")
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
>   net/core/bpf_sk_storage.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> index 14eb7812bda4..b50d26a542ed 100644
> --- a/net/core/bpf_sk_storage.c
> +++ b/net/core/bpf_sk_storage.c
> @@ -558,6 +558,7 @@ static int diag_get(struct bpf_local_storage_data *sdata, struct sk_buff *skb)
>   				      sdata->data, true);
>   	else
>   		copy_map_value(&smap->map, nla_data(nla_value), sdata->data);
> +	check_and_init_map_value(&smap->map, nla_data(nla_value));
>   

I think check_and_init_map_value() should be moved before the
copy_map_value(), because copy_map_value_locked() already uses
spin lock special field, which if uninitialized can deadlock?

>   	nla_nest_end(skb, nla_stg);
>   	return 0;

Re: [PATCH bpf v1 1/1] bpf: Fix sk_local_storage diag dumping uninitialized special fields

[Martin KaFai Lau]

On Fri, Apr 24, 2026 at 12:50:34AM +0100, Mykyta Yatsenko wrote:
> > diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> > index 14eb7812bda4..b50d26a542ed 100644
> > --- a/net/core/bpf_sk_storage.c
> > +++ b/net/core/bpf_sk_storage.c
> > @@ -558,6 +558,7 @@ static int diag_get(struct bpf_local_storage_data *sdata, struct sk_buff *skb)
> >   				      sdata->data, true);
> >   	else
> >   		copy_map_value(&smap->map, nla_data(nla_value), sdata->data);
> > +	check_and_init_map_value(&smap->map, nla_data(nla_value));
> 
> I think check_and_init_map_value() should be moved before the
> copy_map_value(), because copy_map_value_locked() already uses
> spin lock special field, which if uninitialized can deadlock?

The src (sdata->data) lock is used instead of
the dst (nla_data(nla_value)) lock, so it should be fine.

Re: [PATCH bpf v1 1/1] bpf: Fix sk_local_storage diag dumping uninitialized special fields

[Mykyta Yatsenko]

On 4/24/26 7:49 PM, Martin KaFai Lau wrote:
> On Fri, Apr 24, 2026 at 12:50:34AM +0100, Mykyta Yatsenko wrote:
>>> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
>>> index 14eb7812bda4..b50d26a542ed 100644
>>> --- a/net/core/bpf_sk_storage.c
>>> +++ b/net/core/bpf_sk_storage.c
>>> @@ -558,6 +558,7 @@ static int diag_get(struct bpf_local_storage_data *sdata, struct sk_buff *skb)
>>>    				      sdata->data, true);
>>>    	else
>>>    		copy_map_value(&smap->map, nla_data(nla_value), sdata->data);
>>> +	check_and_init_map_value(&smap->map, nla_data(nla_value));
>>
>> I think check_and_init_map_value() should be moved before the
>> copy_map_value(), because copy_map_value_locked() already uses
>> spin lock special field, which if uninitialized can deadlock?
> 
> The src (sdata->data) lock is used instead of
> the dst (nla_data(nla_value)) lock, so it should be fine.

Right, the lock is protecting from torn read, my bad, thanks!

Re: [PATCH bpf v1 1/1] bpf: Fix sk_local_storage diag dumping uninitialized special fields

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf.git (master)
by Martin KaFai Lau <martin.lau@kernel.org>:

On Thu, 23 Apr 2026 15:23:55 -0700 you wrote:
> Call check_and_init_map_value() after the copy_map_value() to zero out
> special field regions. diag_get() copies sk_local_storage map values
> into a netlink message using copy_map_value{_locked}(), which
> intentionally skip special fields. However, the destination buffer from
> nla_reserve_64bit() is not zeroed and the skipped regions contain
> uninitialized skb data can be sent to userspace.
> 
> [...]

Here is the summary with links:
  - [bpf,v1,1/1] bpf: Fix sk_local_storage diag dumping uninitialized special fields
    https://git.kernel.org/bpf/bpf/c/b5c111f4967b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH bpf v1 1/1] bpf: Fix sk_local_storage diag dumping uninitialized special fields

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [Critical] Calling `check_and_init_map_value()` on a netlink payload buffer writes kernel pointers into the message, causing a KASLR bypass.
--

commit 3db95684928f56c741f1722b8018d7049c261634
Author: Amery Hung <ameryhung@gmail.com>

bpf: Fix sk_local_storage diag dumping uninitialized special fields

This commit attempts to prevent uninitialized memory from being leaked to
userspace during socket diag dumps. It calls check_and_init_map_value() on
the netlink payload buffer to initialize special map fields that are skipped
by copy_map_value().

> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> --- a/net/core/bpf_sk_storage.c
> +++ b/net/core/bpf_sk_storage.c
> @@ -558,6 +558,7 @@ static int diag_get(struct bpf_local_storage_data *sdata, struct sk_buff *skb)
>  				      sdata->data, true);
>  	else
>  		copy_map_value(&smap->map, nla_data(nla_value), sdata->data);
> +	check_and_init_map_value(&smap->map, nla_data(nla_value));

Does calling check_and_init_map_value() directly on the netlink payload leak
kernel pointers to userspace?

The function check_and_init_map_value() calls bpf_obj_init() to initialize
the special fields in the map value.

For fields like BPF_LIST_HEAD and BPF_RB_NODE, bpf_obj_init() invokes
initialization macros like INIT_LIST_HEAD() and RB_CLEAR_NODE().

INIT_LIST_HEAD() assigns self-referential pointers to the list structure:

static inline void INIT_LIST_HEAD(struct list_head *list)
{
        WRITE_ONCE(list->next, list);
        WRITE_ONCE(list->prev, list);
}

Because this initialization is performed directly on the allocated netlink
payload inside the socket buffer, valid kernel heap addresses are written
into the message instead of zeros. When this message is sent to userspace,
it leaks the kernel addresses.

>  
>  	nla_nest_end(skb, nla_stg);
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423222356.155387-1-ameryhung@gmail.com?part=1