diff options
author | Martin KaFai Lau <martin.lau@kernel.org> | 2024-01-04 03:34:32 +0300 |
---|---|---|
committer | Martin KaFai Lau <martin.lau@kernel.org> | 2024-01-04 03:50:24 +0300 |
commit | 417fa6d163df6f13fb2cfad5132eff354c8a472e (patch) | |
tree | d8a0d5f195e29edc68c8095a224b082f57ef2dfc /net/unix/unix_bpf.c | |
parent | b4560055c8f11c5e2cfffb4de928b3cfd4eae3b4 (diff) | |
parent | bdbca46d3f84a4455cd5c15a7483666218851549 (diff) | |
download | linux-417fa6d163df6f13fb2cfad5132eff354c8a472e.tar.xz |
Merge branch 'fix sockmap + stream af_unix memleak'
John Fastabend says:
====================
There was a memleak when streaming af_unix sockets were inserted into
multiple sockmap slots and/or maps. This is because each insert would
call a proto update operatino and these must be allowed to be called
multiple times. The streaming af_unix implementation recently added
a refcnt to handle a use after free issue, however it introduced a
memleak when inserted into multiple maps.
This series fixes the memleak, adds a note in the code so we remember
that proto updates need to support this. And then we add three tests
for each of the slightly different iterations of adding sockets into
multiple maps. I kept them as 3 independent test cases here. I have
some slight preference for this they could however be a single test,
but then you don't get to run them independently which was sort of
useful while debugging.
====================
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Diffstat (limited to 'net/unix/unix_bpf.c')
-rw-r--r-- | net/unix/unix_bpf.c | 21 |
1 files changed, 18 insertions, 3 deletions
diff --git a/net/unix/unix_bpf.c b/net/unix/unix_bpf.c index 7ea7c3a0d0d0..bd84785bf8d6 100644 --- a/net/unix/unix_bpf.c +++ b/net/unix/unix_bpf.c @@ -161,15 +161,30 @@ int unix_stream_bpf_update_proto(struct sock *sk, struct sk_psock *psock, bool r { struct sock *sk_pair; + /* Restore does not decrement the sk_pair reference yet because we must + * keep the a reference to the socket until after an RCU grace period + * and any pending sends have completed. + */ if (restore) { sk->sk_write_space = psock->saved_write_space; sock_replace_proto(sk, psock->sk_proto); return 0; } - sk_pair = unix_peer(sk); - sock_hold(sk_pair); - psock->sk_pair = sk_pair; + /* psock_update_sk_prot can be called multiple times if psock is + * added to multiple maps and/or slots in the same map. There is + * also an edge case where replacing a psock with itself can trigger + * an extra psock_update_sk_prot during the insert process. So it + * must be safe to do multiple calls. Here we need to ensure we don't + * increment the refcnt through sock_hold many times. There will only + * be a single matching destroy operation. + */ + if (!psock->sk_pair) { + sk_pair = unix_peer(sk); + sock_hold(sk_pair); + psock->sk_pair = sk_pair; + } + unix_stream_bpf_check_needs_rebuild(psock->sk_proto); sock_replace_proto(sk, &unix_stream_bpf_prot); return 0; |