rpms
/
device-mapper-multipath
7
0
Fork 0
Code Issues Pull Requests Releases Activity
device-mapper-multipath/0024-libmultipath-protect-r...

438 lines
12 KiB
Diff
Raw Blame History

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Martin Wilck <mwilck@suse.com>
Date: Tue, 15 Sep 2020 23:38:48 +0200
Subject: [PATCH] libmultipath: protect racy libdevmapper calls with a mutex
dm_udev_wait() and dm_task_run() may access global / static state
in libdevmapper. They need to be protected by a lock in in our
multithreaded library.
The modified call sequence requires fixing the dmevents test:
devmapper.c must be added to dmevents-test_OBJDEPS to catch calls
to dm_task_run(). Also, the call to dmevent_poll_supported() in
setup() will cause init_versions() to be called, which requires
bypassing the wrappers in the test setup phase.
Cc: lixiaokeng@huawei.com
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/devmapper.c | 73 +++++++++++++++++++------------
libmultipath/devmapper.h | 2 +
libmultipath/libmultipath.version | 6 +++
libmultipath/util.c | 5 +++
libmultipath/util.h | 1 +
multipathd/dmevents.c | 2 +-
multipathd/waiter.c | 2 +-
tests/Makefile | 1 +
tests/dmevents.c | 12 +++++
9 files changed, 75 insertions(+), 29 deletions(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 3e36129b..4eb6f539 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -42,6 +42,7 @@ static unsigned int dm_mpath_target_version[3] = { INVALID_VERSION, };
static pthread_once_t dm_initialized = PTHREAD_ONCE_INIT;
static pthread_once_t versions_initialized = PTHREAD_ONCE_INIT;
+static pthread_mutex_t libmp_dm_lock = PTHREAD_MUTEX_INITIALIZER;
static int dm_conf_verbosity;
@@ -59,16 +60,34 @@ static inline int dm_task_set_cookie(struct dm_task *dmt, uint32_t *c, int a)
return 1;
}
-void dm_udev_wait(unsigned int c)
+static void libmp_udev_wait(unsigned int c)
{
}
-void dm_udev_set_sync_support(int c)
+static void dm_udev_set_sync_support(int c)
{
}
-
+#else
+static void libmp_udev_wait(unsigned int c)
+{
+ pthread_mutex_lock(&libmp_dm_lock);
+ pthread_cleanup_push(cleanup_mutex, &libmp_dm_lock);
+ dm_udev_wait(c);
+ pthread_cleanup_pop(1);
+}
#endif
+int libmp_dm_task_run(struct dm_task *dmt)
+{
+ int r;
+
+ pthread_mutex_lock(&libmp_dm_lock);
+ pthread_cleanup_push(cleanup_mutex, &libmp_dm_lock);
+ r = dm_task_run(dmt);
+ pthread_cleanup_pop(1);
+ return r;
+}
+
__attribute__((format(printf, 4, 5))) static void
dm_write_log (int level, const char *file, int line, const char *f, ...)
{
@@ -198,7 +217,7 @@ static int dm_tgt_version (unsigned int *version, char *str)
dm_task_no_open_count(dmt);
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(2, DM_DEVICE_LIST_VERSIONS, dmt);
condlog(0, "Can not communicate with kernel DM");
goto out;
@@ -382,12 +401,12 @@ dm_simplecmd (int task, const char *name, int no_flush, int need_sync, uint16_t
DM_UDEV_DISABLE_LIBRARY_FALLBACK | udev_flags))
goto out;
- r = dm_task_run (dmt);
+ r = libmp_dm_task_run (dmt);
if (!r)
dm_log_error(2, task, dmt);
if (udev_wait_flag)
- dm_udev_wait(cookie);
+ libmp_udev_wait(cookie);
out:
dm_task_destroy (dmt);
return r;
@@ -474,12 +493,12 @@ dm_addmap (int task, const char *target, struct multipath *mpp,
!dm_task_set_cookie(dmt, &cookie, udev_flags))
goto freeout;
- r = dm_task_run (dmt);
+ r = libmp_dm_task_run (dmt);
if (!r)
dm_log_error(2, task, dmt);
if (task == DM_DEVICE_CREATE)
- dm_udev_wait(cookie);
+ libmp_udev_wait(cookie);
freeout:
if (prefixed_uuid)
FREE(prefixed_uuid);
@@ -589,7 +608,7 @@ do_get_info(const char *name, struct dm_info *info)
dm_task_no_open_count(dmt);
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_INFO, dmt);
goto out;
}
@@ -630,7 +649,7 @@ int dm_get_map(const char *name, unsigned long long *size, char *outparams)
dm_task_no_open_count(dmt);
errno = 0;
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_TABLE, dmt);
if (dm_task_get_errno(dmt) == ENXIO)
r = DMP_NOT_FOUND;
@@ -672,7 +691,7 @@ dm_get_prefixed_uuid(const char *name, char *uuid, int uuid_len)
if (!dm_task_set_name (dmt, name))
goto uuidout;
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_INFO, dmt);
goto uuidout;
}
@@ -743,7 +762,7 @@ int dm_get_status(const char *name, char *outstatus)
dm_task_no_open_count(dmt);
errno = 0;
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_STATUS, dmt);
if (dm_task_get_errno(dmt) == ENXIO)
r = DMP_NOT_FOUND;
@@ -796,7 +815,7 @@ int dm_type(const char *name, char *type)
dm_task_no_open_count(dmt);
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_TABLE, dmt);
goto out;
}
@@ -840,7 +859,7 @@ int dm_is_mpath(const char *name)
dm_task_no_open_count(dmt);
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_TABLE, dmt);
goto out_task;
}
@@ -904,7 +923,7 @@ dm_map_present_by_uuid(const char *uuid)
if (!dm_task_set_uuid(dmt, prefixed_uuid))
goto out_task;
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_INFO, dmt);
goto out_task;
}
@@ -950,7 +969,7 @@ dm_get_opencount (const char * mapname)
if (!dm_task_set_name(dmt, mapname))
goto out;
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_INFO, dmt);
goto out;
}
@@ -1110,7 +1129,7 @@ int dm_flush_maps (int need_suspend, int retries)
dm_task_no_open_count(dmt);
- if (!dm_task_run (dmt)) {
+ if (!libmp_dm_task_run (dmt)) {
dm_log_error(3, DM_DEVICE_LIST, dmt);
goto out;
}
@@ -1156,7 +1175,7 @@ dm_message(const char * mapname, char * message)
dm_task_no_open_count(dmt);
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(2, DM_DEVICE_TARGET_MSG, dmt);
goto out;
}
@@ -1276,7 +1295,7 @@ dm_get_maps (vector mp)
dm_task_no_open_count(dmt);
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_LIST, dmt);
goto out;
}
@@ -1361,7 +1380,7 @@ dm_mapname(int major, int minor)
* daemon uev_trigger -> uev_add_map
*/
while (--loop) {
- r = dm_task_run(dmt);
+ r = libmp_dm_task_run(dmt);
if (r)
break;
@@ -1406,7 +1425,7 @@ do_foreach_partmaps (const char * mapname,
dm_task_no_open_count(dmt);
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_LIST, dmt);
goto out;
}
@@ -1641,11 +1660,11 @@ dm_rename (const char * old, char * new, char *delim, int skip_kpartx)
if (!dm_task_set_cookie(dmt, &cookie, udev_flags))
goto out;
- r = dm_task_run(dmt);
+ r = libmp_dm_task_run(dmt);
if (!r)
dm_log_error(2, DM_DEVICE_RENAME, dmt);
- dm_udev_wait(cookie);
+ libmp_udev_wait(cookie);
out:
dm_task_destroy(dmt);
@@ -1687,7 +1706,7 @@ int dm_reassign_table(const char *name, char *old, char *new)
dm_task_no_open_count(dmt);
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_TABLE, dmt);
goto out;
}
@@ -1720,7 +1739,7 @@ int dm_reassign_table(const char *name, char *old, char *new)
if (modified) {
dm_task_no_open_count(reload_dmt);
- if (!dm_task_run(reload_dmt)) {
+ if (!libmp_dm_task_run(reload_dmt)) {
dm_log_error(3, DM_DEVICE_RELOAD, reload_dmt);
condlog(3, "%s: failed to reassign targets", name);
goto out_reload;
@@ -1767,7 +1786,7 @@ int dm_reassign(const char *mapname)
dm_task_no_open_count(dmt);
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_DEPS, dmt);
goto out;
}
@@ -1835,7 +1854,7 @@ int dm_setgeometry(struct multipath *mpp)
goto out;
}
- r = dm_task_run(dmt);
+ r = libmp_dm_task_run(dmt);
if (!r)
dm_log_error(3, DM_DEVICE_SET_GEOMETRY, dmt);
out:
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index a0bcd137..fa6b3c53 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -93,6 +93,8 @@ enum {
MULTIPATH_VERSION
};
int libmp_get_version(int which, unsigned int version[3]);
+struct dm_task;
+int libmp_dm_task_run(struct dm_task *dmt);
#define dm_log_error(lvl, cmd, dmt) \
condlog(lvl, "%s: libdm task=%d error: %s", __func__, \
diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
index ab5bb0ad..97acdbb2 100644
--- a/libmultipath/libmultipath.version
+++ b/libmultipath/libmultipath.version
@@ -245,3 +245,9 @@ global:
local:
*;
};
+
+LIBMULTIPATH_2.1.0 {
+global:
+ libmp_dm_task_run;
+ cleanup_mutex;
+} LIBMULTIPATH_2.0.0;
diff --git a/libmultipath/util.c b/libmultipath/util.c
index 1f977792..0e37f3ff 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -424,6 +424,11 @@ void cleanup_free_ptr(void *arg)
free(*p);
}
+void cleanup_mutex(void *arg)
+{
+ pthread_mutex_unlock(arg);
+}
+
struct bitfield *alloc_bitfield(unsigned int maxbit)
{
unsigned int n;
diff --git a/libmultipath/util.h b/libmultipath/util.h
index ac19473e..e9b48f9f 100644
--- a/libmultipath/util.h
+++ b/libmultipath/util.h
@@ -49,6 +49,7 @@ int should_exit(void);
void close_fd(void *arg);
void cleanup_free_ptr(void *arg);
+void cleanup_mutex(void *arg);
struct scandir_result {
struct dirent **di;
diff --git a/multipathd/dmevents.c b/multipathd/dmevents.c
index fc97c8a2..b561cbfd 100644
--- a/multipathd/dmevents.c
+++ b/multipathd/dmevents.c
@@ -156,7 +156,7 @@ static int dm_get_events(void)
dm_task_no_open_count(dmt);
- if (!dm_task_run(dmt)) {
+ if (!libmp_dm_task_run(dmt)) {
dm_log_error(3, DM_DEVICE_LIST, dmt);
goto fail;
}
diff --git a/multipathd/waiter.c b/multipathd/waiter.c
index 3bc69807..bbe6c2a1 100644
--- a/multipathd/waiter.c
+++ b/multipathd/waiter.c
@@ -118,7 +118,7 @@ static int waiteventloop (struct event_thread *waiter)
pthread_sigmask(SIG_UNBLOCK, &set, &oldset);
pthread_testcancel();
- r = dm_task_run(waiter->dmt);
+ r = libmp_dm_task_run(waiter->dmt);
if (!r)
dm_log_error(2, DM_DEVICE_WAITEVENT, waiter->dmt);
pthread_testcancel();
diff --git a/tests/Makefile b/tests/Makefile
index 78777bec..908407ea 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -40,6 +40,7 @@ endif
# linker input file).
# XYZ-test_LIBDEPS: Additional libs to link for this test
+dmevents-test_OBJDEPS = ../libmultipath/devmapper.o
dmevents-test_LIBDEPS = -lpthread -ldevmapper -lurcu
hwtable-test_TESTDEPS := test-lib.o
hwtable-test_OBJDEPS := ../libmultipath/discovery.o ../libmultipath/blacklist.o \
diff --git a/tests/dmevents.c b/tests/dmevents.c
index bee117ac..b7c5122b 100644
--- a/tests/dmevents.c
+++ b/tests/dmevents.c
@@ -179,6 +179,8 @@ struct dm_names *build_dm_names(void)
return names;
}
+static bool setup_done;
+
static int setup(void **state)
{
if (dmevent_poll_supported()) {
@@ -186,6 +188,7 @@ static int setup(void **state)
*state = &data;
} else
*state = NULL;
+ setup_done = true;
return 0;
}
@@ -262,14 +265,20 @@ struct dm_task *__wrap_libmp_dm_task_create(int task)
return mock_type(struct dm_task *);
}
+int __real_dm_task_no_open_count(struct dm_task *dmt);
int __wrap_dm_task_no_open_count(struct dm_task *dmt)
{
+ if (!setup_done)
+ return __real_dm_task_no_open_count(dmt);
assert_ptr_equal((struct test_data *)dmt, &data);
return mock_type(int);
}
+int __real_dm_task_run(struct dm_task *dmt);
int __wrap_dm_task_run(struct dm_task *dmt)
{
+ if (!setup_done)
+ return __real_dm_task_run(dmt);
assert_ptr_equal((struct test_data *)dmt, &data);
return mock_type(int);
}
@@ -291,8 +300,11 @@ struct dm_names * __wrap_dm_task_get_names(struct dm_task *dmt)
return data.names;
}
+void __real_dm_task_destroy(struct dm_task *dmt);
void __wrap_dm_task_destroy(struct dm_task *dmt)
{
+ if (!setup_done)
+ return __real_dm_task_destroy(dmt);
assert_ptr_equal((struct test_data *)dmt, &data);
if (data.names) {
--
2.17.2
Reference in New Issue View Git Blame Copy Permalink