linux-kernel - Re: [PATCH] parport: register driver later

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] [day] [month] [year] [list]

Message-ID: <20160307011225.GA6588@kroah.com>
Date:	Sun, 6 Mar 2016 17:12:25 -0800
From:	Greg KH <gregkh@...uxfoundation.org>
To:	Sudip Mukherjee <sudipm.mukherjee@...il.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] parport: register driver later

On Sun, Mar 06, 2016 at 11:26:29PM +0530, Sudip Mukherjee wrote:
> On Sat, Mar 05, 2016 at 12:19:32PM -0800, Greg KH wrote:
> > On Fri, Mar 04, 2016 at 04:20:59PM +0530, Sudip Mukherjee wrote:
> > > If the parport bus is not yet registered and any device using parallel
> > > port tries to register with the bus we get a stackdump with a message
> > > of Kernel bug.
> > > 
> > > Reported-by: Fengguang Wu <fengguang.wu@...el.com>
> > > Tested-by: Ross Zwisler <ross.zwisler@...ux.intel.com>
> > > Cc: <stable@...r.kernel.org> # 4.2+ 
> > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@...ethink.co.uk>
> > > ---
> > > 
> > > We should actually have some deferred probe here. But considering that
> > > you will be closing your trees soon so a quick fix to solve the problem
> > > for now. We will revisit this when we remove the old api (hopefully v4.7).
> > > 
> > >  drivers/parport/share.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/parport/share.c b/drivers/parport/share.c
> > > index 3308427..176b2b6 100644
> > > --- a/drivers/parport/share.c
> > > +++ b/drivers/parport/share.c
> > > @@ -273,6 +273,9 @@ int __parport_register_driver(struct parport_driver *drv, struct module *owner,
> > >  		/* using device model */
> > >  		int ret;
> > >  
> > > +		if (!parport_bus_type.p)
> > > +			return -EAGAIN;
> > 
> > I really don't like it when busses poke into the driver-core
> > internal-only structures like this.  Why can't you have your own "have
> > been registered" flag instead if you really need it?  Don't rely on the
> > driver core here to be doing this always this way, perhaps p could be
> > NULL and it only is created later on somehow?
> 
> I saw that in i2c and spmi and followed. Sent you v2 for your review. I
> will send a patch to remove the use of 'p' in those places.

Thanks, that would be great.

> > I need to rename 'p' to "do_not_touch_you_have_been_warned" or something
> > else...
> 
> something like this (compile tested) ?
> (do you want me to send a proper patch?):

Heh, no, it was kind of a joke, there's probably a better way to do it
without making the driver core look like a mess...

thanks,

greg k-h