From mboxrd@z Thu Jan  1 00:00:00 1970
From: Willy Tarreau <w@1wt.eu>
Subject: Re: MSG_ZEROCOPY doesn't work on half-open TCP sockets
Date: Wed, 9 Jan 2019 15:29:13 +0100
Message-ID: <20190109142913.GF23422@1wt.eu>
References: <CAJPywTKGT8Giuegpm7B4rKp+Qjmz4mCDcZoCwyY63=3s4tqZyQ@mail.gmail.com>
 <CAJPywTKiX-VVebBY0o5S+dpj7K9NKP-ZBUj8jKYBm8YyrhY1cg@mail.gmail.com>
 <CAF=yD-JPwXWmHeHsaC0EcjVwsQSsPCQm+8V-PUxTU_VrW-pKcA@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Marek Majkowski <marek@cloudflare.com>,
        Willem de Bruijn <willemb@google.com>,
        Network Development <netdev@vger.kernel.org>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from wtarreau.pck.nerim.net ([62.212.114.60]:34083 "EHLO 1wt.eu"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1730605AbfAIO3V (ORCPT <rfc822;netdev@vger.kernel.org>);
        Wed, 9 Jan 2019 09:29:21 -0500
Content-Disposition: inline
In-Reply-To: <CAF=yD-JPwXWmHeHsaC0EcjVwsQSsPCQm+8V-PUxTU_VrW-pKcA@mail.gmail.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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