VSOCK: bind to random port for VMADDR_PORT_ANY
Subsystems (3)
PATCH 1/1 VSOCK: bind to random port for VMADDR_PORT_ANY
The old code always starts from fixed port for VMADDR_PORT_ANY. Sometimes when VMM crashed, there is still orphaned vsock which is waiting for close timer, then it could cause connection time out for new started VM if they are trying to connect to same port with same guest cid since the new packets could hit that orphaned vsock. We could also fix this by doing more in vhost_vsock_reset_orphans, but any way, it should be better to start from a random local port instead of a fixed one.
--- net/vmw_vsock/af_vsock.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
net/vmw_vsock/af_vsock.c +5 -1
@@ -107,6 +107,7 @@#include <linux/mutex.h> #include <linux/net.h> #include <linux/poll.h> +#include <linux/random.h> #include <linux/skbuff.h> #include <linux/smp.h> #include <linux/socket.h>
@@ -504,9 +505,12 @@ static void vsock_pending_work(struct work_struct *work)static int __vsock_bind_stream (struct vsock_sock *vsk,struct sockaddr_vm *addr) { -static u32 port =LAST_RESERVED_PORT +1 ; +static u32 port =0 ;struct sockaddr_vm new_addr; +if (!port) + port =prandom_u32 (); +vsock_addr_init (&new_addr, addr->svm_cid, addr->svm_port);if (addr->svm_port ==VMADDR_PORT_ANY ) {
Signature · 1 line
2.20.0.rc2.403.gdbc3b29805-goog
Quoted 8 lines · Lepton Wu
The old code always starts from fixed port for
VMADDR_PORT_ANY. Sometimes when VMM crashed, there is still orphaned vsock which is waiting for close timer, then it could cause connection time out for new started VM if they are trying to connect to same port with same guest cid since the new packets could hit that orphaned vsock. We could also fix this by doing more invhost_vsock_reset_orphans, but any way, it should be better to start from a random local port instead of a fixed one.
Quoted 4 lines
Signed-off-by Lepton Wu <[email protected]>--- net/vmw_vsock/af_vsock.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
Jorgen, Dexuan: Any objection to this? It also affects the other AF_VSOCK transports.
Quoted 29 lines
net/vmw_vsock/af_vsock.c +5
-1@@ -107,6 +107,7 @@#include <linux/mutex.h> #include <linux/net.h> #include <linux/poll.h> +#include <linux/random.h> #include <linux/skbuff.h> #include <linux/smp.h> #include <linux/socket.h> @@ -504,9 +505,12 @@ static void vsock_pending_work(struct work_struct *work)static int __vsock_bind_stream (struct vsock_sock *vsk,struct sockaddr_vm *addr) { -static u32 port =LAST_RESERVED_PORT +1 ; +static u32 port =0 ;struct sockaddr_vm new_addr; +if (!port) + port =prandom_u32 (); +vsock_addr_init (&new_addr, addr->svm_cid, addr->svm_port);if (addr->svm_port ==VMADDR_PORT_ANY ) {Signature · 1 line
2.20.0.rc2.403.gdbc3b29805-goog
Quoted 10 lines · Lepton Wu
Quoted 8 lines
The old code always starts from fixed port for
VMADDR_PORT_ANY. Sometimes when VMM crashed, there is still orphaned vsock which is waiting for close timer, then it could cause connection time out for new started VM if they are trying to connect to same port with same guest cid since the new packets could hit that orphaned vsock. We could also fix this by doing more invhost_vsock_reset_orphans, but any way, it should be better to start from a random local port instead of a fixed one.
Quoted 7 lines
Quoted 4 lines
Signed-off-by Lepton Wu <[email protected]>--- net/vmw_vsock/af_vsock.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)Jorgen, Dexuan: Any objection to this? It also affects the other
AF_VSOCKtransports.
Makes sense to me.
Quoted 20 lines
Quoted 20 lines
net/vmw_vsock/af_vsock.c +2
-1@@ -107,6 +107,7 @@#include <linux/mutex.h> #include <linux/net.h> #include <linux/poll.h> +#include <linux/random.h> #include <linux/skbuff.h> #include <linux/smp.h> #include <linux/socket.h> @@ -504,9 +505,12 @@ static void vsock_pending_work(struct work_struct *work)static int __vsock_bind_stream (struct vsock_sock *vsk,struct sockaddr_vm *addr) { -static u32 port =LAST_RESERVED_PORT +1 ; +static u32 port =0 ;struct sockaddr_vm new_addr;
Quoted 3 lines
net/vmw_vsock/af_vsock.c static void vsock_pending_work(struct work_struct *work) · line 513
Quoted 3 lines
net/vmw_vsock/af_vsock.c +3
-0+if (!port) + port =prandom_u32 (); +
How about making this:
port = LAST_RESERVED_PORT + 1 + prandom_u32_max(U32_MAX - LAST_RESERVED_PORT);
so the initial assignment is a valid port in the unreserved range. It will be corrected in the first iteration, but this would make the intention clearer.
Quoted 6 lines
Quoted 6 lines
net/vmw_vsock/af_vsock.c +0
-1vsock_addr_init (&new_addr, addr->svm_cid, addr->svm_port);if (addr->svm_port ==VMADDR_PORT_ANY ) { --2.20.0.rc2.403.gdbc3b29805-goog
Thanks,
Jorgen
PATCH v2 1/1 VSOCK: bind to random port for VMADDR_PORT_ANY
The old code always starts from fixed port for VMADDR_PORT_ANY. Sometimes when VMM crashed, there is still orphaned vsock which is waiting for close timer, then it could cause connection time out for new started VM if they are trying to connect to same port with same guest cid since the new packets could hit that orphaned vsock. We could also fix this by doing more in vhost_vsock_reset_orphans, but any way, it should be better to start from a random local port instead of a fixed one.
--- net/vmw_vsock/af_vsock.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
net/vmw_vsock/af_vsock.c +6 -1
@@ -107,6 +107,7 @@#include <linux/mutex.h> #include <linux/net.h> #include <linux/poll.h> +#include <linux/random.h> #include <linux/skbuff.h> #include <linux/smp.h> #include <linux/socket.h>
@@ -504,9 +505,13 @@ static void vsock_pending_work(struct work_struct *work)static int __vsock_bind_stream (struct vsock_sock *vsk,struct sockaddr_vm *addr) { -static u32 port =LAST_RESERVED_PORT +1 ; +static u32 port =0 ;struct sockaddr_vm new_addr; +if (!port) + port =LAST_RESERVED_PORT +1 + +prandom_u32_max (U32_MAX -LAST_RESERVED_PORT ); +vsock_addr_init (&new_addr, addr->svm_cid, addr->svm_port);if (addr->svm_port ==VMADDR_PORT_ANY ) {
Signature · 1 line
2.20.0.405.gbc1bbc6f85-goog
Quoted 17 lines
From: Stefan Hajnoczi <[email protected]>
Sent: Tuesday, December 11, 2018 8:27 AM
To: Jorgen Hansen <[email protected]>; Dexuan Cui <[email protected]>
Cc: [email protected]; Lepton Wu <[email protected]>
Subject: Re: [PATCH] VSOCK: bind to random port forVMADDR_PORT_ANYQuoted 1 line · Lepton Wu
The old code always starts from fixed port for
VMADDR_PORT_ANY.Sometimes
Quoted 7 lines
when VMM crashed, there is still orphaned vsock which is waiting for close timer, then it could cause connection time out for new started VM if they are trying to connect to same port with same guest cid since the new packets could hit that orphaned vsock. We could also fix this by doing more in
vhost_vsock_reset_orphans, but any way, it should be better to start from a random local port instead of a fixed one.
Quoted 7 lines
Quoted 4 lines
Signed-off-by Lepton Wu <[email protected]>--- net/vmw_vsock/af_vsock.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)Jorgen, Dexuan: Any objection to this? It also affects the other
AF_VSOCKtransports.
Thanks for letting me notice the patch. :-)
Now I saw Lepton's v2 patch, which looks good to me.
Thanks,
-- Dexuan
Quoted 41 lines
The old code always starts from fixed port for
VMADDR_PORT_ANY. Sometimes when VMM crashed, there is still orphaned vsock which is waiting for close timer, then it could cause connection time out for new started VM if they are trying to connect to same port with same guest cid since the new packets could hit that orphaned vsock. We could also fix this by doing more invhost_vsock_reset_orphans, but any way, it should be better to start from a random local port instead of a fixed one.Signed-off-by Lepton Wu <[email protected]>--- net/vmw_vsock/af_vsock.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)net/vmw_vsock/af_vsock.c index renamed from net/vmw_vsock/af_vsock.c +0
-0
ab27a2872935..43a1dec08825100644net/vmw_vsock/af_vsock.c +6
-1@@ -107,6 +107,7 @@#include <linux/mutex.h> #include <linux/net.h> #include <linux/poll.h> +#include <linux/random.h> #include <linux/skbuff.h> #include <linux/smp.h> #include <linux/socket.h> @@ -504,9 +505,13 @@ static void vsock_pending_work(struct work_struct *work) static int __vsock_bind_stream(struct vsock_sock *vsk,struct sockaddr_vm *addr) { -static u32 port =LAST_RESERVED_PORT +1 ; +static u32 port =0 ;struct sockaddr_vm new_addr; +if (!port) + port =LAST_RESERVED_PORT +1 + +prandom_u32_max (U32_MAX -LAST_RESERVED_PORT ); +vsock_addr_init (&new_addr, addr->svm_cid, addr->svm_port);
Quoted 3 lines
if (
addr->svm_port==VMADDR_PORT_ANY) {
--
2.20.0.405.gbc1bbc6f85-goog
Thanks for the update - looks good to me.
/jsh
From: Lepton Wu <[email protected]>
Date: Tue, 11 Dec 2018 11:12:55 -0800
Quoted 9 lines
The old code always starts from fixed port for
VMADDR_PORT_ANY. Sometimes when VMM crashed, there is still orphaned vsock which is waiting for close timer, then it could cause connection time out for new started VM if they are trying to connect to same port with same guest cid since the new packets could hit that orphaned vsock. We could also fix this by doing more invhost_vsock_reset_orphans, but any way, it should be better to start from a random local port instead of a fixed one.Signed-off-by Lepton Wu <[email protected]>
Applied, thanks.