netdev - Re: [PATCH net] tun: fix return value when the number of iovs exceeds MAX_SKB

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: <20201227061540-mutt-send-email-mst@kernel.org>
Date:   Sun, 27 Dec 2020 06:18:55 -0500
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     wangyunjian <wangyunjian@...wei.com>,
        Network Development <netdev@...r.kernel.org>,
        Jason Wang <jasowang@...hat.com>,
        virtualization@...ts.linux-foundation.org,
        "Lilijun (Jerry)" <jerry.lilijun@...wei.com>,
        chenchanghu <chenchanghu@...wei.com>,
        xudingke <xudingke@...wei.com>,
        "huangbin (J)" <brian.huangbin@...wei.com>
Subject: Re: [PATCH net] tun: fix return value when the number of iovs
 exceeds MAX_SKB_FRAGS

On Thu, Dec 24, 2020 at 10:56:16AM -0500, Willem de Bruijn wrote:
> On Thu, Dec 24, 2020 at 6:51 AM wangyunjian <wangyunjian@...wei.com> wrote:
> >
> > From: Yunjian Wang <wangyunjian@...wei.com>
> >
> > Currently the tun_napi_alloc_frags() function returns -ENOMEM when the
> > number of iovs exceeds MAX_SKB_FRAGS + 1. However this is inappropriate,
> > we should use -EMSGSIZE instead of -ENOMEM.
> >
> > Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
> > Signed-off-by: Yunjian Wang <wangyunjian@...wei.com>
> 
> Acked-by: Willem de Bruijn <willemb@...gle.com>
> 
> It might be good to explain why the distinction matters: one denotes a
> transient failure that the caller (specifically vhost_net) can retry,
> the other a persistent failure due to bad packet geometry that should
> be dropped.

It would be good to have a definition of what constitutes a transient
failure. At the moment there's a proposed patch to vhost that
tests for 
	err == -EAGAIN || err == -ENOMEM || err == -ENOBUFS
and we'll likely add EIO to that.

Alternatively, I'm inclined to say any invalid input should
just return EINVAL except maybe for memory access errors
which traditionally are EFAULT.
Then vhost can handle any failure except EINVAL and EFAULT
as transient.

-- 
MST