netdev - Re: MSG_ZEROCOPY doesn't work on half-open TCP sockets

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: <20190109142913.GF23422@1wt.eu>
Date:   Wed, 9 Jan 2019 15:29:13 +0100
From:   Willy Tarreau <w@....eu>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     Marek Majkowski <marek@...udflare.com>,
        Willem de Bruijn <willemb@...gle.com>,
        Network Development <netdev@...r.kernel.org>
Subject: Re: MSG_ZEROCOPY doesn't work on half-open TCP sockets

On Wed, Jan 09, 2019 at 08:55:14AM -0500, Willem de Bruijn wrote:
> > In other words: because the socket needs to be ESTABLISHED for
> > MSG_ZEROCOPY to work, and because remote party can send FIN and move
> > the socket to CLOSE_WAIT, a sending party must implement a fallback
> > from EINVAL return code on the transmission code. An adversarial
> > client who does shutdown(SHUT_WR), will trigger EINVAL in the sender..
> 
> An adversarial client only affects its own stream, so the impact is limited.

Sure but it doesn't necessarily do it on purpose either :-) The typical

   echo -ne "GET /file HTTP/1.1\r\nHost: foo.example.com\r\n\r\n" | nc host port

is perfectly valid and will not work in this case, possibly forcing the newly
deployed component to toll back.

> Thanks for the report. At first blush it seems like extending the
> check to include state CLOSE_WAIT would resolve the issue
> 
>         if (flags & MSG_ZEROCOPY && size && sock_flag(sk, SOCK_ZEROCOPY)) {
> -               if (sk->sk_state != TCP_ESTABLISHED) {
> +               if ((1 << sk->sk_state) & ~(TCPF_ESTABLISHED |
> TCPF_CLOSE_WAIT)) {
> 
>                         err = -EINVAL;
>                         goto out_err;
>                 }

At first glance I think it should do the job.

Willy