All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -rcu 0/2] locktorture fixes
@ 2016-03-09  7:08 Davidlohr Bueso
  2016-03-09  7:08 ` [PATCH 1/2] locktorture: Fix deboosting nil ptr dereferencing Davidlohr Bueso
  2016-03-09  7:08 ` [PATCH 2/2] locktorture: Fix nil pointer dereferencing for cleanup paths Davidlohr Bueso
  0 siblings, 2 replies; 6+ messages in thread
From: Davidlohr Bueso @ 2016-03-09  7:08 UTC (permalink / raw)
  To: paulmck
  Cc: mingo, wangkefeng.wang, peterz, josh, guohanjun, dave, linux-kernel

Hi Paul,

Here are the requested refreshed patches that fix two nil ptr problems.
Applies on top of -rcu/next, although this file has not really changed
between your tree and tip, which is where I was basing the original changes
 against.

Thanks!

Davidlohr Bueso (2):
  locktorture: Fix deboosting nil ptr dereferencing
  locktorture: Fix nil pointer dereferencing for cleanup paths

 kernel/locking/locktorture.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/2] locktorture: Fix deboosting nil ptr dereferencing
  2016-03-09  7:08 [PATCH -rcu 0/2] locktorture fixes Davidlohr Bueso
@ 2016-03-09  7:08 ` Davidlohr Bueso
  2016-03-09 12:18   ` Paul E. McKenney
  2016-03-09  7:08 ` [PATCH 2/2] locktorture: Fix nil pointer dereferencing for cleanup paths Davidlohr Bueso
  1 sibling, 1 reply; 6+ messages in thread
From: Davidlohr Bueso @ 2016-03-09  7:08 UTC (permalink / raw)
  To: paulmck
  Cc: mingo, wangkefeng.wang, peterz, josh, guohanjun, dave,
	linux-kernel, Davidlohr Bueso

For the case of rtmutex torturing we will randomly call into the
boost() handler, including upon module exiting when the tasks are
deboosted before stopping. In such cases the task may or may not have
already been boosted, and therefore the NULL being explicitly passed
can occur anywhere. Currently we only assume that the task will is
at a higher prio, and in consequence, dereference a nil pointer.

This patch fixes the case of a rmmod locktorture exploding while
pounding on the rtmutex lock (partial trace):

[83317.452251] task: ffff88081026cf80 ti: ffff880816120000 task.ti: ffff880816120000
[83317.452258] RIP: 0010:[<ffffffffa05c6185>]  [<ffffffffa05c6185>] torture_random+0x5/0x60 [torture]
[83317.452261] RSP: 0018:ffff880816123eb0  EFLAGS: 00010206
[83317.452264] RAX: ffff88081026cf80 RBX: ffff880816bfa630 RCX: 0000000000160d1b
[83317.452267] RDX: 0000000000000000 RSI: 0000000000000202 RDI: 0000000000000000
[83317.452269] RBP: ffff88081026cf80 R08: 000000000000001f R09: ffff88017c20ca80
[83317.452271] R10: 0000000000000000 R11: 000000000048c316 R12: ffffffffa05d1840
[83317.452273] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[83317.452275] FS:  0000000000000000(0000) GS:ffff88203f880000(0000) knlGS:0000000000000000
[83317.452277] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[83317.452279] CR2: 0000000000000008 CR3: 0000000001c0a000 CR4: 00000000000406e0
[83317.452281] Stack:
[83317.452288]  ffffffffa05d141d ffff880816bfa630 ffffffffa05d1922 ffff88081e70c2c0
[83317.452295]  ffff880816bfa630 ffffffff81095fed 0000000000000000 ffffffff8107bf60
[83317.452297]  ffff880816bfa630 ffffffff00000000 ffff880800000000 ffff880816123f08
[83317.452297] Call Trace:
[83317.452309]  [<ffffffffa05d141d>] torture_rtmutex_boost+0x1d/0x90 [locktorture]
[83317.452315]  [<ffffffffa05d1922>] lock_torture_writer+0xe2/0x170 [locktorture]
[83317.452321]  [<ffffffff81095fed>] kthread+0xbd/0xe0
[83317.452325]  [<ffffffff815cf40f>] ret_from_fork+0x3f/0x70

This patch ensures that if the random state pointer is not nil and current
is not boosted, then do nothing.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 kernel/locking/locktorture.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 8ef1919d63b2..9e9c5f454f5c 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -394,12 +394,12 @@ static void torture_rtmutex_boost(struct torture_random_state *trsp)
 
 	if (!rt_task(current)) {
 		/*
-		 * (1) Boost priority once every ~50k operations. When the
+		 * Boost priority once every ~50k operations. When the
 		 * task tries to take the lock, the rtmutex it will account
 		 * for the new priority, and do any corresponding pi-dance.
 		 */
-		if (!(torture_random(trsp) %
-		      (cxt.nrealwriters_stress * factor))) {
+		if (trsp && !(torture_random(trsp) %
+			      (cxt.nrealwriters_stress * factor))) {
 			policy = SCHED_FIFO;
 			param.sched_priority = MAX_RT_PRIO - 1;
 		} else /* common case, do nothing */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] locktorture: Fix nil pointer dereferencing for cleanup paths
  2016-03-09  7:08 [PATCH -rcu 0/2] locktorture fixes Davidlohr Bueso
  2016-03-09  7:08 ` [PATCH 1/2] locktorture: Fix deboosting nil ptr dereferencing Davidlohr Bueso
@ 2016-03-09  7:08 ` Davidlohr Bueso
  1 sibling, 0 replies; 6+ messages in thread
From: Davidlohr Bueso @ 2016-03-09  7:08 UTC (permalink / raw)
  To: paulmck
  Cc: mingo, wangkefeng.wang, peterz, josh, guohanjun, dave,
	linux-kernel, Davidlohr Bueso

It has been found that paths that invoke cleanups through
lock_torture_cleanup() can incur in nil pointer dereferencing
bugs during the statistics printing phase. This is mainly
because we should not be calling into statistics before we are
sure things have been setup correctly.

Specifically, early checks (and the need for handling this in
the cleanup call) only include parameter checks and basic
statistics allocation. Once we start write/read kthreads
we then consider the test as started. As such, update the func
in question to check for cxt.lwsa writer stats, if not set,
we either have a bogus parameter or ENOMEM situation and
therefore only need to deal with general torture calls

Reported-and-tested-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 kernel/locking/locktorture.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 9e9c5f454f5c..d066a50dc87e 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -748,6 +748,15 @@ static void lock_torture_cleanup(void)
 	if (torture_cleanup_begin())
 		return;
 
+	/*
+	 * Indicates early cleanup, meaning that the test has not run,
+	 * such as when passing bogus args when loading the module. As
+	 * such, only perform the underlying torture-specific cleanups,
+	 * and avoid anything related to locktorture.
+	 */
+	if (!cxt.lwsa)
+		goto end;
+
 	if (writer_tasks) {
 		for (i = 0; i < cxt.nrealwriters_stress; i++)
 			torture_stop_kthread(lock_torture_writer,
@@ -776,6 +785,7 @@ static void lock_torture_cleanup(void)
 	else
 		lock_torture_print_module_parms(cxt.cur_ops,
 						"End of test: SUCCESS");
+end:
 	torture_cleanup_end();
 }
 
@@ -870,6 +880,7 @@ static int __init lock_torture_init(void)
 			VERBOSE_TOROUT_STRING("cxt.lrsa: Out of memory");
 			firsterr = -ENOMEM;
 			kfree(cxt.lwsa);
+			cxt.lwsa = NULL;
 			goto unwind;
 		}
 
@@ -878,6 +889,7 @@ static int __init lock_torture_init(void)
 			cxt.lrsa[i].n_lock_acquired = 0;
 		}
 	}
+
 	lock_torture_print_module_parms(cxt.cur_ops, "Start of test");
 
 	/* Prepare torture context. */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] locktorture: Fix deboosting nil ptr dereferencing
  2016-03-09  7:08 ` [PATCH 1/2] locktorture: Fix deboosting nil ptr dereferencing Davidlohr Bueso
@ 2016-03-09 12:18   ` Paul E. McKenney
  2016-03-09 19:05     ` Davidlohr Bueso
  0 siblings, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2016-03-09 12:18 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: mingo, wangkefeng.wang, peterz, josh, guohanjun, linux-kernel,
	Davidlohr Bueso

On Tue, Mar 08, 2016 at 11:08:05PM -0800, Davidlohr Bueso wrote:
> For the case of rtmutex torturing we will randomly call into the
> boost() handler, including upon module exiting when the tasks are
> deboosted before stopping. In such cases the task may or may not have
> already been boosted, and therefore the NULL being explicitly passed
> can occur anywhere. Currently we only assume that the task will is
> at a higher prio, and in consequence, dereference a nil pointer.
> 
> This patch fixes the case of a rmmod locktorture exploding while
> pounding on the rtmutex lock (partial trace):
> 
> [83317.452251] task: ffff88081026cf80 ti: ffff880816120000 task.ti: ffff880816120000
> [83317.452258] RIP: 0010:[<ffffffffa05c6185>]  [<ffffffffa05c6185>] torture_random+0x5/0x60 [torture]
> [83317.452261] RSP: 0018:ffff880816123eb0  EFLAGS: 00010206
> [83317.452264] RAX: ffff88081026cf80 RBX: ffff880816bfa630 RCX: 0000000000160d1b
> [83317.452267] RDX: 0000000000000000 RSI: 0000000000000202 RDI: 0000000000000000
> [83317.452269] RBP: ffff88081026cf80 R08: 000000000000001f R09: ffff88017c20ca80
> [83317.452271] R10: 0000000000000000 R11: 000000000048c316 R12: ffffffffa05d1840
> [83317.452273] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [83317.452275] FS:  0000000000000000(0000) GS:ffff88203f880000(0000) knlGS:0000000000000000
> [83317.452277] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [83317.452279] CR2: 0000000000000008 CR3: 0000000001c0a000 CR4: 00000000000406e0
> [83317.452281] Stack:
> [83317.452288]  ffffffffa05d141d ffff880816bfa630 ffffffffa05d1922 ffff88081e70c2c0
> [83317.452295]  ffff880816bfa630 ffffffff81095fed 0000000000000000 ffffffff8107bf60
> [83317.452297]  ffff880816bfa630 ffffffff00000000 ffff880800000000 ffff880816123f08
> [83317.452297] Call Trace:
> [83317.452309]  [<ffffffffa05d141d>] torture_rtmutex_boost+0x1d/0x90 [locktorture]
> [83317.452315]  [<ffffffffa05d1922>] lock_torture_writer+0xe2/0x170 [locktorture]
> [83317.452321]  [<ffffffff81095fed>] kthread+0xbd/0xe0
> [83317.452325]  [<ffffffff815cf40f>] ret_from_fork+0x3f/0x70
> 
> This patch ensures that if the random state pointer is not nil and current
> is not boosted, then do nothing.
> 
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>

Queued for 4.7, thank you!!!

(If you need it earlier, please let me know.)

							Thanx, Paul

> ---
>  kernel/locking/locktorture.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
> index 8ef1919d63b2..9e9c5f454f5c 100644
> --- a/kernel/locking/locktorture.c
> +++ b/kernel/locking/locktorture.c
> @@ -394,12 +394,12 @@ static void torture_rtmutex_boost(struct torture_random_state *trsp)
> 
>  	if (!rt_task(current)) {
>  		/*
> -		 * (1) Boost priority once every ~50k operations. When the
> +		 * Boost priority once every ~50k operations. When the
>  		 * task tries to take the lock, the rtmutex it will account
>  		 * for the new priority, and do any corresponding pi-dance.
>  		 */
> -		if (!(torture_random(trsp) %
> -		      (cxt.nrealwriters_stress * factor))) {
> +		if (trsp && !(torture_random(trsp) %
> +			      (cxt.nrealwriters_stress * factor))) {
>  			policy = SCHED_FIFO;
>  			param.sched_priority = MAX_RT_PRIO - 1;
>  		} else /* common case, do nothing */
> -- 
> 2.1.4
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] locktorture: Fix deboosting nil ptr dereferencing
  2016-03-09 12:18   ` Paul E. McKenney
@ 2016-03-09 19:05     ` Davidlohr Bueso
  2016-03-10  2:24       ` Paul E. McKenney
  0 siblings, 1 reply; 6+ messages in thread
From: Davidlohr Bueso @ 2016-03-09 19:05 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: mingo, wangkefeng.wang, peterz, josh, guohanjun, linux-kernel,
	Davidlohr Bueso

On Wed, 09 Mar 2016, Paul E. McKenney wrote:
>Queued for 4.7, thank you!!!
>
>(If you need it earlier, please let me know.)

Thanks, Paul. Could we have them for 4.6 at least? Both are small and
rather trivial but fix really annoying problems. ie, I've now come to
see that the rtmutex torturing is pretty useless if you plan on exiting
the run without the fix.

Thanks,
Davidlohr

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] locktorture: Fix deboosting nil ptr dereferencing
  2016-03-09 19:05     ` Davidlohr Bueso
@ 2016-03-10  2:24       ` Paul E. McKenney
  0 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2016-03-10  2:24 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: mingo, wangkefeng.wang, peterz, josh, guohanjun, linux-kernel,
	Davidlohr Bueso

On Wed, Mar 09, 2016 at 11:05:01AM -0800, Davidlohr Bueso wrote:
> On Wed, 09 Mar 2016, Paul E. McKenney wrote:
> >Queued for 4.7, thank you!!!
> >
> >(If you need it earlier, please let me know.)
> 
> Thanks, Paul. Could we have them for 4.6 at least? Both are small and
> rather trivial but fix really annoying problems. ie, I've now come to
> see that the rtmutex torturing is pretty useless if you plan on exiting
> the run without the fix.

OK, rebased them down to just after my recent pull request and set
rcu/next to that spot.  If no problems from -next testing and no
valid complaints, I will send them along.

							Thanx, Paul

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-10  2:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-09  7:08 [PATCH -rcu 0/2] locktorture fixes Davidlohr Bueso
2016-03-09  7:08 ` [PATCH 1/2] locktorture: Fix deboosting nil ptr dereferencing Davidlohr Bueso
2016-03-09 12:18   ` Paul E. McKenney
2016-03-09 19:05     ` Davidlohr Bueso
2016-03-10  2:24       ` Paul E. McKenney
2016-03-09  7:08 ` [PATCH 2/2] locktorture: Fix nil pointer dereferencing for cleanup paths Davidlohr Bueso

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.