From c2598e4729bb3b1e35e16549a1441fab0786c66b8bc7e4ba235f438dc54983bc Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Thu, 22 Jun 2017 20:20:58 +0000 Subject: [PATCH] Accepting request 505781 from home:jeff_mahoney:branches:filesystems - Fix crash in xfs_repair when threads fail to start (bsc#1019938). * xfs_repair-clear-pthread_t-when-pthread_create-fails.patch * Added xfs_repair-add-prefetch-trace-calls-to-debug-thread-creation-failures.patch * xfs_repair-fix-thread-creation-failure-recovery.patch - fsr: fix uninitialized fs usage after timeout (bsc#1002699). * Added fsr-fix-uninitialized-fs-usage-after-timeout.patch OBS-URL: https://build.opensuse.org/request/show/505781 OBS-URL: https://build.opensuse.org/package/show/filesystems/xfsprogs?expand=0&rev=49 --- ...uninitialized-fs-usage-after-timeout.patch | 39 +++++ ...ls-to-debug-thread-creation-failures.patch | 60 ++++++++ ...-pthread_t-when-pthread_create-fails.patch | 55 +++++++ ...fix-thread-creation-failure-recovery.patch | 136 ++++++++++++++++++ xfsprogs.changes | 14 ++ xfsprogs.spec | 9 ++ 6 files changed, 313 insertions(+) create mode 100644 fsr-fix-uninitialized-fs-usage-after-timeout.patch create mode 100644 xfs_repair-add-prefetch-trace-calls-to-debug-thread-creation-failures.patch create mode 100644 xfs_repair-clear-pthread_t-when-pthread_create-fails.patch create mode 100644 xfs_repair-fix-thread-creation-failure-recovery.patch diff --git a/fsr-fix-uninitialized-fs-usage-after-timeout.patch b/fsr-fix-uninitialized-fs-usage-after-timeout.patch new file mode 100644 index 0000000..fc9172f --- /dev/null +++ b/fsr-fix-uninitialized-fs-usage-after-timeout.patch @@ -0,0 +1,39 @@ +From: Jeff Mahoney +Date: Fri, 2 Jun 2017 14:15:41 -0400 +Subject: fsr: fix uninitialized fs usage after timeout +Patch-mainline: Submitted to linux-xfs, 2 Jun 2017 +References: bsc#1002699 + +In the main loop of fsrallfs, we exit when we've hit the timeout but +we increment fs before we get there. If we're operating on the last +file system in the array, we'll hit an uninitialized fsdesc and +crash in fsrall_cleanup. + +Signed-off-by: Jeff Mahoney +--- + fsr/xfs_fsr.c | 4 +--- + 1 file changed, 1 insertion(+), 3 deletions(-) + +diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c +index 517b75f0..e695c243 100644 +--- a/fsr/xfs_fsr.c ++++ b/fsr/xfs_fsr.c +@@ -598,7 +598,7 @@ fsrallfs(char *mtab, int howlong, char *leftofffile) + signal(SIGTERM, aborter); + + /* reorg for 'howlong' -- checked in 'fsrfs' */ +- while (endtime > time(0)) { ++ for (; endtime > time(0); fs->npass++, fs++) { + pid_t pid; + if (fs == fsend) + fs = fsbase; +@@ -629,8 +629,6 @@ fsrallfs(char *mtab, int howlong, char *leftofffile) + break; + } + startino = 0; /* reset after the first time through */ +- fs->npass++; +- fs++; + } + fsrall_cleanup(endtime <= time(0)); + } + diff --git a/xfs_repair-add-prefetch-trace-calls-to-debug-thread-creation-failures.patch b/xfs_repair-add-prefetch-trace-calls-to-debug-thread-creation-failures.patch new file mode 100644 index 0000000..108276e --- /dev/null +++ b/xfs_repair-add-prefetch-trace-calls-to-debug-thread-creation-failures.patch @@ -0,0 +1,60 @@ +From: Jeff Mahoney +Subject: xfs_repair: add prefetch trace calls to debug thread creation failures +Patch-mainline: Submitted, 16 Jan 2017 +References: bsc#1019938 + +When debugging prefetch failures, it's useful to have thread creation +failure messages that are output as warnings on stderr in the trace +log as well. It's also helpful to see when an AG gets queued behind +another one rather than having the thread started directly, which +has a separate trace line. + +Signed-off-by: Jeff Mahoney +--- + repair/prefetch.c | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +diff --git a/repair/prefetch.c b/repair/prefetch.c +index 044fab2..37d60d6 100644 +--- a/repair/prefetch.c ++++ b/repair/prefetch.c +@@ -703,6 +703,8 @@ pf_queuing_worker( + if (err != 0) { + do_warn(_("failed to create prefetch thread: %s\n"), + strerror(err)); ++ pftrace("failed to create prefetch thread for AG %d: %s", ++ args->agno, strerror(err)); + args->io_threads[i] = 0; + if (i == 0) { + pf_start_processing(args); +@@ -817,6 +819,8 @@ pf_create_prefetch_thread( + if (err != 0) { + do_warn(_("failed to create prefetch thread: %s\n"), + strerror(err)); ++ pftrace("failed to create prefetch thread for AG %d: %s", ++ args->agno, strerror(err)); + args->queuing_thread = 0; + cleanup_inode_prefetch(args); + } +@@ -882,8 +886,11 @@ start_inode_prefetch( + if (prev_args->prefetch_done) { + if (!pf_create_prefetch_thread(args)) + args = NULL; +- } else ++ } else { + prev_args->next_args = args; ++ pftrace("queued AG %d after AG %d", ++ args->agno, prev_args->agno); ++ } + pthread_mutex_unlock(&prev_args->lock); + } + +-- +2.7.1 + +-- +To unsubscribe from this list: send the line "unsubscribe linux-xfs" in +the body of a message to majordomo@vger.kernel.org +More majordomo info at http://vger.kernel.org/majordomo-info.html + + diff --git a/xfs_repair-clear-pthread_t-when-pthread_create-fails.patch b/xfs_repair-clear-pthread_t-when-pthread_create-fails.patch new file mode 100644 index 0000000..c77d3bc --- /dev/null +++ b/xfs_repair-clear-pthread_t-when-pthread_create-fails.patch @@ -0,0 +1,55 @@ +From: Jeff Mahoney +Subject: xfs_repair: clear pthread_t when pthread_create fails +Patch-mainline: Submitted, 16 Jan 2017 +References: bsc#1019938 + +pf_queuing_worker and pf_create_prefetch_thread both try to handle +thread creation failure gracefully, but assume that pthread_create +doesn't modify the pthread_t when it fails. + +>From the pthread_create man page: +On success, pthread_create() returns 0; on error, it returns an error +number, and the contents of *thread are undefined. + +In fact, glibc's pthread_create writes the pthread_t value before +calling clone(). When we join the created threads in +cleanup_inode_prefetch and the cleanup stage of pf_queuing_worker, we +assume that if the pthread_t is nonzero that it's a valid thread handle +and end up crashing in pthread_join. + +This patch zeros out the handle after pthread_create failure. + +Signed-off-by: Jeff Mahoney +--- + repair/prefetch.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/repair/prefetch.c b/repair/prefetch.c +index ff50606..044fab2 100644 +--- a/repair/prefetch.c ++++ b/repair/prefetch.c +@@ -703,6 +703,7 @@ pf_queuing_worker( + if (err != 0) { + do_warn(_("failed to create prefetch thread: %s\n"), + strerror(err)); ++ args->io_threads[i] = 0; + if (i == 0) { + pf_start_processing(args); + return NULL; +@@ -816,6 +817,7 @@ pf_create_prefetch_thread( + if (err != 0) { + do_warn(_("failed to create prefetch thread: %s\n"), + strerror(err)); ++ args->queuing_thread = 0; + cleanup_inode_prefetch(args); + } + +-- +2.7.1 + +-- +To unsubscribe from this list: send the line "unsubscribe linux-xfs" in +the body of a message to majordomo@vger.kernel.org +More majordomo info at http://vger.kernel.org/majordomo-info.html + + diff --git a/xfs_repair-fix-thread-creation-failure-recovery.patch b/xfs_repair-fix-thread-creation-failure-recovery.patch new file mode 100644 index 0000000..84cbaf0 --- /dev/null +++ b/xfs_repair-fix-thread-creation-failure-recovery.patch @@ -0,0 +1,136 @@ +From: Jeff Mahoney +Subject: xfs_repair: fix thread creation failure recovery +Patch-mainline: Submitted, 16 Jan 2017 +References: bsc#1019938 + +When pf_create_prefetch_thread fails, it tears down the args struct +and frees it. This causes a use-after-free in prefetch_ag_range, which +then passes the now-invalid pointer to start_inode_prefetch. The struct +is only freed when the queuing thread can't be started. When we can't +start even one worker thread, we mark the args ready for processing and +allow it to proceed single-threaded. Unfortunately, this only marks +the current args ready for processing and since we return immediately, +the call to pf_create_prefetch_thread at the end of pf_queuing_worker +never gets called and we wait forever for prefetch to start on the +next AG. + +This patch factors out the cleanup into a new pf_skip_prefetch_thread +that is called when we fail to create either the queuing thread or +the first of the workers. It marks the args ready for processing, marks +it done so start_inode_prefetch doesn't add another AG to the list, and +tries to start a new thread for the next AG in the list. We also clear +->next_args and check for it in cleanup_inode_prefetch so this condition +is easier to catch should it arise again. + +Signed-off-by: Jeff Mahoney +--- + repair/prefetch.c | 38 ++++++++++++++++++++++++++++++++------ + 1 file changed, 32 insertions(+), 6 deletions(-) + +diff --git a/repair/prefetch.c b/repair/prefetch.c +index 37d60d6..4c74b6e 100644 +--- a/repair/prefetch.c ++++ b/repair/prefetch.c +@@ -679,11 +679,32 @@ static int + pf_create_prefetch_thread( + prefetch_args_t *args); + ++/* ++ * If we fail to create the queuing thread or can't create even one ++ * prefetch thread, we need to let processing continue without it. ++ */ ++static void ++pf_skip_prefetch_thread(prefetch_args_t *args) ++{ ++ prefetch_args_t *next; ++ ++ pthread_mutex_lock(&args->lock); ++ args->prefetch_done = 1; ++ pf_start_processing(args); ++ next = args->next_args; ++ args->next_args = NULL; ++ pthread_mutex_unlock(&args->lock); ++ ++ if (next) ++ pf_create_prefetch_thread(next); ++} ++ + static void * + pf_queuing_worker( + void *param) + { + prefetch_args_t *args = param; ++ prefetch_args_t *next_args; + int num_inos; + ino_tree_node_t *irec; + ino_tree_node_t *cur_irec; +@@ -707,7 +728,7 @@ pf_queuing_worker( + args->agno, strerror(err)); + args->io_threads[i] = 0; + if (i == 0) { +- pf_start_processing(args); ++ pf_skip_prefetch_thread(args); + return NULL; + } + /* +@@ -798,11 +819,13 @@ pf_queuing_worker( + ASSERT(btree_is_empty(args->io_queue)); + + args->prefetch_done = 1; +- if (args->next_args) +- pf_create_prefetch_thread(args->next_args); +- ++ next_args = args->next_args; ++ args->next_args = NULL; + pthread_mutex_unlock(&args->lock); + ++ if (next_args) ++ pf_create_prefetch_thread(next_args); ++ + return NULL; + } + +@@ -822,7 +845,7 @@ pf_create_prefetch_thread( + pftrace("failed to create prefetch thread for AG %d: %s", + args->agno, strerror(err)); + args->queuing_thread = 0; +- cleanup_inode_prefetch(args); ++ pf_skip_prefetch_thread(args); + } + + return err == 0; +@@ -884,14 +907,15 @@ start_inode_prefetch( + } else { + pthread_mutex_lock(&prev_args->lock); + if (prev_args->prefetch_done) { ++ pthread_mutex_unlock(&prev_args->lock); + if (!pf_create_prefetch_thread(args)) + args = NULL; + } else { + prev_args->next_args = args; + pftrace("queued AG %d after AG %d", + args->agno, prev_args->agno); ++ pthread_mutex_unlock(&prev_args->lock); + } +- pthread_mutex_unlock(&prev_args->lock); + } + + return args; +@@ -1066,6 +1090,8 @@ cleanup_inode_prefetch( + + pftrace("AG %d prefetch done", args->agno); + ++ ASSERT(args->next_args == NULL); ++ + pthread_mutex_destroy(&args->lock); + pthread_cond_destroy(&args->start_reading); + pthread_cond_destroy(&args->start_processing); +-- +2.7.1 + +-- +To unsubscribe from this list: send the line "unsubscribe linux-xfs" in +the body of a message to majordomo@vger.kernel.org +More majordomo info at http://vger.kernel.org/majordomo-info.html + + diff --git a/xfsprogs.changes b/xfsprogs.changes index 5fe7d20..e4162b5 100644 --- a/xfsprogs.changes +++ b/xfsprogs.changes @@ -1,3 +1,17 @@ +------------------------------------------------------------------- +Thu Jun 22 20:15:28 UTC 2017 - jeffm@suse.com + +- Fix crash in xfs_repair when threads fail to start (bsc#1019938). + * xfs_repair-clear-pthread_t-when-pthread_create-fails.patch + * Added xfs_repair-add-prefetch-trace-calls-to-debug-thread-creation-failures.patch + * xfs_repair-fix-thread-creation-failure-recovery.patch + +------------------------------------------------------------------- +Fri Jun 2 18:23:03 UTC 2017 - jeffm@suse.com + +- fsr: fix uninitialized fs usage after timeout (bsc#1002699). + * Added fsr-fix-uninitialized-fs-usage-after-timeout.patch + ------------------------------------------------------------------- Tue May 16 14:52:53 UTC 2017 - jeffm@suse.com diff --git a/xfsprogs.spec b/xfsprogs.spec index 44c1400..29e699e 100644 --- a/xfsprogs.spec +++ b/xfsprogs.spec @@ -31,6 +31,10 @@ Source2: %{name}.keyring Source3: module-setup.sh.in Source4: dracut-fsck-help.txt Patch0: xfsprogs-docdir.diff +Patch1: fsr-fix-uninitialized-fs-usage-after-timeout.patch +Patch2: xfs_repair-clear-pthread_t-when-pthread_create-fails.patch +Patch3: xfs_repair-add-prefetch-trace-calls-to-debug-thread-creation-failures.patch +Patch4: xfs_repair-fix-thread-creation-failure-recovery.patch BuildRequires: libblkid-devel BuildRequires: readline-devel BuildRoot: %{_tmppath}/%{name}-%{version}-build @@ -42,6 +46,7 @@ Supplements: filesystem(xfs) %if 0%{?suse_version} >= 1310 BuildRequires: suse-module-tools %endif +BuildRequires: xz %description A set of commands to use the XFS file system, including mkfs.xfs. @@ -75,6 +80,10 @@ want to install xfsprogs. %if 0%{?suse_version} %patch0 %endif +%patch1 -p1 +%patch2 -p1 +%patch3 -p1 +%patch4 -p1 %build export OPTIMIZER="-fPIC"