fs/aio: Add explicit RCU grace period when freeing kioctx

While fixing refcounting, e34ecee2ae79 ("aio: Fix a trinity splat")
incorrectly removed explicit RCU grace period before freeing kioctx.
The intention seems to be depending on the internal RCU grace periods
of percpu_ref; however, percpu_ref uses a different flavor of RCU,
sched-RCU.  This can lead to kioctx being freed while RCU read
protected dereferences are still in progress.

Fix it by updating free_ioctx() to go through call_rcu() explicitly.

v2: Comment added to explain double bouncing.

Signed-off-by: Tejun Heo <[email protected]>
Reported-by: Jann Horn <[email protected]>
Fixes: e34ecee2ae79 ("aio: Fix a trinity splat")
Cc: Kent Overstreet <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected] # v3.13+

diff --git a/fs/aio.c b/fs/aio.c
index a062d75109cb380aeb819fe42d43644c8be33a91..eb2e0cfbe2d29bfc7dc01f6ecce349adeb1efa33 100644 (file)
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -115,7 +115,8 @@ struct kioctx {
        struct page             **ring_pages;
        long                    nr_pages;
 
        struct page             **ring_pages;
        long                    nr_pages;
 

-       struct work_struct      free_work;
+       struct rcu_head         free_rcu;
+       struct work_struct      free_work;      /* see free_ioctx() */

 
        /*
         * signals when all in-flight requests are done
 
        /*
         * signals when all in-flight requests are done


@@ -588,6 +589,12 @@ static int kiocb_cancel(struct aio_kiocb *kiocb)
        return cancel(&kiocb->common);
 }
 
        return cancel(&kiocb->common);
 }
 

+/*
+ * free_ioctx() should be RCU delayed to synchronize against the RCU
+ * protected lookup_ioctx() and also needs process context to call
+ * aio_free_ring(), so the double bouncing through kioctx->free_rcu and
+ * ->free_work.
+ */

 static void free_ioctx(struct work_struct *work)
 {
        struct kioctx *ctx = container_of(work, struct kioctx, free_work);
 static void free_ioctx(struct work_struct *work)
 {
        struct kioctx *ctx = container_of(work, struct kioctx, free_work);


@@ -601,6 +608,14 @@ static void free_ioctx(struct work_struct *work)
        kmem_cache_free(kioctx_cachep, ctx);
 }
 
        kmem_cache_free(kioctx_cachep, ctx);
 }
 

+static void free_ioctx_rcufn(struct rcu_head *head)
+{
+       struct kioctx *ctx = container_of(head, struct kioctx, free_rcu);
+
+       INIT_WORK(&ctx->free_work, free_ioctx);
+       schedule_work(&ctx->free_work);
+}
+

 static void free_ioctx_reqs(struct percpu_ref *ref)
 {
        struct kioctx *ctx = container_of(ref, struct kioctx, reqs);
 static void free_ioctx_reqs(struct percpu_ref *ref)
 {
        struct kioctx *ctx = container_of(ref, struct kioctx, reqs);


@@ -609,8 +624,8 @@ static void free_ioctx_reqs(struct percpu_ref *ref)
        if (ctx->rq_wait && atomic_dec_and_test(&ctx->rq_wait->count))
                complete(&ctx->rq_wait->comp);
 
        if (ctx->rq_wait && atomic_dec_and_test(&ctx->rq_wait->count))
                complete(&ctx->rq_wait->comp);
 

-       INIT_WORK(&ctx->free_work, free_ioctx);
-       schedule_work(&ctx->free_work);
+       /* Synchronize against RCU protected table->table[] dereferences */
+       call_rcu(&ctx->free_rcu, free_ioctx_rcufn);

 }
 
 /*
 }
 
 /*


@@ -838,7 +853,7 @@ static int kill_ioctx(struct mm_struct *mm, struct kioctx *ctx,
        table->table[ctx->id] = NULL;
        spin_unlock(&mm->ioctx_lock);
 
        table->table[ctx->id] = NULL;
        spin_unlock(&mm->ioctx_lock);
 

-       /* percpu_ref_kill() will do the necessary call_rcu() */
+       /* free_ioctx_reqs() will do the necessary RCU synchronization */

        wake_up_all(&ctx->wait);
 
        /*
        wake_up_all(&ctx->wait);
 
        /*