On Thu, Jan 28, 2021 at 09:45:44PM +0100, mwilck@xxxxxxxx wrote:
> From: Martin Wilck <mwilck@xxxxxxxx>
>
> glibc's implementation of pthread_cancel() loads symbols from
> libgcc_s.so using dlopen() when pthread_cancel() is called
> for the first time. This happens even with LD_BIND_NOW=1.
> This may imply the need for file system access when a thread is
> cancelled, which in the case of multipath-tools might be in a
> dangerous situation where multipathd must avoid blocking.
>
> Call load_unwinder() during startup to make sure the dynamic
> linker has all necessary symbols resolved early on.
>
> This implementation simply creates a dummy thread and cancels
> it. This way all necessary symbols for thread cancellation
> will be loaded, no matter what the C library needs to implement
> cancellation.
>
Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
> ---
> multipathd/Makefile | 2 +-
> multipathd/init_unwinder.c | 34 ++++++++++++++++++++++++++++++++++
> multipathd/init_unwinder.h | 21 +++++++++++++++++++++
> multipathd/main.c | 2 ++
> 4 files changed, 58 insertions(+), 1 deletion(-)
> create mode 100644 multipathd/init_unwinder.c
> create mode 100644 multipathd/init_unwinder.h
>
> diff --git a/multipathd/Makefile b/multipathd/Makefile
> index 632b82b..d053c1e 100644
> --- a/multipathd/Makefile
> +++ b/multipathd/Makefile
> @@ -30,7 +30,7 @@ ifeq ($(ENABLE_DMEVENTS_POLL),0)
> endif
>
> OBJS = main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o waiter.o \
> - dmevents.o
> + dmevents.o init_unwinder.o
>
> EXEC = multipathd
>
> diff --git a/multipathd/init_unwinder.c b/multipathd/init_unwinder.c
> new file mode 100644
> index 0000000..14467f3
> --- /dev/null
> +++ b/multipathd/init_unwinder.c
> @@ -0,0 +1,34 @@
> +#include <pthread.h>
> +#include <unistd.h>
> +#include "init_unwinder.h"
> +
> +static pthread_mutex_t dummy_mtx = PTHREAD_MUTEX_INITIALIZER;
> +static pthread_cond_t dummy_cond = PTHREAD_COND_INITIALIZER;
> +
> +static void *dummy_thread(void *arg __attribute__((unused)))
> +{
> + pthread_mutex_lock(&dummy_mtx);
> + pthread_cond_broadcast(&dummy_cond);
> + pthread_mutex_unlock(&dummy_mtx);
> + pause();
> + return NULL;
> +}
> +
> +int init_unwinder(void)
> +{
> + pthread_t dummy;
> + int rc;
> +
> + pthread_mutex_lock(&dummy_mtx);
> +
> + rc = pthread_create(&dummy, NULL, dummy_thread, NULL);
> + if (rc != 0) {
> + pthread_mutex_unlock(&dummy_mtx);
> + return rc;
> + }
> +
> + pthread_cond_wait(&dummy_cond, &dummy_mtx);
> + pthread_mutex_unlock(&dummy_mtx);
> +
> + return pthread_cancel(dummy);
> +}
> diff --git a/multipathd/init_unwinder.h b/multipathd/init_unwinder.h
> new file mode 100644
> index 0000000..ada09f8
> --- /dev/null
> +++ b/multipathd/init_unwinder.h
> @@ -0,0 +1,21 @@
> +#ifndef _INIT_UNWINDER_H
> +#define _INIT_UNWINDER_H 1
> +
> +/*
> + * init_unwinder(): make sure unwinder symbols are loaded
> + *
> + * libc's implementation of pthread_cancel() loads symbols from
> + * libgcc_s.so using dlopen() when pthread_cancel() is called
> + * for the first time. This happens even with LD_BIND_NOW=1.
> + * This may imply the need for file system access when a thread is
> + * cancelled, which in the case of multipath-tools might be in a
> + * dangerous situation where multipathd must avoid blocking.
> + *
> + * Call load_unwinder() during startup to make sure the dynamic
> + * linker has all necessary symbols resolved early on.
> + *
> + * Return: 0 if successful, an error number otherwise.
> + */
> +int init_unwinder(void);
> +
> +#endif
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 99a89a6..6f851ae 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -83,6 +83,7 @@
> #include "wwids.h"
> #include "foreign.h"
> #include "../third-party/valgrind/drd.h"
> +#include "init_unwinder.h"
>
> #define FILE_NAME_SIZE 256
> #define CMDSIZE 160
> @@ -3041,6 +3042,7 @@ child (__attribute__((unused)) void *param)
> enum daemon_status state;
> int exit_code = 1;
>
> + init_unwinder();
> mlockall(MCL_CURRENT | MCL_FUTURE);
> signal_init();
> mp_rcu_data = setup_rcu();
> --
> 2.29.2
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
