Re: [PATCH bpf] bpf, sockmap: Fix af_unix null-ptr-deref in proto update

[Martin KaFai Lau <martin.lau@linux.dev>]

On 2/4/26 4:55 PM, Martin KaFai Lau wrote:
> 
> 
> On 2/4/26 1:09 PM, Kuniyuki Iwashima wrote:
>> From: Martin KaFai Lau <martin.lau@linux.dev>
>> Date: Wed, 4 Feb 2026 11:34:55 -0800
>>> On 2/4/26 7:41 AM, Michal Luczaj wrote:
>>>>>>>>> If the concern is the bpf iterator prog may use a released 
>>>>>>>>> unix_peer(sk)
>>>>>>>>> pointer, it should be fine. The unix_peer(sk) pointer is not a 
>>>>>>>>> trusted
>>>>>>>>> pointer to the bpf prog, so nothing bad will happen other than
>>>>>>>>> potentially reading incorrect values.
>>>
>>> I misremembered that following unix->peer would be marked as
>>> (PTR_TO_BTF_ID | PTR_UNTRUSTED). I forgot there are some legacy supports
>>> on the PTR_TO_BTF_ID (i.e. without PTR_UNTRUSTED marking).
>>>
>>>>>>>>
>>>>>>>> But if the prog passes a released peer pointer to a bpf helper:
>>>>>>>>
>>>>>>>> BUG: KASAN: slab-use-after-free in bpf_skc_to_unix_sock+0x95/0xb0
>>>>>>>> Read of size 1 at addr ffff888110654c92 by task test_progs/1936
>>>>>>
>>>>>> hmm... bpf_skc_to_unix_sock is exposed to tracing. bpf_iter is a 
>>>>>> tracing
>>>>>> bpf prog.
>>>>>>
>>>>>>>
>>>>>>> Can you cook a patch for this ? probably like below
>>>>>>
>>>>>> This can help the bpf_iter but not the other tracing prog such as 
>>>>>> fentry.
>>>>>
>>>>> Oh well ... then bpf_skc_to_unix_sock() can be used even
>>>>> with SEQ_START_TOKEN at fentry of bpf_iter_unix_seq_show() ??
>>>
>>> It is fine. The type is void.
>>>
>>>>>
>>>>> How about adding notrace to all af_unix bpf iterator functions ?
>>>
>>> but right, other functions taking [unix_]sock pointer could be audited.
>>> I don't know af_unix well enough to assess the blast radius or whether
>>> some useful functions may become untraceable.
>>
>> Considering SOCK_DGRAM, the blast radus is much bigger than
>> I thought, so I'd avoid this way if possible by modifying
>> the verifier.
>>
>>
>>>
>>>>>
>>>>> The procfs iterator holds a spinlock of the hashtable from
>>>>> ->start/next() to ->stop() to prevent the race with 
>>>>> unix_release_sock().
>>>>>
>>>>> I think other (non-iterator) functions cannot do such racy
>>>>> access with tracing prog.
>>>>
>>>> But then there's SOCK_DGRAM where you can drop unix_peer(sk) without
>>>> releasing sk; see AF_UNSPEC in unix_dgram_connect(). I think Martin is
>>>> right, we can crash at many fentries.
>>>>
>>>> BUG: KASAN: slab-use-after-free in bpf_skc_to_unix_sock+0xa4/0xb0
>>>> Read of size 2 at addr ffff888147d38890 by task test_progs/2495
>>>> Call Trace:
>>>>    dump_stack_lvl+0x5d/0x80
>>>>    print_report+0x170/0x4f3
>>>>    kasan_report+0xe1/0x180
>>>>    bpf_skc_to_unix_sock+0xa4/0xb0
>>>>    bpf_prog_564a1c39c35d86a2_unix_shutdown_entry+0x8a/0x8e
>>>>    bpf_trampoline_6442564662+0x47/0xab
>>>>    unix_shutdown+0x9/0x880
>>>>    __sys_shutdown+0xe1/0x160
>>>>    __x64_sys_shutdown+0x52/0x90
>>>>    do_syscall_64+0x6b/0x3a0
>>>>    entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>>
>>> This probably is the first case where reading a sk pointer requires a
>>> lock. I think it will need to be marked as PTR_UNTRUSTED in the verifier
>>> for the unix->peer access, so that it cannot be passed to a helper.
>>> There is a BTF_TYPE_SAFE_TRUSTED list. afaik, there is no untrusted 
>>> one now.
>>
>> Just skimmed the code, and I guess something like below would
>> do that ?  and if needed, we could add another helper to fetch
>> peer with a proper release function ?
>>
>> ---8<---
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 3135643d5695..ef8b4dd21923 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -7177,6 +7177,14 @@ static bool type_is_rcu_or_null(struct 
>> bpf_verifier_env *env,
>>       return btf_nested_type_is_trusted(&env->log, reg, field_name, 
>> btf_id, "__safe_rcu_or_null");
>>   }
>> +static bool type_is_untrusted(struct bpf_verifier_env *env,
>> +                  struct bpf_reg_state *reg,
>> +                  const char *field_name, u32 btf_id)
>> +{
>> +    /* TODO: return true if field_name and btf_id is unix_sock.peer. */
>> +    return false;
>> +}
>> +
>>   static bool type_is_trusted(struct bpf_verifier_env *env,
>>                   struct bpf_reg_state *reg,
>>                   const char *field_name, u32 btf_id)
>> @@ -7307,7 +7315,9 @@ static int check_ptr_to_btf_access(struct 
>> bpf_verifier_env *env,
>>            * A regular RCU-protected pointer with __rcu tag can also 
>> be deemed
>>            * trusted if we are in an RCU CS. Such pointer can be NULL.
>>            */
>> -        if (type_is_trusted(env, reg, field_name, btf_id)) {
>> +        if (type_is_untrusted(env, reg, field_name, btf_id)) {
>> +            flag |= PTR_UNTRUSTED;
> 
> Something like this but I think the PTR_UNTRUSTED marking should be done 
> right after the clear_trusted_flags() where it is for supporting the 
> depreciated PTR_TO_BTF_ID. Before that ...
> 
> Alexei, can you advise if we should change the verifier to mark 
> PTR_UNTRUSTED on unix_sock->peer or we can deprecate the bpf_skc_to_* 
> helper support from tracing and ask the user to switch to bpf_core_cast 
> (i.e. bpf_rdonly_cast) by using a WARN_ON_ONCE message?

After trying more, taking out bpf_skc_to_* is not enough. It still needs 
to reject passing unix->peer to bpf_setsockopt for bpf_iter, so 
PTR_UNTRUSTED mark is needed.

> 
> The problem is that the unix_sock->peer pointer is not always valid when 
> passing to the bpf_skc_to_* helpers, so it is a UAF.
>