public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: ast@kernel.org, daniel@iogearbox.net
Cc: netdev@vger.kernel.org
Subject: [bpf PATCH 0/6] BPF fixes for sockhash
Date: Wed, 13 Jun 2018 10:49:54 -0700	[thread overview]
Message-ID: <20180613174202.4264.59970.stgit@john-Precision-Tower-5810> (raw)

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(-)

             reply	other threads:[~2018-06-13 17:50 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-13 17:49 John Fastabend [this message]
2018-06-13 17:49 ` [bpf PATCH 1/6] bpf: sockmap, fix crash when ipv6 sock is added John Fastabend
2018-06-13 17:50 ` [bpf PATCH 2/6] bpf: sockmap only allow ESTABLISHED sock state John Fastabend
2018-06-13 17:50 ` [bpf PATCH 3/6] bpf: sockhash fix omitted bucket lock in sock_close John Fastabend
2018-06-13 17:50 ` [bpf PATCH 4/6] bpf: sockmap, tcp_disconnect to listen transition John Fastabend
2018-06-14  0:56   ` Martin KaFai Lau
2018-06-14  5:48     ` John Fastabend
2018-06-14 16:47       ` John Fastabend
2018-06-13 17:50 ` [bpf PATCH 5/6] bpf: sockhash, add release routine John Fastabend
2018-06-13 17:50 ` [bpf PATCH 6/6] bpf: selftest remove attempts to add LISTEN sockets to sockmap John Fastabend

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180613174202.4264.59970.stgit@john-Precision-Tower-5810 \
    --to=john.fastabend@gmail.com \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox