From: jeffm@suse.com
To: linux-xfs@vger.kernel.org
Cc: Jeff Mahoney <jeffm@suse.com>
Subject: [PATCH 3/3] xfs_repair: fix thread creation failure recovery
Date: Mon, 16 Jan 2017 23:39:33 -0500 [thread overview]
Message-ID: <1484627973-11535-4-git-send-email-jeffm@suse.com> (raw)
In-Reply-To: <1484627973-11535-1-git-send-email-jeffm@suse.com>
From: Jeff Mahoney <jeffm@suse.com>
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 <jeffm@suse.com>
---
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
prev parent reply other threads:[~2017-01-17 4:39 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-17 4:39 [PATCH 0/3] xfs_repair: pthread creation failure recovery fixes jeffm
2017-01-17 4:39 ` [PATCH 1/3] xfs_repair: clear pthread_t when pthread_create fails jeffm
2017-06-15 19:11 ` Eric Sandeen
2017-01-17 4:39 ` [PATCH 2/3] xfs_repair: add prefetch trace calls to debug thread creation failures jeffm
2017-06-15 19:11 ` Eric Sandeen
2017-01-17 4:39 ` jeffm [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1484627973-11535-4-git-send-email-jeffm@suse.com \
--to=jeffm@suse.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).