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]

Message-ID: <20160307133745.GQ3577@linux.vnet.ibm.com>
Date:	Mon, 7 Mar 2016 05:37:45 -0800
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Kefeng Wang <wangkefeng.wang@...wei.com>
Cc:	Davidlohr Bueso <dave@...olabs.net>, 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, 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.wang@...wei.com>
> > Signed-off-by: Davidlohr Bueso <dbueso@...e.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. */
>