netdev - Re: [syzbot] [kvm?] [net?] [virt?] WARNING in virtio_transport_send_pkt

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

Message-ID: <aJ9WsFovkgZM3z09@willie-the-truck>
Date: Fri, 15 Aug 2025 16:48:00 +0100
From: Will Deacon <will@...nel.org>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: syzbot <syzbot+b4d960daf7a3c7c2b7b1@...kaller.appspotmail.com>,
	davem@...emloft.net, edumazet@...gle.com, eperezma@...hat.com,
	horms@...nel.org, jasowang@...hat.com, kuba@...nel.org,
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, pabeni@...hat.com, sgarzare@...hat.com,
	stefanha@...hat.com, syzkaller-bugs@...glegroups.com,
	virtualization@...ts.linux.dev, xuanzhuo@...ux.alibaba.com
Subject: Re: [syzbot] [kvm?] [net?] [virt?] WARNING in
 virtio_transport_send_pkt_info

On Fri, Aug 15, 2025 at 01:00:59PM +0100, Will Deacon wrote:
> On Fri, Aug 15, 2025 at 06:44:47AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Aug 15, 2025 at 11:09:24AM +0100, Will Deacon wrote:
> > > On Tue, Aug 12, 2025 at 06:15:46AM -0400, Michael S. Tsirkin wrote:
> > > > On Tue, Aug 12, 2025 at 03:03:02AM -0700, syzbot wrote:
> > > > > Hello,
> > > > > 
> > > > > syzbot has tested the proposed patch but the reproducer is still triggering an issue:
> > > > > WARNING in virtio_transport_send_pkt_info
> > > > 
> > > > OK so the issue triggers on
> > > > commit 6693731487a8145a9b039bc983d77edc47693855
> > > > Author: Will Deacon <will@...nel.org>
> > > > Date:   Thu Jul 17 10:01:16 2025 +0100
> > > > 
> > > >     vsock/virtio: Allocate nonlinear SKBs for handling large transmit buffers
> > > >     
> > > > 
> > > > but does not trigger on:
> > > > 
> > > > commit 8ca76151d2c8219edea82f1925a2a25907ff6a9d
> > > > Author: Will Deacon <will@...nel.org>
> > > > Date:   Thu Jul 17 10:01:15 2025 +0100
> > > > 
> > > >     vsock/virtio: Rename virtio_vsock_skb_rx_put()
> > > >     
> > > > 
> > > > 
> > > > Will, I suspect your patch merely uncovers a latent bug
> > > > in zero copy handling elsewhere.
> 
> I'm still looking at this, but I'm not sure zero-copy is the right place
> to focus on.
> 
> The bisected patch 6693731487a8 ("vsock/virtio: Allocate nonlinear SKBs
> for handling large transmit buffers") only has two hunks. The first is
> for the non-zcopy case and the latter is a no-op for zcopy, as
> skb_len == VIRTIO_VSOCK_SKB_HEADROOM and so we end up with a linear SKB
> regardless.

It's looking like this is caused by moving from memcpy_from_msg() to
skb_copy_datagram_from_iter(), which is necessary to handle non-linear
SKBs correctly.

In the case of failure (i.e. faulting on the source and returning
-EFAULT), memcpy_from_msg() rewinds the message iterator whereas
skb_copy_datagram_from_iter() does not. If we have previously managed to
transmit some of the packet, then I think
virtio_transport_send_pkt_info() can end up returning a positive "bytes
written" error code and the caller will call it again. If we've advanced
the message iterator, then this can end up with the reported warning if
we run out of input data.

As a hack (see below), I tried rewinding the iterator in the error path
of skb_copy_datagram_from_iter() but I'm not sure whether other callers
would be happy with that. If not, then we could save/restore the
iterator state in virtio_transport_fill_skb() if the copy fails. Or we
could add a variant of skb_copy_datagram_from_iter(), say
skb_copy_datagram_from_iter_full(), which has the rewind behaviour.

What do you think?

Will

--->8

diff --git a/net/core/datagram.c b/net/core/datagram.c
index 94cc4705e91d..62e44ab136b7 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -551,7 +551,7 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
 				 int len)
 {
 	int start = skb_headlen(skb);
-	int i, copy = start - offset;
+	int i, copy = start - offset, start_off = offset;
 	struct sk_buff *frag_iter;
 
 	/* Copy header. */
@@ -614,6 +614,7 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
 		return 0;
 
 fault:
+	iov_iter_revert(from, offset - start_off);
 	return -EFAULT;
 }
 EXPORT_SYMBOL(skb_copy_datagram_from_iter);