netdev - Re: [PATCH 1/5] virtio-net: fix overflow inside virtnet_rq

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: <20241025132205.GV1202098@kernel.org>
Date: Fri, 25 Oct 2024 14:22:05 +0100
From: Simon Horman <horms@...nel.org>
To: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
Cc: Paolo Abeni <pabeni@...hat.com>, "Michael S. Tsirkin" <mst@...hat.com>,
	Jason Wang <jasowang@...hat.com>,
	Eugenio Pérez <eperezma@...hat.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, virtualization@...ts.linux.dev,
	Si-Wei Liu <si-wei.liu@...cle.com>, netdev@...r.kernel.org
Subject: Re: [PATCH 1/5] virtio-net: fix overflow inside virtnet_rq_alloc

On Fri, Oct 25, 2024 at 10:35:53AM +0800, Xuan Zhuo wrote:
> On Thu, 17 Oct 2024 15:42:59 +0200, Paolo Abeni <pabeni@...hat.com> wrote:
> >
> >
> > On 10/14/24 05:12, Xuan Zhuo wrote:
> > > When the frag just got a page, then may lead to regression on VM.
> > > Specially if the sysctl net.core.high_order_alloc_disable value is 1,
> > > then the frag always get a page when do refill.
> > >
> > > Which could see reliable crashes or scp failure (scp a file 100M in size
> > > to VM):
> > >
> > > The issue is that the virtnet_rq_dma takes up 16 bytes at the beginning
> > > of a new frag. When the frag size is larger than PAGE_SIZE,
> > > everything is fine. However, if the frag is only one page and the
> > > total size of the buffer and virtnet_rq_dma is larger than one page, an
> > > overflow may occur.
> > >
> > > Here, when the frag size is not enough, we reduce the buffer len to fix
> > > this problem.
> > >
> > > Fixes: f9dac92ba908 ("virtio_ring: enable premapped mode whatever use_dma_api")
> > > Reported-by: "Si-Wei Liu" <si-wei.liu@...cle.com>
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
> >
> > This looks like a fix that should target the net tree, but the following
> > patches looks like net-next material. Any special reason to bundle them
> > together?
> 
> Sorry, I forgot to add net-next as a target tree.
> 
> This may look like a fix. But the feature was disabled in the last Linux
> version. So the bug cannot be triggered, so we don't need to push to the net
> tree.

I think it would be useful to be clear in the commit message, use of tags,
and target tree regarding fixes and non-fixes.

Please describe in the commit message why this is not fixing a bug, as you
have described above. And please do not include Fixes tags in patches that
are not bug fixes, which seems to be the case here.

If you want to refer to the patch that introduced the problem, you can use
the following syntax, in the commit message, before the tags. Unlike Fixes
tags, this may be line wrapped.

  This problem is not a bug fix for net because... It was was introduced by
  commit f9dac92ba908 ("virtio_ring: enable premapped mode whatever use_dma_api").

  Reported-by: ...
  ...