On Wed, 10 Dec 2025 17:31:25 +0900, Jakub Kicinski wrote: > > In zerocopy_fill_skb_from_iter(), if two copy operations are performed > > and the first one succeeds while the second one fails, it returns a > > failure but the count in iterator has already been decremented due to > > the first successful copy. This ultimately affects the local variable > > rest_len in virtio_transport_send_pkt_info(), causing the remaining > > count in rest_len to be greater than the actual iterator count. As a > > result, packet sending operations continue even when the iterator count > > is zero, which further leads to skb->len being 0 and triggers the warning > > reported by syzbot [1]. > > > > Therefore, if the zerocopy operation fails, we should revert the iterator > > to its original state. > > > > The iov_iter_revert() in skb_zerocopy_iter_stream() is no longer needed > > and has been removed. > > > > [1] > > 'send_pkt()' returns 0, but 4096 expected > > WARNING: net/vmw_vsock/virtio_transport_common.c:430 at > > virtio_transport_send_pkt_info+0xd1e/0xef0 > > net/vmw_vsock/virtio_transport_common.c:428, CPU#1: syz.0.17/5986 > > Call Trace: > > virtio_transport_stream_enqueue > > net/vmw_vsock/virtio_transport_common.c:1113 [inline] > > virtio_transport_seqpacket_enqueue+0x143/0x1c0 > > net/vmw_vsock/virtio_transport_common.c:841 > > vsock_connectible_sendmsg+0xabf/0x1040 net/vmw_vsock/af_vsock.c:2158 > > sock_sendmsg_nosec net/socket.c:727 [inline] > > __sock_sendmsg+0x21c/0x270 net/socket.c:746 > > > > Reported-by: [email protected] > > Closes: https://syzkaller.appspot.com/bug?extid=28e5f3d207b14bae122a > > Signed-off-by: Edward Adam Davis <[email protected]> > > --- > > v3: > > - fix test tcp_zerocopy_maxfrags timeout > > v2: > > https://lore.kernel.org/all/[email protected]/ > > - Remove iov_iter_revert() in skb_zerocopy_iter_stream() > > v1: > > https://lore.kernel.org/all/[email protected]/ > > Have you investigated the other callers? Given problems with previous > version of this patch I'm worried you have not. If you did please extend > the commit message with the appropriate explanation. Are you asking if I investigated other zerocopy tests? NO. The test results [T2] for this version of the patch do not show any failures related to zerocopy.
[T2] https://patchwork.kernel.org/project/netdevbpf/patch/[email protected] > > Alternative would be to add a _full() flavor of this API, but not sure > if other callers actually care. > > > diff --git a/net/core/datagram.c b/net/core/datagram.c > > index c285c6465923..3a612ebbbe80 100644 > > --- a/net/core/datagram.c > > +++ b/net/core/datagram.c > > @@ -748,9 +748,13 @@ int __zerocopy_sg_from_iter(struct msghdr *msg, struct > > sock *sk, > > size_t length, > > struct net_devmem_dmabuf_binding *binding) > > { > > + struct iov_iter_state state; > > nit: if you respin move this one line down OK. > > > unsigned long orig_size = skb->truesize; > > unsigned long truesize; > > - int ret; > > + int ret, orig_len; > > + > > + iov_iter_save_state(from, &state); > > + orig_len = skb->len; > > > > if (msg && msg->msg_ubuf && msg->sg_from_iter) > > ret = msg->sg_from_iter(skb, from, length); > > @@ -759,6 +763,9 @@ int __zerocopy_sg_from_iter(struct msghdr *msg, struct > > sock *sk, > > else > > ret = zerocopy_fill_skb_from_iter(skb, from, length); > > > > + if (ret == -EFAULT || (ret == -EMSGSIZE && skb->len == orig_len)) > > I'd think that for the purpose of reverting iter the second part of > this condition is completely moot. If skb len didn't change there should > be nothing to revert? Regarding the second judgment condition, I aligned it with the condition in skb_zerocopy_iter_stream(). During my testing, I only encountered -EFAULT failures, not -EMSGSIZE. > > > + iov_iter_restore(from, &state); > > + > > truesize = skb->truesize - orig_size; > > if (sk && sk->sk_type == SOCK_STREAM) { > > sk_wmem_queued_add(sk, truesize); > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > > index a00808f7be6a..7b8836f668b7 100644 > > --- a/net/core/skbuff.c > > +++ b/net/core/skbuff.c > > @@ -1908,7 +1908,6 @@ int skb_zerocopy_iter_stream(struct sock *sk, struct > > sk_buff *skb, > > struct sock *save_sk = skb->sk; > > > > /* Streams do not free skb on error. Reset to prev state. */ > > - iov_iter_revert(&msg->msg_iter, skb->len - orig_len); > > skb->sk = sk; > > ___pskb_trim(skb, orig_len); > > skb->sk = save_sk;
