[dm-devel] [PATCH v2 23/23] multipathd: fix signal blocking logic

[Benjamin Marzinski]

On Tue, Mar 06, 2018 at 12:15:07AM +0100, Martin Wilck wrote:
> multipathd is supposed to block all signals in all threads, except
> the uxlsnr thread which handles termination and reconfiguration
> signals (SIGUSR1) in its ppoll() call, SIGUSR2 in the waiter thread
> and the marginal path checker thread, and occasional SIGALRM. The current
> logic does exactly the oppsite, it blocks termination signals in SIGPOLL and
> allows multipathd to be killed e.g. by SIGALRM.
> 
> Fix that by inverting the logic. The argument to pthread_sigmask and
> ppoll is the set of *blocked* signals, not vice versa.
> 
> The marginal paths code needs to unblock SIGUSR2 now explicity, as
> the dm-event waiter code already does. Doing this with pselect()
> avoids asynchronous cancellation.
> 
> Fixes: 810082e "libmultipath, multipathd: Rework SIGPIPE handling"
> Fixes: 534ec4c "multipathd: Ensure that SIGINT, SIGTERM, SIGHUP and SIGUSR1
> are delivered to the uxsock thread"
> 

Reviewed-by: Benjamin Marzinski <bmarzins at redhat.com>

> Signed-off-by: Martin Wilck <mwilck at suse.com>
> ---
>  libmultipath/io_err_stat.c | 17 ++++++++++++++++-
>  multipathd/main.c          |  7 +++++--
>  multipathd/uxlsnr.c        | 10 +++++-----
>  3 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
> index 5b10f0372f9f..00bac9e0e755 100644
> --- a/libmultipath/io_err_stat.c
> +++ b/libmultipath/io_err_stat.c
> @@ -21,6 +21,7 @@
>  #include <libaio.h>
>  #include <errno.h>
>  #include <sys/mman.h>
> +#include <sys/select.h>
>  
>  #include "vector.h"
>  #include "memory.h"
> @@ -691,14 +692,28 @@ static void service_paths(void)
>  
>  static void *io_err_stat_loop(void *data)
>  {
> +	sigset_t set;
> +
>  	vecs = (struct vectors *)data;
>  	pthread_cleanup_push(rcu_unregister, NULL);
>  	rcu_register_thread();
>  
> +	sigfillset(&set);
> +	sigdelset(&set, SIGUSR2);
> +
>  	mlockall(MCL_CURRENT | MCL_FUTURE);
>  	while (1) {
> +		struct timespec ts;
> +
>  		service_paths();
> -		usleep(100000);
> +
> +		ts.tv_sec = 0;
> +		ts.tv_nsec = 100 * 1000 * 1000;
> +		/*
> +		 * pselect() with no fds, a timeout, and a sigmask:
> +		 * sleep for 100ms and react on SIGUSR2.
> +		 */
> +		pselect(1, NULL, NULL, NULL, &ts, &set);
>  	}
>  
>  	pthread_cleanup_pop(1);
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 4abdd8f071c3..68595836d723 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2261,10 +2261,13 @@ signal_init(void)
>  {
>  	sigset_t set;
>  
> -	sigemptyset(&set);
> -	sigaddset(&set, SIGUSR2);
> +	/* block all signals */
> +	sigfillset(&set);
> +	/* SIGPIPE occurs if logging fails */
> +	sigdelset(&set, SIGPIPE);
>  	pthread_sigmask(SIG_SETMASK, &set, NULL);
>  
> +	/* Other signals will be unblocked in the uxlsnr thread */
>  	signal_set(SIGHUP, sighup);
>  	signal_set(SIGUSR1, sigusr1);
>  	signal_set(SIGUSR2, sigusr2);
> diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
> index 98ac25a68c43..a2ca36ba1653 100644
> --- a/multipathd/uxlsnr.c
> +++ b/multipathd/uxlsnr.c
> @@ -170,11 +170,11 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
>  		condlog(0, "uxsock: failed to allocate poll fds");
>  		return NULL;
>  	}
> -	sigemptyset(&mask);
> -	sigaddset(&mask, SIGINT);
> -	sigaddset(&mask, SIGTERM);
> -	sigaddset(&mask, SIGHUP);
> -	sigaddset(&mask, SIGUSR1);
> +	sigfillset(&mask);
> +	sigdelset(&mask, SIGINT);
> +	sigdelset(&mask, SIGTERM);
> +	sigdelset(&mask, SIGHUP);
> +	sigdelset(&mask, SIGUSR1);
>  	while (1) {
>  		struct client *c, *tmp;
>  		int i, poll_count, num_clients;
> -- 
> 2.16.1