Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752056AbcCGFkT (ORCPT ); Mon, 7 Mar 2016 00:40:19 -0500 Received: from mx2.suse.de ([195.135.220.15]:47477 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751357AbcCGFkP (ORCPT ); Mon, 7 Mar 2016 00:40:15 -0500 Date: Sun, 6 Mar 2016 21:40:07 -0800 From: Davidlohr Bueso To: Kefeng Wang Cc: paulmck@linux.vnet.ibm.com, linux-kernel@vger.kernel.org, peterz@infradead.org, mingo@redhat.com, Josh Triplett , "Guohanjun (Hanjun Guo)" Subject: Re: [PATCH v2] locktorture: Fix NULL pointer when torture_type is invalid Message-ID: <20160307054007.GA8192@linux-uzut.site> References: <20160201030235.GC16147@linux-uzut.site> <56AED0C7.7050505@huawei.com> <20160202064635.GH6719@linux.vnet.ibm.com> <20160203002331.GA3385@linux-uzut.site> <20160302195543.GA12593@linux-uzut.site> <20160302211216.GC3577@linux.vnet.ibm.com> <56D79566.7010302@huawei.com> <56D7BE33.5010605@huawei.com> <20160303083626.GA10957@linux-uzut.site> <56DCE0D4.1060109@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <56DCE0D4.1060109@huawei.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3009 Lines: 87 On Mon, 07 Mar 2016, Kefeng Wang wrote: >On 2016/3/3 16:36, Davidlohr Bueso wrote: >> + /* >> + * 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; > >Sorry for the late response, the cxt.lrsa should be taken into account too. I am taking it into account, note that we kfree lwsa if lrsa fails memory allocation. Of course we should be defensive, so go ahead and explicitly set it to nil. v2 below, otherwise same patch. -----8<-------------------------- Subject: [PATCH v2] locktorture: Fix nil pointer dereferencing for cleanup paths 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-by: Kefeng Wang Signed-off-by: Davidlohr Bueso --- kernel/locking/locktorture.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c index 8ef1919..b5bc243 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