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

[Shuah Khan <skhan@linuxfoundation.org>]

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