netdev - Re: [PATCH v1 bpf-next/net 2/8] bpf: Add a bpf hook in __inet

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <93ddc9c9-e087-4a8d-a76c-8a081cf3f1ac@linux.dev>
Date: Tue, 26 Aug 2025 15:02:23 -0700
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Kuniyuki Iwashima <kuniyu@...gle.com>
Cc: almasrymina@...gle.com, andrii@...nel.org, ast@...nel.org,
 bpf@...r.kernel.org, daniel@...earbox.net, davem@...emloft.net,
 edumazet@...gle.com, hannes@...xchg.org, john.fastabend@...il.com,
 kuba@...nel.org, kuni1840@...il.com, mhocko@...nel.org,
 ncardwell@...gle.com, netdev@...r.kernel.org, pabeni@...hat.com,
 roman.gushchin@...ux.dev, sdf@...ichev.me, shakeel.butt@...ux.dev,
 willemb@...gle.com
Subject: Re: [PATCH v1 bpf-next/net 2/8] bpf: Add a bpf hook in
 __inet_accept().

On 8/26/25 2:08 PM, Kuniyuki Iwashima wrote:
>> ... need a way to disallow this SK_BPF_MEMCG_SOCK_ISOLATED bit being changed
>> once the socket fd is visible to the user. The current approach is to use the
>> observation in the owned_by_user and sk->sk_socket in the create and accept
>> hook. [ unrelated, I am not sure about the owned_by_user check considering
>> sol_socket_sockopt can be called from bh ].
> 
> [ my expectation was bh checks sock_owned_by_user() before
>    processing packets and entering where bpf_setsockopt() can
>    be called ]

hmm... so if a bpf prog is run in bh, owned_by_user should be false and the bh 
bpf prog can continue to do the bpf_setsockopt(SK_BPF_MEMCG_FLAGS). I was 
looking at this comment in v1 and v2, "Don't allow once sk has been published to 
userspace.". Regardless, it seems that v3 allows other bpf hooks to do the 
bpf_setsockopt(SK_BPF_MEMCG_FLAGS)?, so not sure if this point is still relevant.

> 
>>
>> If it is needed, there are other ways to stop the SK_BPF_MEMCG_SOCK_ISOLATED
>> being changed again once the fd is visible to user. e.g. there are still bits
>> left in the sk_memcg pointer to freeze it at runtime. If doing it statically
>> (i.e. at prog load time), it can probably return a different setsockopt_proto
>> that can understand the SK_BPF_MEMCG_FLAGS.
> 
> I was thinking a kind of the latter, passing caller info to general
> __bpf_setsockopt(), and gave up as it was ugly, but wrapping it
> as different setsockopt_proto sounds good.
> 
> Then, we don't need to care about how to limit the caller context.
passing caller info is doable in helper but it is not pretty for helper and 
needs verifier changes (kfunc would be easier), so I wouldn't go there also. fyi 
only, take a look at bpf_timer_set_callback. However, all five args in 
bpf_setsockopt is used, so the same way won't work.

Right, wrapping it into a different setsockopt_proto is an option but admittedly 
not pretty also but should work. Lets not go there first. It seems the v3 design 
has changed a bit. Lets discuss and explore there.