This addresses two syzbot issues that lead to identifing (by Eric and
Wei) a class of bugs where we don't correctly check for IPv4/v6
sockets and their associated state. The second issue was a locking
error in sockhash.
The first 2 patches address handling IPv4 correctly and then ensuring
that only sockets in ESTABLISHED state can be added. There is then a
follow up fix (patch4) to fix the other issue Eric noted, namely that
we depend on sockets to call tcp_close to remove them from the map.
However, we missed that a socket can transition through
tcp_disconnect() and never call tcp_close() missing our hook. To
resolve this implement the unhash hook which is also called from the
tcp_disconnect() flow.
The other issue syzbot found that the tcp_close() handler missed
locking the hash bucket lock which could result in corrupting the
sockhash bucket list if delete and close ran at the same time. To
fix this we had to restructure the tcp_close() lock handling. This is
done in patch 3.
Finally, during review I noticed the release handler was ommitted
from the upstream code (patch 5) due to an incorrect merge conflict
fix when I ported the code to latest bpf-next before submitting. And
then patch 6 fixes up selftests for the above.
The tcp_disconnect() catch also appears to be missing in kTLS so
a follow up patch will need to address that as well.
v2: Added sock lock to update paths in patch2. Martin noticed this
during review. I was planning to do this in a follow up patch but
I agree its a bit odd to not do it upfront so incorporated into
'bpf: sockmap only allow ESTABLISHED sock state'. In bpf-next we
may consider also taking sock lock on delete/map_free and which
point we could drop some usages of sk_callback_lock but need to
think a bit on the trade-offs of this.
---
John Fastabend (6):
bpf: sockmap, fix crash when ipv6 sock is added
bpf: sockmap only allow ESTABLISHED sock state
bpf: sockhash fix omitted bucket lock in sock_close
bpf: sockmap, tcp_disconnect to listen transition
bpf: sockhash, add release routine
bpf: selftest remove attempts to add LISTEN sockets to sockmap
0 files changed
--
Signature
