[bpf PATCH 0/6] BPF fixes for sockhash

John Fastabend
2018-06-13T17:50:26Z

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.

---

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

kernel/bpf/sockmap.c | 266 ++++++++++++++++++++++++-------
tools/testing/selftests/bpf/test_maps.c | 4
2 files changed, 208 insertions(+), 62 deletions(-)