[PATCH] usbip: Close all listening sockets in request workers

[PATCH] usbip: Close all listening sockets in request workers

[raoxu]

From: Xu Rao <raoxu@uniontech.com>

process_request() forks a worker after accepting a connection, but the
child only closes the listener used for that connection. All other
listening sockets are inherited and remain open until the worker exits.
This occurs when usbipd listens on multiple addresses, including the
default IPv4 and IPv6 setup.

A worker waiting for a client request can therefore keep an unrelated
listener alive after the main daemon exits. Restarting usbipd then fails
to bind that address with EADDRINUSE, or starts with only the addresses
that are still available. A single-listener configuration is unaffected.

Pass the complete listening socket array and its count to
process_request(), and close every listener in the child before handling
the request. The accepted connection remains open, and the parent's
descriptors are unaffected. Make process_request() static as its only
caller is in the same file.

Fixes: 328f7f8a0a1c ("staging: usbip: userspace: eliminate glib dependency")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Rao <raoxu@uniontech.com>
---
 tools/usb/usbip/src/usbipd.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/usb/usbip/src/usbipd.c b/tools/usb/usbip/src/usbipd.c
index 3e22b651c754..5ff5b46674a2 100644
--- a/tools/usb/usbip/src/usbipd.c
+++ b/tools/usb/usbip/src/usbipd.c
@@ -339,17 +339,20 @@ static int do_accept(int listenfd)
 	return connfd;
 }

-int process_request(int listenfd)
+static int process_request(int listenfd, const int *listenfds, int nlistenfds)
 {
 	pid_t childpid;
 	int connfd;
+	int i;

 	connfd = do_accept(listenfd);
 	if (connfd < 0)
 		return -1;
 	childpid = fork();
 	if (childpid == 0) {
-		close(listenfd);
+		/* Do not keep unrelated listening sockets alive in workers. */
+		for (i = 0; i < nlistenfds; i++)
+			close(listenfds[i]);
 		recv_pdu(connfd);
 		exit(0);
 	}
@@ -568,7 +571,8 @@ static int do_standalone_mode(int daemonize, int ipv4, int ipv6)
 				if (fds[i].revents & POLLIN) {
 					dbg("read event on fd[%d]=%d",
 					    i, sockfdlist[i]);
-					process_request(sockfdlist[i]);
+					process_request(sockfdlist[i], sockfdlist,
+							nsockfd);
 				}
 			}
 		} else {
--
2.50.1

Re: [PATCH] usbip: Close all listening sockets in request workers

[Shuah Khan]

On 11/12/26 05:16, raoxu wrote:
> From: Xu Rao <raoxu@uniontech.com>
> 
> process_request() forks a worker after accepting a connection, but the
> child only closes the listener used for that connection. All other
> listening sockets are inherited and remain open until the worker exits.
> This occurs when usbipd listens on multiple addresses, including the
> default IPv4 and IPv6 setup.
> 
> A worker waiting for a client request can therefore keep an unrelated
> listener alive after the main daemon exits. Restarting usbipd then fails
> to bind that address with EADDRINUSE, or starts with only the addresses
> that are still available. A single-listener configuration is unaffected.
> 
> Pass the complete listening socket array and its count to
> process_request(), and close every listener in the child before handling
> the request. The accepted connection remains open, and the parent's
> descriptors are unaffected. Make process_request() static as its only
> caller is in the same file.

Looks reasonable to me. How did you find the problem? Can you share
how to reproduce this problem?

> 
> Fixes: 328f7f8a0a1c ("staging: usbip: userspace: eliminate glib dependency")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
> ---
>   tools/usb/usbip/src/usbipd.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/usb/usbip/src/usbipd.c b/tools/usb/usbip/src/usbipd.c
> index 3e22b651c754..5ff5b46674a2 100644
> --- a/tools/usb/usbip/src/usbipd.c
> +++ b/tools/usb/usbip/src/usbipd.c
> @@ -339,17 +339,20 @@ static int do_accept(int listenfd)
>   	return connfd;
>   }
> 
> -int process_request(int listenfd)
> +static int process_request(int listenfd, const int *listenfds, int nlistenfds)
>   {
>   	pid_t childpid;
>   	int connfd;
> +	int i;
> 
>   	connfd = do_accept(listenfd);
>   	if (connfd < 0)
>   		return -1;
>   	childpid = fork();
>   	if (childpid == 0) {
> -		close(listenfd);
> +		/* Do not keep unrelated listening sockets alive in workers. */
> +		for (i = 0; i < nlistenfds; i++)
> +			close(listenfds[i]);
>   		recv_pdu(connfd);
>   		exit(0);
>   	}
> @@ -568,7 +571,8 @@ static int do_standalone_mode(int daemonize, int ipv4, int ipv6)
>   				if (fds[i].revents & POLLIN) {
>   					dbg("read event on fd[%d]=%d",
>   					    i, sockfdlist[i]);
> -					process_request(sockfdlist[i]);
> +					process_request(sockfdlist[i], sockfdlist,
> +							nsockfd);
>   				}
>   			}
>   		} else {
> --
> 2.50.1
> 

thanks,
-- Shuah