Re: [dm-devel] [PATCH 16/35] multipathd: uxlsnr: handle client HUP

[Benjamin Marzinski <bmarzins@redhat.com>]

On Fri, Sep 10, 2021 at 01:41:01PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> The unix socket listener thread doesn't even look at the revents
> returned by poll() while the daemon is configuring. This may cause a
> closed client socket to be kept open for a long time by the server,
> while the listener basically performs a busy loop, as ppoll() always
> returns immediately as long as the POLLHUP condition exists.
> 
> Worse, it can happen that multipathd reads data from such a closed
> client socket after the client has disconnected. See the description
> of POLLHUP in poll(2).
> 
> Close connections immediately if HUP is received.
> 
> Also, use the fd in log messages to identify the client rather
> than the random index.
> 
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  multipathd/uxlsnr.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
> index 20efbd3..7e88538 100644
> --- a/multipathd/uxlsnr.c
> +++ b/multipathd/uxlsnr.c
> @@ -393,7 +393,7 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, long ux_sock,
>  
>  		/* see if a client wants to speak to us */
>  		for (i = POLLFDS_BASE; i < n_pfds; i++) {
> -			if (polls[i].revents & POLLIN) {
> +			if (polls[i].revents & (POLLIN|POLLHUP|POLLERR)) {
>  				struct timespec start_time;
>  
>  				c = NULL;
> @@ -410,6 +410,12 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, long ux_sock,
>  						i, polls[i].fd);
>  					continue;
>  				}
> +				if (polls[i].revents & (POLLHUP|POLLERR)) {
> +					condlog(4, "cli[%d]: Disconnected",
> +						c->fd);
> +					dead_client(c);
> +					continue;
> +				}
>  				get_monotonic_time(&start_time);
>  				if (recv_packet_from_client(c->fd, &inbuf,
>  							    uxsock_timeout)
> @@ -423,7 +429,7 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, long ux_sock,
>  					continue;
>  				}
>  				condlog(4, "cli[%d]: Got request [%s]",
> -					i, inbuf);
> +					polls[i].fd, inbuf);
>  				uxsock_trigger(inbuf, &reply, &rlen,
>  					       _socket_client_is_root(c->fd),
>  					       trigger_data);
> @@ -434,7 +440,7 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, long ux_sock,
>  					} else {
>  						condlog(4, "cli[%d]: "
>  							"Reply [%d bytes]",
> -							i, rlen);
> +							polls[i].fd, rlen);
>  					}
>  					FREE(reply);
>  					reply = NULL;
> -- 
> 2.33.0

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel