Hello Jens, Andrew
We have accidentally commit "loop: queue_lock NULL pointer
derefence in blk_throtl_exit V1" applied: ee71a968672a9951aee6014c55511007596425bc
It's not a huge problem, but I think, it's better to have V3 applied
https://lkml.org/lkml/2011/1/21/158
since V3 targets all the throttle users, not only loop (by moving
`fall-back' from loop_free to blk_release_queue).
V3 is in Andrew's mmotm as for now
loop-queue_lock-null-pointer-derefence-in-blk_throtl_exit-v3.patch
Could we please revert ee71a968672a9951aee6014c55511007596425bc when
loop-queue_lock-null-pointer-derefence-in-blk_throtl_exit-v3 will land
in the kernel?
Sorry.
Thanks in advance,
Sergey
On 2011-02-11 13:59, Sergey Senozhatsky wrote:
> Hello Jens, Andrew
>
> We have accidentally commit "loop: queue_lock NULL pointer
> derefence in blk_throtl_exit V1" applied: ee71a968672a9951aee6014c55511007596425bc
>
> It's not a huge problem, but I think, it's better to have V3 applied
> https://lkml.org/lkml/2011/1/21/158
>
> since V3 targets all the throttle users, not only loop (by moving
> `fall-back' from loop_free to blk_release_queue).
>
> V3 is in Andrew's mmotm as for now
> loop-queue_lock-null-pointer-derefence-in-blk_throtl_exit-v3.patch
>
> Could we please revert ee71a968672a9951aee6014c55511007596425bc when
> loop-queue_lock-null-pointer-derefence-in-blk_throtl_exit-v3 will land
> in the kernel?
Send an incremental patch if there's overlap, otherwise we'll do the
revert. I'm on my way out the door and I'll be away from the computer
all of next week, so perhaps Andrew can assist you in upstreaming such a
patch?
--
Jens Axboe
block: fix queue_lock NULL pointer derefence in blk_throtl_exit
BUG: unable to handle kernel NULL pointer dereference at 0000000000000004
IP: [<ffffffff812479d4>] do_raw_spin_lock+0x14/0x122
Process modprobe (pid: 6189, threadinfo ffff88009a898000, task
ffff880154a88000)
Call Trace:
[<ffffffff81486788>] _raw_spin_lock_irq+0x4a/0x51
[<ffffffff8123404b>] ? blk_throtl_exit+0x3b/0xa0
[<ffffffff8105b120>] ? cancel_delayed_work_sync+0xd/0xf
[<ffffffff8123404b>] blk_throtl_exit+0x3b/0xa0
[<ffffffff81229bc8>] blk_release_queue+0x21/0x65
[<ffffffff8123bb06>] kobject_release+0x51/0x66
[<ffffffff8123bab5>] ? kobject_release+0x0/0x66
[<ffffffff8123ce1e>] kref_put+0x43/0x4d
[<ffffffff8123ba27>] kobject_put+0x47/0x4b
[<ffffffff8122717c>] blk_cleanup_queue+0x56/0x5b
[<ffffffffa01c3824>] loop_exit+0x68/0x844 [loop]
[<ffffffff8107cccc>] sys_delete_module+0x1e8/0x25b
[<ffffffff814864c9>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[<ffffffff81002112>] system_call_fastpath+0x16/0x1b
because of an attempt to acquire NULL queue_lock.
Added the same lines as in blk_queue_make_request:
'fall back to embedded per-queue lock' - when call blk_release_queue on
allocated but never initialized queue.
v4:
Incremental patch to move fall back to embedded per-queue lock from loop_free
to blk_release_queue, because v1 patch was accidentally applied.
v3: According to comment by Vivek Goyal, queue_lock NULL check and fix moved
out from loop driver code to blk_release_queue.
Signed-off-by: Sergey Senozhatsky <[email protected]>
---
block/blk-sysfs.c | 6 ++++++
drivers/block/loop.c | 3 ---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 41fb691..2b426ac 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -471,6 +471,12 @@ static void blk_release_queue(struct kobject *kobj)
blk_sync_queue(q);
+ /* It's possible that blk_release_queue will be called on allocated
+ * but never initialized queue. Fall back to our embedded per-queue
+ * locks in this case. */
+ if (!q->queue_lock)
+ q->queue_lock = &q->__queue_lock;
+
blk_throtl_exit(q);
if (rl->rq_pool)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 49e6a54..44e18c0 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1641,9 +1641,6 @@ out:
static void loop_free(struct loop_device *lo)
{
- if (!lo->lo_queue->queue_lock)
- lo->lo_queue->queue_lock = &lo->lo_queue->__queue_lock;
-
blk_cleanup_queue(lo->lo_queue);
put_disk(lo->lo_disk);
list_del(&lo->lo_list);