Commit cf535659 authored Mar 11, 2020 by Sahitya Tummala Committed by Greg Kroah-Hartman Apr 17, 2020

block: Fix use-after-free issue accessing struct io_cq



[ 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>

parent 1b16ddb2

block/blk-ioc.c

+7 −0

Original line number	Diff line number	Diff line
		@@ -87,6 +87,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);
		}

		@@ -230,15 +231,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();
		}

		/**

include/linux/iocontext.h

+1 −0

Original line number	Diff line number	Diff line
		@@ -8,6 +8,7 @@

		enum {
		ICQ_EXITED = 1 << 2,
		ICQ_DESTROYED = 1 << 3,
		};

		/*