Commit 3b918507 authored by Joe Thornber's avatar Joe Thornber Committed by Ben Hutchings

dm io: fix a race condition in the wake up code for sync_io

commit 10f1d5d1 upstream.

There's a race condition between the atomic_dec_and_test(&io->count)
in dec_count() and the waking of the sync_io() thread.  If the thread
is spuriously woken immediately after the decrement it may exit,
making the on stack io struct invalid, yet the dec_count could still
be using it.

Fix this race by using a completion in sync_io() and dec_count().
Reported-by: default avatarMinfei Huang <huangminfei@ucloud.cn>
Signed-off-by: default avatarJoe Thornber <thornber@redhat.com>
Signed-off-by: default avatarMike Snitzer <snitzer@redhat.com>
Acked-by: default avatarMikulas Patocka <mpatocka@redhat.com>
[bwh: Backported to 3.2: use wait_for_completion() as wait_for_completion_io()
 is not available]
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
parent 6df6ecaf
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include <linux/device-mapper.h> #include <linux/device-mapper.h>
#include <linux/bio.h> #include <linux/bio.h>
#include <linux/completion.h>
#include <linux/mempool.h> #include <linux/mempool.h>
#include <linux/module.h> #include <linux/module.h>
#include <linux/sched.h> #include <linux/sched.h>
...@@ -34,7 +35,7 @@ struct dm_io_client { ...@@ -34,7 +35,7 @@ struct dm_io_client {
struct io { struct io {
unsigned long error_bits; unsigned long error_bits;
atomic_t count; atomic_t count;
struct task_struct *sleeper; struct completion *wait;
struct dm_io_client *client; struct dm_io_client *client;
io_notify_fn callback; io_notify_fn callback;
void *context; void *context;
...@@ -122,8 +123,8 @@ static void dec_count(struct io *io, unsigned int region, int error) ...@@ -122,8 +123,8 @@ static void dec_count(struct io *io, unsigned int region, int error)
invalidate_kernel_vmap_range(io->vma_invalidate_address, invalidate_kernel_vmap_range(io->vma_invalidate_address,
io->vma_invalidate_size); io->vma_invalidate_size);
if (io->sleeper) if (io->wait)
wake_up_process(io->sleeper); complete(io->wait);
else { else {
unsigned long r = io->error_bits; unsigned long r = io->error_bits;
...@@ -384,6 +385,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions, ...@@ -384,6 +385,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions,
*/ */
volatile char io_[sizeof(struct io) + __alignof__(struct io) - 1]; volatile char io_[sizeof(struct io) + __alignof__(struct io) - 1];
struct io *io = (struct io *)PTR_ALIGN(&io_, __alignof__(struct io)); struct io *io = (struct io *)PTR_ALIGN(&io_, __alignof__(struct io));
DECLARE_COMPLETION_ONSTACK(wait);
if (num_regions > 1 && (rw & RW_MASK) != WRITE) { if (num_regions > 1 && (rw & RW_MASK) != WRITE) {
WARN_ON(1); WARN_ON(1);
...@@ -392,7 +394,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions, ...@@ -392,7 +394,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions,
io->error_bits = 0; io->error_bits = 0;
atomic_set(&io->count, 1); /* see dispatch_io() */ atomic_set(&io->count, 1); /* see dispatch_io() */
io->sleeper = current; io->wait = &wait;
io->client = client; io->client = client;
io->vma_invalidate_address = dp->vma_invalidate_address; io->vma_invalidate_address = dp->vma_invalidate_address;
...@@ -400,15 +402,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions, ...@@ -400,15 +402,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions,
dispatch_io(rw, num_regions, where, dp, io, 1); dispatch_io(rw, num_regions, where, dp, io, 1);
while (1) { wait_for_completion(&wait);
set_current_state(TASK_UNINTERRUPTIBLE);
if (!atomic_read(&io->count))
break;
io_schedule();
}
set_current_state(TASK_RUNNING);
if (error_bits) if (error_bits)
*error_bits = io->error_bits; *error_bits = io->error_bits;
...@@ -431,7 +425,7 @@ static int async_io(struct dm_io_client *client, unsigned int num_regions, ...@@ -431,7 +425,7 @@ static int async_io(struct dm_io_client *client, unsigned int num_regions,
io = mempool_alloc(client->pool, GFP_NOIO); io = mempool_alloc(client->pool, GFP_NOIO);
io->error_bits = 0; io->error_bits = 0;
atomic_set(&io->count, 1); /* see dispatch_io() */ atomic_set(&io->count, 1); /* see dispatch_io() */
io->sleeper = NULL; io->wait = NULL;
io->client = client; io->client = client;
io->callback = fn; io->callback = fn;
io->context = context; io->context = context;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment