linux-kernel - Re: [PATCH v2] locktorture: Fix NULL pointer when torture

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-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 2 Feb 2016 16:23:31 -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 Mon, 01 Feb 2016, Paul E. McKenney wrote:

>On Mon, Feb 01, 2016 at 11:28:07AM +0800, Kefeng Wang wrote:

>> Just like I mentioned before, keep consistent with rcutorture???

Because rcutorture does it doesn't mean locktorture has to do it ;)
In any case, I'd suggest the same be done for rcutorture.

[...]

>
>Hmmm...   If nothing happened, then I agree that it makes sense not to
>print any statistics.  But if some testing actually was carried out, then
>we really need to print the statistics.

Right, so how about the following? It introduces an early cleanup helper
that all it does is do torture specific cleanups. I don't really love the
begin/end calls there, but it's not the end of the world and it seems better
than a more messier refactoring. ie, I had also considered adding an 'early'
flag to lock_torture_cleanup() such that we can enable it for this bogus param
scenario, but seems over complicating things and we also call it for such a
small issue.

Thanks,
Davidlohr


diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 8ef1919..05e2649 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -741,6 +741,19 @@ lock_torture_print_module_parms(struct lock_torture_ops *cur_ops,
  		 onoff_interval, onoff_holdoff);
  }
  
+/*
+ * 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.
+ */
+static inline void lock_torture_early_cleanup(void)
+{
+	if (torture_cleanup_begin())
+		return;
+	torture_cleanup_end();
+}
+
  static void lock_torture_cleanup(void)
  {
  	int i;
@@ -811,8 +824,10 @@ 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();
+		lock_torture_early_cleanup();
+		return -EINVAL;
  	}
  	if (cxt.cur_ops->init)
  		cxt.cur_ops->init();