netdev - Re: [PATCH 2/2] vsock/virtio: Don't reset the created SOCKET during s2r

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: <rnri3i5jues4rjgtb36purbjmct56u4m5e6swaqb3smevtlozw@ki7gdlbdbmve>
Date: Mon, 10 Feb 2025 12:48:03 +0100
From: leonardi@...hat.com
To: Junnan Wu <junnan01.wu@...sung.com>
Cc: stefanha@...hat.com, sgarzare@...hat.com, kvm@...r.kernel.org, 
	virtualization@...ts.linux-foundation.org, netdev@...r.kernel.org, mindong.zhao@...sung.com, 
	q1.huang@...sung.com, ying01.gao@...sung.com, ying123.xu@...sung.com
Subject: Re: [PATCH 2/2] vsock/virtio: Don't reset the created SOCKET during
 s2r

Like for the other patch, some maintainers have not been CCd.

On Fri, Feb 07, 2025 at 01:20:33PM +0800, Junnan Wu wrote:
>From: Ying Gao <ying01.gao@...sung.com>
>
>If suspend is executed during vsock communication and the
>socket is reset, the original socket will be unusable after resume.
>
>Judge the value of vdev->priv in function virtio_vsock_vqs_del,
>only when the function is invoked by virtio_vsock_remove,
>all vsock connections will be reset.
>
The second part of the commit message is not that clear, do you mind 
rephrasing it?

>Signed-off-by: Ying Gao <ying01.gao@...sung.com>
Missing Co-developed-by?
>Signed-off-by: Junnan Wu <junnan01.wu@...sung.com>


>---
> net/vmw_vsock/virtio_transport.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index 9eefd0fba92b..9df609581755 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -717,8 +717,10 @@ static void virtio_vsock_vqs_del(struct virtio_vsock *vsock)
> 	struct sk_buff *skb;
>
> 	/* Reset all connected sockets when the VQs disappear */
>-	vsock_for_each_connected_socket(&virtio_transport.transport,
>-					virtio_vsock_reset_sock);
I would add a comment explaining why you are adding this check.
>+	if (!vdev->priv) {
>+		vsock_for_each_connected_socket(&virtio_transport.transport,
>+						virtio_vsock_reset_sock);
>+	}
>
> 	/* Stop all work handlers to make sure no one is accessing the device,
> 	 * so we can safely call virtio_reset_device().
>-- 
>2.34.1
>

I am not familiar with freeze/resume, but I don't see any problems with 
this patch.

Thank you,
Luigi