rpms/device-mapper-multipath
7
0
Fork 0
Code Issues Pull Requests Releases Activity
0a156fc43d
device-mapper-multipath/0024-libmultipath-protect-racy-libdevmapper-calls-with-a-.patch
DistroBaker 28d3ae407e Merged update from upstream sources 
This is an automated DistroBaker update from upstream sources.
If you do not know what this is about or would like to opt out,
contact the OSCI team.

Source: https://src.fedoraproject.org/rpms/device-mapper-multipath.git#26a2cd7a3e189bf91263d17bc8a8c449cc043fb0
2021-01-21 16:56:27 +00:00

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