From 2cd095e706a9689e4eb91cd8e59989a7a38d5036 Mon Sep 17 00:00:00 2001 From: wangzhiqiang Date: Tue, 19 Dec 2023 16:32:54 +0800 Subject: [PATCH] fix coredump in io_err_stat Signed-off-by: wangzhiqiang --- ...err_stat-don-t-free-aio-memory-befor.patch | 98 +++++++++++++++++++ ...err_stat-call-io_destroy-inside-free.patch | 70 +++++++++++++ multipath-tools.spec | 7 +- 3 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 0040-libmultipath-io_err_stat-don-t-free-aio-memory-befor.patch create mode 100644 0041-libmultipath-io_err_stat-call-io_destroy-inside-free.patch diff --git a/0040-libmultipath-io_err_stat-don-t-free-aio-memory-befor.patch b/0040-libmultipath-io_err_stat-don-t-free-aio-memory-befor.patch new file mode 100644 index 0000000..e150841 --- /dev/null +++ b/0040-libmultipath-io_err_stat-don-t-free-aio-memory-befor.patch @@ -0,0 +1,98 @@ +From eff53df4b1cdd74643f54568a327ae7b824337bb Mon Sep 17 00:00:00 2001 +From: Martin Wilck +Date: Wed, 27 Sep 2023 12:34:08 +0200 +Subject: [PATCH] libmultipath: io_err_stat: don't free aio memory before + completion + +It is wrong to assume that aio data structures can be reused or freed +after io_cancel(). io_cancel() will almost always return -EINPROGRESS, +anyway. Use the io_starttime field to indicate whether an io event +has been completed by the kernel. Make sure no in-flight buffers are freed. + +Fixes https://github.com/opensvc/multipath-tools/issues/73. + +Signed-off-by: Martin Wilck +Reviewed-by: Benjamin Marzinski +Cc: Li Xiao Keng +Cc: Miao Guanqin +Cc: Guan Junxiong +--- + libmultipath/io_err_stat.c | 26 ++++++++++++++++---------- + 1 file changed, 16 insertions(+), 10 deletions(-) + +diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c +index d8d91f6..480034b 100644 +--- a/libmultipath/io_err_stat.c ++++ b/libmultipath/io_err_stat.c +@@ -112,10 +112,14 @@ static int init_each_dio_ctx(struct dio_ctx *ct, int blksize, + return 0; + } + +-static void deinit_each_dio_ctx(struct dio_ctx *ct) ++static int deinit_each_dio_ctx(struct dio_ctx *ct) + { +- if (ct->buf) +- free(ct->buf); ++ if (!ct->buf) ++ return 0; ++ if (ct->io_starttime.tv_sec != 0 || ct->io_starttime.tv_nsec != 0) ++ return 1; ++ free(ct->buf); ++ return 0; + } + + static int setup_directio_ctx(struct io_err_stat_path *p) +@@ -164,6 +168,7 @@ fail_close: + static void free_io_err_stat_path(struct io_err_stat_path *p) + { + int i; ++ int inflight = 0; + + if (!p) + return; +@@ -173,8 +178,13 @@ static void free_io_err_stat_path(struct io_err_stat_path *p) + cancel_inflight_io(p); + + for (i = 0; i < CONCUR_NR_EVENT; i++) +- deinit_each_dio_ctx(p->dio_ctx_array + i); +- FREE(p->dio_ctx_array); ++ inflight += deinit_each_dio_ctx(p->dio_ctx_array + i); ++ ++ if (!inflight) ++ FREE(p->dio_ctx_array); ++ else ++ io_err_stat_log(2, "%s: can't free aio space of %s, %d IOs in flight", ++ __func__, p->devname, inflight); + + if (p->fd > 0) + close(p->fd); +@@ -503,7 +513,7 @@ static int try_to_cancel_timeout_io(struct dio_ctx *ct, struct timespec *t, + int rc = PATH_UNCHECKED; + int r; + +- if (ct->io_starttime.tv_sec == 0) ++ if (ct->io_starttime.tv_sec == 0 && ct->io_starttime.tv_nsec == 0) + return rc; + timespecsub(t, &ct->io_starttime, &difftime); + if (difftime.tv_sec > IOTIMEOUT_SEC) { +@@ -514,8 +524,6 @@ static int try_to_cancel_timeout_io(struct dio_ctx *ct, struct timespec *t, + if (r) + io_err_stat_log(5, "%s: io_cancel error %i", + dev, errno); +- ct->io_starttime.tv_sec = 0; +- ct->io_starttime.tv_nsec = 0; + rc = PATH_TIMEOUT; + } else { + rc = PATH_PENDING; +@@ -559,8 +567,6 @@ static void cancel_inflight_io(struct io_err_stat_path *pp) + if (r) + io_err_stat_log(5, "%s: io_cancel error %d, %i", + pp->devname, r, errno); +- ct->io_starttime.tv_sec = 0; +- ct->io_starttime.tv_nsec = 0; + } + } + +-- +2.33.0 + diff --git a/0041-libmultipath-io_err_stat-call-io_destroy-inside-free.patch b/0041-libmultipath-io_err_stat-call-io_destroy-inside-free.patch new file mode 100644 index 0000000..cbb80d2 --- /dev/null +++ b/0041-libmultipath-io_err_stat-call-io_destroy-inside-free.patch @@ -0,0 +1,70 @@ +From 26dab1986820b1d84c27ea02c5343edd88383842 Mon Sep 17 00:00:00 2001 +From: Martin Wilck +Date: Thu, 19 Oct 2023 18:53:33 +0200 +Subject: [PATCH] libmultipath: io_err_stat: call io_destroy() inside + free_io_err_pathvec() + +Memory used by aio iocbs must not be freed before either all iocbs +have finished, or io_destroy() has been called (which will wait until +all iocbs have finished). + +Don't call cancel_inflight_io() from free_io_err_stat_path(). Rather, +cancel directly from free_io_err_pathvec(). This way we make sure that +we can't call io_cancel() with an already destroyed ioctx, and that +we don't free memory of in-flight requests. + +The other callers of free_io_err_stat_path() also don't need to call +cancel_inflight_io(). In service_paths(), where free_io_err_stat_path() +it is called for paths on which all IO has either finished or timed out, +hanging requests will already have been cancelled in the +try_to_cancel_timeout_io() code path (note that total_time is at least +2 * IO_TIMEOUT_SEC). In the failure case of enqueue_io_err_stat_by_path(), no +IO has been submitted yet. + +Signed-off-by: Martin Wilck +Reviewed-by: Benjamin Marzinski +Cc: Li Xiao Keng +Cc: Miao Guanqin +Cc: Guan Junxiong +--- + libmultipath/io_err_stat.c | 12 +++++++++--- + 1 file changed, 9 insertions(+), 3 deletions(-) + +diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c +index c474c340..3f32e32e 100644 +--- a/libmultipath/io_err_stat.c ++++ b/libmultipath/io_err_stat.c +@@ -175,8 +175,6 @@ static void free_io_err_stat_path(struct io_err_stat_path *p) + if (!p->dio_ctx_array) + goto free_path; + +- cancel_inflight_io(p); +- + for (i = 0; i < CONCUR_NR_EVENT; i++) + inflight += deinit_each_dio_ctx(p->dio_ctx_array + i); + +@@ -221,6 +219,15 @@ static void free_io_err_pathvec(void) + pthread_cleanup_push(cleanup_mutex, &io_err_pathvec_lock); + if (!io_err_pathvec) + goto out; ++ ++ /* io_cancel() is a noop, but maybe in the future it won't be */ ++ vector_foreach_slot(io_err_pathvec, path, i) { ++ if (path && path->dio_ctx_array) ++ cancel_inflight_io(path); ++ } ++ ++ /* This blocks until all I/O is finished */ ++ io_destroy(ioctx); + vector_foreach_slot(io_err_pathvec, path, i) + free_io_err_stat_path(path); + vector_free(io_err_pathvec); +@@ -752,5 +759,4 @@ void stop_io_err_stat_thread(void) + + pthread_join(io_err_stat_thr, NULL); + free_io_err_pathvec(); +- io_destroy(ioctx); + } +-- +2.33.0 + diff --git a/multipath-tools.spec b/multipath-tools.spec index 8c45fb0..4f061b3 100644 --- a/multipath-tools.spec +++ b/multipath-tools.spec @@ -2,7 +2,7 @@ Name: multipath-tools Version: 0.8.4 -Release: 18 +Release: 19 Summary: Tools to manage multipath devices with the device-mapper License: GPL-2.0-or-later and LGPL-2.0-only URL: http://christophe.varoqui.free.fr/ @@ -49,6 +49,8 @@ Patch36: 0036-multipathd-make-all-cli_handlers-static.patch Patch37: 0037-multipathd-Fix-command-completion-in-interactive-mod.patch Patch38: 0038-multipathd-more-robust-command-parsing.patch Patch39: 0039-multipathd-Fixed-multipathd-parameter-invoking-seque.patch +Patch40: 0040-libmultipath-io_err_stat-don-t-free-aio-memory-befor.patch +Patch41: 0041-libmultipath-io_err_stat-call-io_destroy-inside-free.patch BuildRequires: multipath-tools, libcmocka, libcmocka-devel BuildRequires: gcc, libaio-devel, userspace-rcu-devel, device-mapper-devel >= 1.02.89 @@ -196,6 +198,9 @@ fi %changelog +* Tue Dec 19 2023 wangzhiqiang - 0.8.4-19 +- fix coredump in io_err_stat + * Mon Nov 28 2022 miaoguanqin - 0.8.4-18 - fix CVE-2022-41974 cause mpathpersist and multipathd execute error -- Gitee