On Mon, Mar 07, 2016 at 03:02:05PM +0800, Kefeng Wang wrote: > > > On 2016/3/7 13:40, Davidlohr Bueso wrote: > > 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. > > This one looks good, and tested on my board.
Very good! May we add your Tested-by? Thanx, Paul > > -----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 <wangkefeng.w...@huawei.com> > > Signed-off-by: Davidlohr Bueso <dbu...@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 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. */ >