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. > > -----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 <[email protected]> > Signed-off-by: Davidlohr Bueso <[email protected]> > --- > 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. */
