block: Fix use-after-free issue accessing struct io_cq
authorSahitya Tummala <stummala@codeaurora.org>
Wed, 11 Mar 2020 10:37:50 +0000 (16:07 +0530)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 17 Apr 2020 08:50:04 +0000 (10:50 +0200)
[ Upstream commit 30a2da7b7e225ef6c87a660419ea04d3cef3f6a7 ]

There is a potential race between ioc_release_fn() and
ioc_clear_queue() as shown below, due to which below kernel
crash is observed. It also can result into use-after-free
issue.

context#1: context#2:
ioc_release_fn() __ioc_clear_queue() gets the same icq
->spin_lock(&ioc->lock); ->spin_lock(&ioc->lock);
->ioc_destroy_icq(icq);
  ->list_del_init(&icq->q_node);
  ->call_rcu(&icq->__rcu_head,
   icq_free_icq_rcu);
->spin_unlock(&ioc->lock);
->ioc_destroy_icq(icq);
  ->hlist_del_init(&icq->ioc_node);
  This results into below crash as this memory
  is now used by icq->__rcu_head in context#1.
  There is a chance that icq could be free'd
  as well.

22150.386550:   <6> Unable to handle kernel write to read-only memory
at virtual address ffffffaa8d31ca50
...
Call trace:
22150.607350:   <2>  ioc_destroy_icq+0x44/0x110
22150.611202:   <2>  ioc_clear_queue+0xac/0x148
22150.615056:   <2>  blk_cleanup_queue+0x11c/0x1a0
22150.619174:   <2>  __scsi_remove_device+0xdc/0x128
22150.623465:   <2>  scsi_forget_host+0x2c/0x78
22150.627315:   <2>  scsi_remove_host+0x7c/0x2a0
22150.631257:   <2>  usb_stor_disconnect+0x74/0xc8
22150.635371:   <2>  usb_unbind_interface+0xc8/0x278
22150.639665:   <2>  device_release_driver_internal+0x198/0x250
22150.644897:   <2>  device_release_driver+0x24/0x30
22150.649176:   <2>  bus_remove_device+0xec/0x140
22150.653204:   <2>  device_del+0x270/0x460
22150.656712:   <2>  usb_disable_device+0x120/0x390
22150.660918:   <2>  usb_disconnect+0xf4/0x2e0
22150.664684:   <2>  hub_event+0xd70/0x17e8
22150.668197:   <2>  process_one_work+0x210/0x480
22150.672222:   <2>  worker_thread+0x32c/0x4c8

Fix this by adding a new ICQ_DESTROYED flag in ioc_destroy_icq() to
indicate this icq is once marked as destroyed. Also, ensure
__ioc_clear_queue() is accessing icq within rcu_read_lock/unlock so
that icq doesn't get free'd up while it is still using it.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Co-developed-by: Pradeep P V K <ppvk@codeaurora.org>
Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
block/blk-ioc.c
include/linux/iocontext.h

index 5ed59ac..9df50fb 100644 (file)
@@ -84,6 +84,7 @@ static void ioc_destroy_icq(struct io_cq *icq)
         * making it impossible to determine icq_cache.  Record it in @icq.
         */
        icq->__rcu_icq_cache = et->icq_cache;
+       icq->flags |= ICQ_DESTROYED;
        call_rcu(&icq->__rcu_head, icq_free_icq_rcu);
 }
 
@@ -212,15 +213,21 @@ static void __ioc_clear_queue(struct list_head *icq_list)
 {
        unsigned long flags;
 
+       rcu_read_lock();
        while (!list_empty(icq_list)) {
                struct io_cq *icq = list_entry(icq_list->next,
                                                struct io_cq, q_node);
                struct io_context *ioc = icq->ioc;
 
                spin_lock_irqsave(&ioc->lock, flags);
+               if (icq->flags & ICQ_DESTROYED) {
+                       spin_unlock_irqrestore(&ioc->lock, flags);
+                       continue;
+               }
                ioc_destroy_icq(icq);
                spin_unlock_irqrestore(&ioc->lock, flags);
        }
+       rcu_read_unlock();
 }
 
 /**
index dba15ca..1dcd919 100644 (file)
@@ -8,6 +8,7 @@
 
 enum {
        ICQ_EXITED              = 1 << 2,
+       ICQ_DESTROYED           = 1 << 3,
 };
 
 /*