[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

[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

[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!