On Tue, Aug 26, 2025 at 3:02 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > 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. In v3, it's nuanced to limit hooks with sk->sk_memcg to unlocked hooks, socket(2), but if there is unlocked place with non-NULL sk_memcg in _bh context, we will sill need to use setsockopt_proto. sk_clone_lock() and reuseport_migrate_sock() in inet_csk_listen_stop() are the only places where we don't check sock_owned_by_user(). sk_clone_lock ()'s path is fine as sk_memcg is NULL until accept(), and sk_reuseport_func_proto() doesn't allow setsockopt() for now (error-prone to future changes), but I may be missing something.
