Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] vsock/virtio: premature credit_update clear in virtio_transport_send_skb() --
commit 2cdb1a5b77890d0e5001ce1a907ca3477d945d5d Author: Oleg Drokin <[email protected]> vsock/virtio: queue at most one credit update per socket This commit limits each vsock socket to at most one queued credit update packet when its last queued packet is already an OP_CREDIT_UPDATE. When the transport transmits the queued credit update and no newer packet has been queued after it, virtio_transport_credit_update_xmit() refreshes its fwd_cnt and buf_alloc and clears vvs->credit_update. > diff --git a/net/vmw_vsock/virtio_transport.c > b/net/vmw_vsock/virtio_transport.c > index 4f9aa9c4c3aa5..82cd52249de3c 100644 > --- a/net/vmw_vsock/virtio_transport.c > +++ b/net/vmw_vsock/virtio_transport.c > @@ -104,6 +104,8 @@ static int virtio_transport_send_skb(struct sk_buff *skb, > struct virtqueue *vq, > int ret, in_sg = 0, out_sg = 0; > struct scatterlist **sgs; > > + virtio_transport_credit_update_xmit(skb); > + > sgs = vsock->out_sgs; > sg_init_one(sgs[out_sg], virtio_vsock_hdr(skb), > sizeof(*virtio_vsock_hdr(skb))); [Severity: High] Does calling virtio_transport_credit_update_xmit(skb) before virtqueue_add_sgs() clear vvs->credit_update and advance vvs->last_fwd_cnt even when adding the packet to the tx virtqueue fails? In virtio_transport_send_skb(), virtqueue_add_sgs() can fail (for example with -ENOSPC when VSOCK_VQ_TX is full, or -ENOMEM under GFP_ATOMIC on the fast path): net/vmw_vsock/virtio_transport.c:virtio_transport_send_skb() { virtio_transport_credit_update_xmit(skb); ... ret = virtqueue_add_sgs(vq, sgs, out_sg, in_sg, skb, gfp); if (ret < 0) return ret; } When that happens, virtio_transport_credit_update_xmit(skb) has already set vvs->credit_update = NULL and vvs->last_fwd_cnt = vvs->fwd_cnt, and the caller then queues or requeues skb in vsock->send_pkt_queue: - via virtio_transport_send_pkt() -> virtio_transport_send_skb_fast_path(), which falls back to virtio_vsock_skb_queue_tail(&vsock->send_pkt_queue, skb) when vsock->send_pkt_queue was initially empty, or - via virtio_transport_send_pkt_work(), which puts skb back with virtio_vsock_skb_queue_head(&vsock->send_pkt_queue, skb). While skb sits in vsock->send_pkt_queue with vvs->credit_update cleared to NULL, subsequent calls to virtio_transport_send_credit_update() (from OP_CREDIT_REQUEST or local reads) will queue additional OP_CREDIT_UPDATE skbs behind it. In addition, when skb is later sent by virtio_transport_send_pkt_work(), virtio_transport_credit_update_xmit(skb) sees vvs->credit_update != skb and will not refresh hdr->fwd_cnt or hdr->buf_alloc with credits freed while it was queued. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
