| 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
| ||
|
Message-ID: <20160131221736.GB16147@linux-uzut.site>
Date: Sun, 31 Jan 2016 14:17:36 -0800
From: Davidlohr Bueso <dave@...olabs.net>
To: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc: Kefeng Wang <wangkefeng.wang@...wei.com>,
linux-kernel@...r.kernel.org, peterz@...radead.org,
mingo@...hat.com, Josh Triplett <josh@...htriplett.org>,
"Guohanjun (Hanjun Guo)" <guohanjun@...wei.com>
Subject: Re: [PATCH v2] locktorture: Fix NULL pointer when torture_type is
invalid
On Sat, 30 Jan 2016, Paul E. McKenney wrote:
>> On 2016/1/28 12:25, Kefeng Wang wrote:
>> > Insmod locktorture with torture_type=mutex will lead to crash,
You actually want mutex_lock here, we always use the _lock suffix, mainly
because it all started out with spin_lock. And you just showed how fragile
this is -- I'd say most of use use this module in a scripted setup, which
is why it was not seen before.
>> >
>> > Unable to handle kernel NULL pointer dereference at virtual address 00000008
>> > pgd = ffffffc0f6c10000
>> > [00000008] *pgd=000000013b221003, *pud=000000013b221003, *pmd=0000000000000000a
>> > Internal error: Oops: 94000006 [#1] PREEMPT SMP
>> > Modules linked in: locktorture(+) torture
>> > CPU: 2 PID: 1462 Comm: insmod Not tainted 4.4.0+ #19
>> > Hardware name: linux,dummy-virt (DT)
>> > task: ffffffc0fb2b3700 ti: ffffffc0fa938000 task.ti: ffffffc0fa938000
>> > PC is at __torture_print_stats+0x18/0x180 [locktorture]
>> > LR is at lock_torture_stats_print+0x68/0x110 [locktorture]
>> > pc : [<ffffffbffc017028>] lr : [<ffffffbffc017500>] pstate: 60000145
>> > sp : ffffffc0fa93bb20
>> > [snip...]
>> > Call trace:
>> > [<ffffffbffc017028>] __torture_print_stats+0x18/0x180 [locktorture]
>> > [<ffffffbffc017500>] lock_torture_stats_print+0x68/0x110 [locktorture]
>> > [<ffffffbffc0180fc>] lock_torture_cleanup+0xc4/0x278 [locktorture]
>> > [<ffffffbffc01d144>] lock_torture_init+0x144/0x5b0 [locktorture]
>> > [<ffffffc000082940>] do_one_initcall+0x94/0x1a0
>> > [<ffffffc000141888>] do_init_module+0x60/0x1c8
>> > [<ffffffc00011c628>] load_module+0x1880/0x1c9c
>> > [<ffffffc00011cc00>] SyS_finit_module+0x7c/0x88
>> > [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
>> >
>> > Fix it by check statp in __torture_print_stats(), if it is NULL, just
>> > create a lock-torture-statistics message with zero statistic argument.
>>
>> It is keep to get the statistics printout at the end if with bad argument,
>> what's your opinion about this version???
So we shouldn't be doing anything with statistics here in the first place, as
it was a bogus argument. Instead, we should just exit with EINVAL. Something
like the below does the trick for me.
Thanks,
Davidlohr
----8<--------------------------------------------------------------------------
From: Davidlohr Bueso <dave@...olabs.net>
Subject: [PATCH] locktorture: Do not deal with statistics upon bogus input
Kefeng reported the following nil pointer dereference when
passing an invalid lock type to torture.
[ 114.887250] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
[ 114.887254] IP: [<ffffffffa077808d>] __torture_print_stats+0x1d/0x100 [locktorture]
...
[ 114.887315] CPU: 6 PID: 7080 Comm: modprobe Tainted: G I E 4.5.0-rc1-52.39-default+ #2
[ 114.887316] Hardware name: Dell Inc. PowerEdge R510/0W844P, BIOS 1.3.6 05/25/2010
[ 114.887318] task: ffff880196610040 ti: ffff8801a8ca4000 task.ti: ffff8801a8ca4000
[ 114.887322] RIP: 0010:[<ffffffffa077808d>] [<ffffffffa077808d>] __torture_print_stats+0x1d/0x100 [locktorture]
[ 114.887324] RSP: 0018:ffff8801a8ca7c38 EFLAGS: 00010202
[ 114.887326] RAX: 0000000000000000 RBX: 0000000000004000 RCX: ffffea0006a12b20
[ 114.887327] RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff8801aa7bc000
[ 114.887328] RBP: ffff8801a8ca7c58 R08: 0000000000000004 R09: ffff8801aa7bc000
[ 114.887329] R10: 000000000001c1a8 R11: ffff8801afc77d10 R12: 0000000000004000
[ 114.887330] R13: ffff8801aa7bc000 R14: ffff8801aaa312b0 R15: ffff8801a8ca7ec8
[ 114.887332] FS: 00007f13182bc700(0000) GS:ffff8801afc60000(0000) knlGS:0000000000000000
[ 114.887333] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 114.887334] CR2: 0000000000000008 CR3: 0000000196eb1000 CR4: 00000000000006e0
[ 114.887335] Stack:
[ 114.887337] 0000000000004000 ffffffffa077a000 ffff8801aaa312b0 0000000000004000
[ 114.887339] ffff8801a8ca7c80 ffffffffa0778ba1 0000000000000048 00000000ffffffff
[ 114.887341] ffffffffa077a000 ffff8801a8ca7cd0 ffffffffa0778d6d ffff8801a8ca7cb0
[ 114.887341] Call Trace:
[ 114.887347] [<ffffffffa0778ba1>] lock_torture_stats_print+0x71/0x100 [locktorture]
[ 114.887351] [<ffffffffa0778d6d>] lock_torture_cleanup+0xcd/0x28b [locktorture]
[ 114.887358] [<ffffffff81084f6c>] ? blocking_notifier_chain_register+0x5c/0xa0
[ 114.887361] [<ffffffff81085f88>] ? register_reboot_notifier+0x18/0x20
[ 114.887365] [<ffffffffa077e0fa>] lock_torture_init+0xfa/0x1000 [locktorture]
[ 114.887367] [<ffffffffa077e000>] ? 0xffffffffa077e000
[ 114.887372] [<ffffffff810003dd>] do_one_initcall+0xad/0x1e0
There is no reason for us to be calling into statistics logic
as we should just return to the user:
lock-torture: invalid torture type: "mutex"
Reported-by: Kefeng Wang <wangkefeng.wang@...wei.com>
Signed-off-by: Davidlohr Bueso <dbueso@...e.de>
---
kernel/locking/locktorture.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 8ef1919..f613fff 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -811,8 +811,9 @@ static int __init lock_torture_init(void)
for (i = 0; i < ARRAY_SIZE(torture_ops); i++)
pr_alert(" %s", torture_ops[i]->name);
pr_alert("\n");
- firsterr = -EINVAL;
- goto unwind;
+
+ torture_init_end();
+ return -EINVAL;
}
if (cxt.cur_ops->init)
cxt.cur_ops->init();
--
2.1.4
Powered by blists - more mailing lists