Messages in this thread | | | Date | Wed, 9 Aug 2023 07:39:28 -0700 | Subject | Re: [PATCH bpf-next] net: Fix slab-out-of-bounds in inet[6]_steal_sock | From | Martin KaFai Lau <> |
| |
On 8/9/23 1:33 AM, Lorenz Bauer wrote: > Kumar reported a KASAN splat in tcp_v6_rcv: > > bash-5.2# ./test_progs -t btf_skc_cls_ingress > ... > [ 51.810085] BUG: KASAN: slab-out-of-bounds in tcp_v6_rcv+0x2d7d/0x3440 > [ 51.810458] Read of size 2 at addr ffff8881053f038c by task test_progs/226 > > The problem is that inet[6]_steal_sock accesses sk->sk_protocol without > accounting for request sockets. I added the check to ensure that we only > every try to perform a reuseport lookup on a supported socket. > > It turns out that this isn't necessary at all. struct sock_common contains > a skc_reuseport flag which indicates whether a socket is part of a
Does it go back to the earlier discussion (https://lore.kernel.org/bpf/7188429a-c380-14c8-57bb-9d05d3ba4e5e@linux.dev/) that the sk->sk_reuseport is 1 from sk_clone for TCP_ESTABLISHED? It works because there is sk->sk_reuseport"_cb" check going deeper into reuseport_select_sock() but there is an extra inet6_ehashfn for all TCP_ESTABLISHED.
> reuseport group. inet[6]_lookup_reuseport already check this flag, > so we can't execute an erroneous reuseport lookup by definition. > > Remove the unnecessary assertions to fix the out of bounds access. > > Fixes: 9c02bec95954 ("bpf, net: Support SO_REUSEPORT sockets with bpf_sk_assign") > Reported-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > Signed-off-by: Lorenz Bauer <lmb@isovalent.com> > --- > include/net/inet6_hashtables.h | 10 ---------- > include/net/inet_hashtables.h | 10 ---------- > 2 files changed, 20 deletions(-) > > diff --git a/include/net/inet6_hashtables.h b/include/net/inet6_hashtables.h > index 284b5ce7205d..f9907ed36d54 100644 > --- a/include/net/inet6_hashtables.h > +++ b/include/net/inet6_hashtables.h > @@ -119,16 +119,6 @@ struct sock *inet6_steal_sock(struct net *net, struct sk_buff *skb, int doff, > if (!prefetched) > return sk; > > - if (sk->sk_protocol == IPPROTO_TCP) { > - if (sk->sk_state != TCP_LISTEN) > - return sk; > - } else if (sk->sk_protocol == IPPROTO_UDP) { > - if (sk->sk_state != TCP_CLOSE) > - return sk; > - } else { > - return sk; > - } > - > reuse_sk = inet6_lookup_reuseport(net, sk, skb, doff, > saddr, sport, daddr, ntohs(dport), > ehashfn); > diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h > index 1177effabed3..57a46993383a 100644 > --- a/include/net/inet_hashtables.h > +++ b/include/net/inet_hashtables.h > @@ -465,16 +465,6 @@ struct sock *inet_steal_sock(struct net *net, struct sk_buff *skb, int doff, > if (!prefetched) > return sk; > > - if (sk->sk_protocol == IPPROTO_TCP) { > - if (sk->sk_state != TCP_LISTEN) > - return sk; > - } else if (sk->sk_protocol == IPPROTO_UDP) { > - if (sk->sk_state != TCP_CLOSE) > - return sk; > - } else { > - return sk; > - } > - > reuse_sk = inet_lookup_reuseport(net, sk, skb, doff, > saddr, sport, daddr, ntohs(dport), > ehashfn); > > --- > base-commit: eb62e6aef940fcb1879100130068369d4638088f > change-id: 20230808-bpf-next-a442a095562b > > Best regards,
| |