bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bpf PATCH v2 0/3] Sockmap RCU splat fix
@ 2020-06-25 23:12 John Fastabend
  2020-06-25 23:12 ` [bpf PATCH v2 1/3] bpf, sockmap: RCU splat with redirect and strparser error or TLS John Fastabend
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: John Fastabend @ 2020-06-25 23:12 UTC (permalink / raw)
  To: kafai, jakub, daniel, ast; +Cc: netdev, bpf, john.fastabend

Fix a splat introduced by recent changes to avoid skipping ingress policy
when kTLS is enabled. The RCU splat was introduced because in the non-TLS
case the caller is wrapped in an rcu_read_lock/unlock. But, in the TLS
case we have a reference to the psock and the caller did not wrap its
call in rcu_read_lock/unlock.

To fix extend the RCU section to include the redirect case which was
missed. From v1->v2 I changed the location a bit to simplify the code
some. See patch 1.

But, then Martin asked why it was not needed in the non-TLS case. The
answer for patch 1 was, as stated above, because the caller has the
rcu read lock. However, there was still a missing case where a BPF
user could in-theory line up a set of parameters to hit a case
where the code was entered from strparser side from a different context
then the initial caller. To hit this user would need a parser program
to return value greater than skb->len then an ENOMEM error could happen
in the strparser codepath triggering strparser to retry from a workqueue
and without rcu_read_lock original caller used. See patch 2 for details.

Finally, we don't actually have any selftests for parser returning a
value geater than skb->len so add one in patch 3. This is especially
needed because at least I don't have any code that uses the parser
to return value greater than skb->len. So I wouldn't have caught any
errors here in my own testing.

Thanks, John

v1->v2: simplify code in patch 1 some and add patches 2 and 3.

---

John Fastabend (3):
      bpf, sockmap: RCU splat with redirect and strparser error or TLS
      bpf, sockmap: RCU dereferenced psock may be used outside RCU block
      bpf, sockmap: Add ingres skb tests that utilize merge skbs


 net/core/skmsg.c                                   |   23 +++++++++++++-------
 .../selftests/bpf/progs/test_sockmap_kern.h        |    8 ++++++-
 tools/testing/selftests/bpf/test_sockmap.c         |   18 ++++++++++++++++
 3 files changed, 40 insertions(+), 9 deletions(-)

--
Signature

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-06-29 20:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-25 23:12 [bpf PATCH v2 0/3] Sockmap RCU splat fix John Fastabend
2020-06-25 23:12 ` [bpf PATCH v2 1/3] bpf, sockmap: RCU splat with redirect and strparser error or TLS John Fastabend
2020-06-26  6:10   ` Martin KaFai Lau
2020-06-26 11:11   ` Jakub Sitnicki
2020-06-25 23:13 ` [bpf PATCH v2 2/3] bpf, sockmap: RCU dereferenced psock may be used outside RCU block John Fastabend
2020-06-26  6:13   ` Martin KaFai Lau
2020-06-29  7:42   ` Jakub Sitnicki
2020-06-25 23:13 ` [bpf PATCH v2 3/3] bpf, sockmap: Add ingres skb tests that utilize merge skbs John Fastabend
2020-06-26  6:14   ` Martin KaFai Lau
2020-06-28 15:40 ` [bpf PATCH v2 0/3] Sockmap RCU splat fix Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).