lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <641a2b1c44846_80a24208b9@john.notmuch>
Date:   Tue, 21 Mar 2023 15:09:32 -0700
From:   John Fastabend <john.fastabend@...il.com>
To:     Eric Dumazet <edumazet@...gle.com>,
        John Fastabend <john.fastabend@...il.com>
Cc:     jakub@...udflare.com, daniel@...earbox.net, lmb@...valent.com,
        cong.wang@...edance.com, bpf@...r.kernel.org,
        netdev@...r.kernel.org, ast@...nel.org, andrii@...nel.org,
        will@...valent.com
Subject: Re: [PATCH bpf 05/11] bpf: sockmap, TCP data stall on recv before
 accept

Eric Dumazet wrote:
> On Tue, Mar 21, 2023 at 2:52 PM John Fastabend <john.fastabend@...il.com> wrote:
> >
> > A common mechanism to put a TCP socket into the sockmap is to hook the
> > BPF_SOCK_OPS_{ACTIVE_PASSIVE}_ESTABLISHED_CB event with a BPF program
> > that can map the socket info to the correct BPF verdict parser. When
> > the user adds the socket to the map the psock is created and the new
> > ops are assigned to ensure the verdict program will 'see' the sk_buffs
> > as they arrive.
> >

[...]

> >         lock_sock(sk);
> > +
> > +       /* We may have received data on the sk_receive_queue pre-accept and
> > +        * then we can not use read_skb in this context because we haven't
> > +        * assigned a sk_socket yet so have no link to the ops. The work-around
> > +        * is to check the sk_receive_queue and in these cases read skbs off
> > +        * queue again. The read_skb hook is not running at this point because
> > +        * of lock_sock so we avoid having multiple runners in read_skb.
> > +        */
> > +       if (unlikely(!skb_queue_empty_lockless(&sk->sk_receive_queue))) {
> 
> socket is locked here, please use skb_queue_empty() ?
> 
> We shall reserve skb_queue_empty_lockless() for lockless contexts.

Yep will do thanks.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ