linux-kernel - [PATCH] block: systemace: Fix refcount underflow in error handler

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1488756960-3659-1-git-send-email-linux@roeck-us.net>
Date:   Sun,  5 Mar 2017 15:36:00 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Michal Simek <michal.simek@...inx.com>
Cc:     Sören Brinkmann <soren.brinkmann@...inx.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Guenter Roeck <linux@...ck-us.net>
Subject: [PATCH] block: systemace: Fix refcount underflow in error handler

Since commit 29dee3c03abc ("locking/refcounts: Out-of-line everything"),
the following runtime warning is seen if xsysace fails to initialize.
The commit only exposes the problem.

refcount_t: underflow; use-after-free.
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at lib/refcount.c:128 refcount_sub_and_test+0x90/0xd0
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.11.0-rc1+ #1
task: cf81d5a0 task.stack: cf81e000
NIP: c02104d0 LR: c02104d0 CTR: c0279d90
REGS: cf81fc80 TRAP: 0700   Not tainted  (4.11.0-rc1+)
MSR: 00029000 <CE,EE,ME>
CR: 24000022  XER: 00000000

GPR00: c02104d0 cf81fd30 cf81d5a0 00000026 00000000 00000000 c027a7f0 00000000
GPR08: c05412dc 00000800 00000000 00000000 24000024 00000000 c0001ad0 00000000
GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 c054b3c0 c0550000
GPR24: 00000000 00000001 cf95a010 00000015 00000000 00000000 cfb2b2c0 cfb2b0e8
NIP [c02104d0] refcount_sub_and_test+0x90/0xd0
LR [c02104d0] refcount_sub_and_test+0x90/0xd0
Call Trace:
[cf81fd30] [c02104d0] refcount_sub_and_test+0x90/0xd0 (unreliable)
[cf81fd40] [c01f68c4] kobject_put+0x34/0x90
[cf81fd50] [c01cef64] blk_cleanup_queue+0x164/0x1e0
[cf81fd60] [c02acab0] ace_probe+0x4c0/0x510
[cf81fda0] [c0293b64] platform_drv_probe+0x44/0xc0
[cf81fdc0] [c0291974] driver_probe_device+0x234/0x340
[cf81fdf0] [c0291b4c] __driver_attach+0xcc/0xd0
[cf81fe10] [c028f4d8] bus_for_each_dev+0x68/0xc0
[cf81fe40] [c0290d18] bus_add_driver+0x208/0x280
[cf81fe60] [c0292678] driver_register+0x88/0x140
[cf81fe70] [c050ce24] ace_init+0x48/0xa4
[cf81fe90] [c0001350] do_one_initcall+0x40/0x180
[cf81fef0] [c04f3ae0] kernel_init_freeable+0x134/0x1d0
[cf81ff30] [c0001ae4] kernel_init+0x14/0x110
[cf81ff40] [c000c3d0] ret_from_kernel_thread+0x5c/0x64

Debugging shows that blk_put_queue() is called twice during error
handling, once from disk_release() (called from put_disk) and once
from blk_cleanup_queue(). disk_release() only calls blk_put_queue()
if the queue pointer is initialized, so moving the initialization of
gd->queue avoids the problem.

Signed-off-by: Guenter Roeck <linux@...ck-us.net>
---
 drivers/block/xsysace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index 757dce2147e0..a22b38968930 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -1004,7 +1004,6 @@ static int ace_setup(struct ace_device *ace)
 	ace->gd->major = ace_major;
 	ace->gd->first_minor = ace->id * ACE_NUM_MINORS;
 	ace->gd->fops = &ace_fops;
-	ace->gd->queue = ace->queue;
 	ace->gd->private_data = ace;
 	snprintf(ace->gd->disk_name, 32, "xs%c", ace->id + 'a');
 
@@ -1032,6 +1031,8 @@ static int ace_setup(struct ace_device *ace)
 	ace_out(ace, ACE_CTRL, ACE_CTRL_FORCECFGMODE |
 		ACE_CTRL_DATABUFRDYIRQ | ACE_CTRL_ERRORIRQ);
 
+	ace->gd->queue = ace->queue;
+
 	/* Now we can hook up the irq handler */
 	if (ace->irq) {
 		rc = request_irq(ace->irq, ace_interrupt, 0, "systemace", ace);
-- 
2.7.4