linux-kernel - Re: [PATCH v1] VSOCK: fix Information Leak in virtio_transport

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: <4sbamls46k3dxlqgreifhhhd66iaosbeoxgbpyvwaipwlnwiba@dep4mseknust>
Date: Tue, 5 Aug 2025 09:00:50 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: bsdhenrymartin@...il.com
Cc: huntazhang@...cent.com, jitxie@...cent.com, landonsun@...cent.com, 
	stefanha@...hat.com, mst@...hat.com, jasowang@...hat.com, xuanzhuo@...ux.alibaba.com, 
	eperezma@...hat.com, davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, 
	pabeni@...hat.com, horms@...nel.org, kvm@...r.kernel.org, 
	virtualization@...ts.linux.dev, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Henry Martin <bsdhenryma@...cent.com>, TCS Robot <tcs_robot@...cent.com>
Subject: Re: [PATCH v1] VSOCK: fix Information Leak in
 virtio_transport_shutdown()

On Tue, Aug 05, 2025 at 01:10:09PM +0800, bsdhenrymartin@...il.com wrote:
>From: Henry Martin <bsdhenryma@...cent.com>
>
>The `struct virtio_vsock_pkt_info` is declared on the stack but only
>partially initialized (only `op`, `flags`, and `vsk` are set)
>
>The uninitialized fields (including `pkt_len`, `remote_cid`,
>`remote_port`, etc.) contain residual kernel stack data. This structure
>is passed to `virtio_transport_send_pkt_info()`, which uses the
>uninitialized fields.
>
>Fixes: 06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko")
>Reported-by: TCS Robot <tcs_robot@...cent.com>
>Signed-off-by: Henry Martin <bsdhenryma@...cent.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index fe92e5fa95b4..cb391a98d025 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1073,14 +1073,14 @@ EXPORT_SYMBOL_GPL(virtio_transport_connect);
>
> int virtio_transport_shutdown(struct vsock_sock *vsk, int mode)
> {
>-	struct virtio_vsock_pkt_info info = {
>-		.op = VIRTIO_VSOCK_OP_SHUTDOWN,
>-		.flags = (mode & RCV_SHUTDOWN ?
>-			  VIRTIO_VSOCK_SHUTDOWN_RCV : 0) |
>-			 (mode & SEND_SHUTDOWN ?
>-			  VIRTIO_VSOCK_SHUTDOWN_SEND : 0),
>-		.vsk = vsk,
>-	};

The compiler sets all other fields to 0, so I don't understand what this 
patch solves.
Can you give an example of the problem you found?

Furthermore, even if this fix were valid, why do it for just one 
function?

Stefano

>+	struct virtio_vsock_pkt_info info = {0};
>+
>+	info.op = VIRTIO_VSOCK_OP_SHUTDOWN;
>+	info.flags = (mode & RCV_SHUTDOWN ?
>+			VIRTIO_VSOCK_SHUTDOWN_RCV : 0) |
>+			(mode & SEND_SHUTDOWN ?
>+			VIRTIO_VSOCK_SHUTDOWN_SEND : 0);
>+	info.vsk = vsk;
>
> 	return virtio_transport_send_pkt_info(vsk, &info);
> }
>-- 
>2.41.3
>