lkml.org 
[lkml]   [2016]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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@stgolabs.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@huawei.com>
Signed-off-by: Davidlohr Bueso <dbueso@suse.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
\
 
 \ /
  Last update: 2016-01-31 23:41    [W:0.347 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site