netdev - Re: [net] net/tls: missing received data after fast remote close

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] [day] [month] [year] [list]

Message-ID: <20201118164913.3b8a34f3@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Wed, 18 Nov 2020 16:49:13 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Vadim Fedorenko <vfedorenko@...ek.ru>
Cc:     Boris Pismenny <borisp@...dia.com>,
        Aviad Yehezkel <aviadye@...dia.com>, netdev@...r.kernel.org
Subject: Re: [net] net/tls: missing received data after fast remote close

On Thu, 19 Nov 2020 00:26:52 +0000 Vadim Fedorenko wrote:
> > Damn, you may be seeing some problem I'm missing again ;) Running
> > __unparse can be opportunistic, if it doesn't parse anything that's
> > fine. I was thinking:
> >
> > diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> > index 95ab5545a931..6478bd968506 100644
> > --- a/net/tls/tls_sw.c
> > +++ b/net/tls/tls_sw.c
> > @@ -1295,6 +1295,10 @@ static struct sk_buff *tls_wait_data(struct sock *sk, struct sk_psock *psock,
> >                          return NULL;
> >                  }
> >   
> > +               __strp_unpause(&ctx->strp);
> > +               if (ctx->recv_pkt)
> > +                       return ctx->recv_pkt;
> > +
> >                  if (sk->sk_shutdown & RCV_SHUTDOWN)
> >                          return NULL;
> >   
> > Optionally it would be nice if unparse cancelled the work if it managed
> > to parse the record out.  
> Oh, simple and fine solution. But is it better to unpause parser conditionally when
> there is something in the socket queue? Otherwise this call will be just wasting
> cycles. Maybe like this:
> 
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 2fe9e2c..97c5f6e 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -1295,6 +1295,12 @@ static struct sk_buff *tls_wait_data(struct sock *sk, 
> struct sk_psock *psock,
>                          return NULL;
>                  }
> 
> +               if (!skb_queue_empty(&sk->sk_receive_queue)) {
> +                       __strp_unpause(&ctx->strp);
> +                       if (ctx->recv_pkt)
> +                               return ctx->recv_pkt;
> +               }
> +
>                  if (sk->sk_shutdown & RCV_SHUTDOWN)
>                          return NULL;
> 

LGTM!