On Thu, Jun 25, 2020 at 4:12 PM John Fastabend <john.fastabend@xxxxxxxxx> wrote: > > 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. Applied. Thanks
