dm io: fix a race condition in the wake up code for sync_io
authorJoe Thornber <thornber@redhat.com>
Fri, 27 Jun 2014 19:29:04 +0000 (15:29 -0400)
committerMike Snitzer <snitzer@redhat.com>
Thu, 10 Jul 2014 20:44:14 +0000 (16:44 -0400)
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: Minfei Huang <huangminfei@ucloud.cn>
Signed-off-by: Joe Thornber <thornber@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Acked-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
drivers/md/dm-io.c

index 3842ac738f98ff324f5e1152f08fb546a5bb47fa..db404a0f7e2c83ead70bbf32e2346ecd60aa2edf 100644 (file)
@@ -10,6 +10,7 @@
 #include <linux/device-mapper.h>
 
 #include <linux/bio.h>
+#include <linux/completion.h>
 #include <linux/mempool.h>
 #include <linux/module.h>
 #include <linux/sched.h>
@@ -32,7 +33,7 @@ struct dm_io_client {
 struct io {
        unsigned long error_bits;
        atomic_t count;
-       struct task_struct *sleeper;
+       struct completion *wait;
        struct dm_io_client *client;
        io_notify_fn callback;
        void *context;
@@ -121,8 +122,8 @@ static void dec_count(struct io *io, unsigned int region, int error)
                        invalidate_kernel_vmap_range(io->vma_invalidate_address,
                                                     io->vma_invalidate_size);
 
-               if (io->sleeper)
-                       wake_up_process(io->sleeper);
+               if (io->wait)
+                       complete(io->wait);
 
                else {
                        unsigned long r = io->error_bits;
@@ -387,6 +388,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions,
         */
        volatile char io_[sizeof(struct io) + __alignof__(struct io) - 1];
        struct io *io = (struct io *)PTR_ALIGN(&io_, __alignof__(struct io));
+       DECLARE_COMPLETION_ONSTACK(wait);
 
        if (num_regions > 1 && (rw & RW_MASK) != WRITE) {
                WARN_ON(1);
@@ -395,7 +397,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions,
 
        io->error_bits = 0;
        atomic_set(&io->count, 1); /* see dispatch_io() */
-       io->sleeper = current;
+       io->wait = &wait;
        io->client = client;
 
        io->vma_invalidate_address = dp->vma_invalidate_address;
@@ -403,15 +405,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions,
 
        dispatch_io(rw, num_regions, where, dp, io, 1);
 
-       while (1) {
-               set_current_state(TASK_UNINTERRUPTIBLE);
-
-               if (!atomic_read(&io->count))
-                       break;
-
-               io_schedule();
-       }
-       set_current_state(TASK_RUNNING);
+       wait_for_completion_io(&wait);
 
        if (error_bits)
                *error_bits = io->error_bits;
@@ -434,7 +428,7 @@ static int async_io(struct dm_io_client *client, unsigned int num_regions,
        io = mempool_alloc(client->pool, GFP_NOIO);
        io->error_bits = 0;
        atomic_set(&io->count, 1); /* see dispatch_io() */
-       io->sleeper = NULL;
+       io->wait = NULL;
        io->client = client;
        io->callback = fn;
        io->context = context;