Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] vhost/vsock: missing dgram_allow check in vhost_vsock_handle_tx_kick() --
commit 635912e90dc603f9b7384d44cf174e18056f799d Author: Sergio Lopez <[email protected]> vhost/vsock: negotiate datagram support with guests This commit exposes VIRTIO_VSOCK_F_DGRAM in the vhost feature bitmap and tracks whether datagram support was negotiated per guest via vsock->dgram_allow. It also wires up vhost_transport_dgram_allow() to check the negotiated feature on the target guest for outbound datagrams. > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c > index 0cb87971d7491..eb2fbdb721aa9 100644 > --- a/drivers/vhost/vsock.c > +++ b/drivers/vhost/vsock.c [ ... ] > @@ -516,6 +520,24 @@ static bool vhost_transport_seqpacket_allow(struct > vsock_sock *vsk, > return seqpacket_allow; > } > > +static bool vhost_transport_dgram_allow(struct vsock_sock *vsk, > + u32 cid, u32 port) > +{ > + struct net *net = sock_net(sk_vsock(vsk)); > + struct vhost_vsock *vsock; > + bool dgram_allow = false; > + > + rcu_read_lock(); > + vsock = vhost_vsock_get(cid, net); > + > + if (vsock) > + dgram_allow = vsock->dgram_allow; > + > + rcu_read_unlock(); > + > + return dgram_allow; > +} > + > static void vhost_vsock_handle_tx_kick(struct vhost_work *work) > { > struct vhost_virtqueue *vq = container_of(work, struct vhost_virtqueue, [Severity: Medium] Can a guest that has not negotiated VIRTIO_VSOCK_F_DGRAM still send VIRTIO_VSOCK_TYPE_DGRAM packets to unconnected bound SOCK_DGRAM sockets on the host? vhost_transport_dgram_allow() checks vsock->dgram_allow on outbound host connect() and sendmsg() paths, but on the guest-to-host path in drivers/vhost/vsock.c:vhost_vsock_handle_tx_kick(), incoming packets are passed to virtio_transport_recv_pkt() without checking vsock->dgram_allow: drivers/vhost/vsock.c:vhost_vsock_handle_tx_kick() { ... /* Only accept correctly addressed packets */ if (le64_to_cpu(hdr->src_cid) == vsock->guest_cid && le64_to_cpu(hdr->dst_cid) == vhost_transport_get_local_cid()) virtio_transport_recv_pkt(&vhost_transport, skb, vsock->net); else kfree_skb(skb); ... } Unlike VIRTIO_VSOCK_TYPE_SEQPACKET, where inbound connection requests go through virtio_transport_recv_listen() -> vsock_assign_transport() -> vhost_transport_seqpacket_allow(), inbound VIRTIO_VSOCK_TYPE_DGRAM packets in net/vmw_vsock/virtio_transport_common.c:virtio_transport_recv_pkt() are queued directly onto bound unconnected SOCK_DGRAM sockets without calling dgram_allow(): net/vmw_vsock/virtio_transport_common.c:virtio_transport_recv_pkt() { ... if (le16_to_cpu(hdr->type) == VIRTIO_VSOCK_TYPE_DGRAM) { if (le16_to_cpu(hdr->op) != VIRTIO_VSOCK_OP_RW) goto free_pkt; sk = vsock_find_bound_socket_net(&dst, net); ... if (sock_flag(sk, SOCK_DEAD) || vsk->transport != &t->transport || (sk->sk_socket->state != SS_UNCONNECTED && !vsock_check_source(vsk, &t->transport, &src))) { release_sock(sk); sock_put(sk); goto free_pkt; } if (sock_queue_rcv_skb(sk, skb)) kfree_skb(skb); ... } Even with the later virtio_transport_dgram_source_allow() check, unconnected bound SOCK_DGRAM sockets return true before consulting dgram_allow(). Should vhost_vsock_handle_tx_kick() drop VIRTIO_VSOCK_TYPE_DGRAM packets when !vsock->dgram_allow, or should virtio_transport_recv_pkt() validate dgram_allow() for incoming datagrams regardless of whether the receiving socket is connected? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
