Skip to content

Commit 7fb1291

Browse files
willdeaconkuba-moo
authored andcommitted
vsock/virtio: Fix message iterator handling on transmit path
Commit 6693731 ("vsock/virtio: Allocate nonlinear SKBs for handling large transmit buffers") converted the virtio vsock transmit path to utilise nonlinear SKBs when handling large buffers. As part of this change, virtio_transport_fill_skb() was updated to call skb_copy_datagram_from_iter() instead of memcpy_from_msg() as the latter expects a single destination buffer and cannot handle nonlinear SKBs correctly. Unfortunately, during this conversion, I overlooked the error case when the copying function returns -EFAULT due to a fault on the input buffer in userspace. In this case, memcpy_from_msg() reverts the iterator to its initial state thanks to copy_from_iter_full() whereas skb_copy_datagram_from_iter() leaves the iterator partially advanced. This results in a WARN_ONCE() from the vsock code, which expects the iterator to stay in sync with the number of bytes transmitted so that virtio_transport_send_pkt_info() can return -EFAULT when it is called again: ------------[ cut here ]------------ 'send_pkt()' returns 0, but 65536 expected WARNING: CPU: 0 PID: 5503 at net/vmw_vsock/virtio_transport_common.c:428 virtio_transport_send_pkt_info+0xd11/0xf00 net/vmw_vsock/virtio_transport_common.c:426 Modules linked in: CPU: 0 UID: 0 PID: 5503 Comm: syz.0.17 Not tainted 6.16.0-syzkaller-12063-g37816488247d #0 PREEMPT(full) Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014 Call virtio_transport_fill_skb_full() to restore the previous iterator behaviour. Cc: Jason Wang <[email protected]> Cc: Stefano Garzarella <[email protected]> Fixes: 6693731 ("vsock/virtio: Allocate nonlinear SKBs for handling large transmit buffers") Reported-by: [email protected] Signed-off-by: Will Deacon <[email protected]> Acked-by: Michael S. Tsirkin <[email protected]> Reviewed-by: Stefan Hajnoczi <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent b08a784 commit 7fb1291

File tree

1 file changed

+5
-3
lines changed

1 file changed

+5
-3
lines changed

net/vmw_vsock/virtio_transport_common.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,14 @@ static int virtio_transport_fill_skb(struct sk_buff *skb,
105105
size_t len,
106106
bool zcopy)
107107
{
108+
struct msghdr *msg = info->msg;
109+
108110
if (zcopy)
109-
return __zerocopy_sg_from_iter(info->msg, NULL, skb,
110-
&info->msg->msg_iter, len, NULL);
111+
return __zerocopy_sg_from_iter(msg, NULL, skb,
112+
&msg->msg_iter, len, NULL);
111113

112114
virtio_vsock_skb_put(skb, len);
113-
return skb_copy_datagram_from_iter(skb, 0, &info->msg->msg_iter, len);
115+
return skb_copy_datagram_from_iter_full(skb, 0, &msg->msg_iter, len);
114116
}
115117

116118
static void virtio_transport_init_hdr(struct sk_buff *skb,

0 commit comments

Comments
 (0)