linux-kernel - Re: [PATCH v2 1/4] cdx: Introduce lock to protect controller ops and controller list

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: <2023080105-eggnog-probably-9ff3@gregkh>
Date:   Tue, 1 Aug 2023 10:40:38 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     "Gangurde, Abhijit" <abhijit.gangurde@....com>
Cc:     "masahiroy@...nel.org" <masahiroy@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Simek, Michal" <michal.simek@....com>,
        "git (AMD-Xilinx)" <git@....com>,
        "Agarwal, Nikhil" <nikhil.agarwal@....com>,
        "Gupta, Nipun" <Nipun.Gupta@....com>
Subject: Re: [PATCH v2 1/4] cdx: Introduce lock to protect controller ops and
 controller list

On Tue, Aug 01, 2023 at 08:34:00AM +0000, Gangurde, Abhijit wrote:
> > From: Greg KH <gregkh@...uxfoundation.org>
> > Sent: Monday, July 31, 2023 5:55 PM
> > To: Gangurde, Abhijit <abhijit.gangurde@....com>
> > Cc: masahiroy@...nel.org; linux-kernel@...r.kernel.org; Simek, Michal
> > <michal.simek@....com>; git (AMD-Xilinx) <git@....com>; Agarwal, Nikhil
> > <nikhil.agarwal@....com>; Gupta, Nipun <Nipun.Gupta@....com>
> > Subject: Re: [PATCH v2 1/4] cdx: Introduce lock to protect controller ops and
> > controller list
> > 
> > On Mon, Jul 31, 2023 at 05:38:10PM +0530, Abhijit Gangurde wrote:
> > > Add a mutex lock to prevent race between controller ops initiated by
> > > the bus subsystem and the controller registration/unregistration.
> > >
> > > Signed-off-by: Abhijit Gangurde <abhijit.gangurde@....com>
> > > ---
> > >  drivers/cdx/cdx.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
> > > index d2cad4c670a0..66797c8fe400 100644
> > > --- a/drivers/cdx/cdx.c
> > > +++ b/drivers/cdx/cdx.c
> > > @@ -72,6 +72,8 @@
> > >
> > >  /* CDX controllers registered with the CDX bus */
> > >  static DEFINE_XARRAY_ALLOC(cdx_controllers);
> > > +/* Lock to protect controller ops and controller list */
> > > +static DEFINE_MUTEX(cdx_controller_lock);
> > 
> > Wait, why do you have a local list and not just rely on the list the
> > driver core has for you already?  Isn't this a duplicate list where you
> > have objects on two different lists with a lifespan controlled only by
> > one of them?
> 
> cdx_controllers list is holding just the controllers registered on the cdx bus system.

Which are devices on the bus, so why do you need a separate list?

> CDX devices are still maintained by driver core list. Controller list is used by rescan
> which triggers rescan on all the controllers.

Again, why a separate list?  The driver core already tracks these,
right?

thanks,

greg k-h