linux-kernel - Re: [PATCH] usb: ulpi: don't register drivers if bus doesn't exist

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]

Date:	Wed, 27 May 2015 09:49:01 -0700
From:	David Cohen <david.a.cohen@...ux.intel.com>
To:	Greg KH <gregkh@...uxfoundation.org>
Cc:	Bjørn Mork <bjorn@...k.no>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Tal Shorer <tal.shorer@...il.com>,
	Sudip Mukherjee <sudipm.mukherjee@...il.com>,
	Sasha Levin <sasha.levin@...cle.com>,
	USB list <linux-usb@...r.kernel.org>,
	"<linux-kernel@...r.kernel.org>" <linux-kernel@...r.kernel.org>,
	Felipe Balbi <balbi@...com>,
	Lu Baolu <baolu.lu@...ux.intel.com>
Subject: Re: [PATCH] usb: ulpi: don't register drivers if bus doesn't exist

Hi Greg,

On Tue, May 26, 2015 at 07:41:18PM -0700, Greg KH wrote:
> On Tue, May 26, 2015 at 10:54:01AM -0700, David Cohen wrote:
> > Hi,
> > 
> > On Mon, May 25, 2015 at 07:00:13PM +0200, Bjørn Mork wrote:
> > > Greg KH <gregkh@...uxfoundation.org> writes:
> > > 
> > > > If there are other bus drivers that do this, I'll go fix them up,
> > > > pointers to those files would be appreciated.
> > > 
> > > git grep -E 'if .*\.p\W' found a couple of interesting candidates you
> > > might want to check out:
> > > 
> > >  drivers/i2c/i2c-core.c: if (unlikely(WARN_ON(!i2c_bus_type.p))) {
> > >  drivers/i2c/i2c-core.c: if (unlikely(WARN_ON(!i2c_bus_type.p)))
> > >  drivers/spmi/spmi.c:    if (WARN_ON(!spmi_bus_type.p))
> > > 
> > > And this looks somewhat suspicious too, but could very well be OK for
> > > all I know (very close to nothing :)
> > > 
> > >  drivers/gpio/gpiolib-sysfs.c:   if (!gpio_class.p) {
> > >  drivers/gpio/gpiolib-sysfs.c:   if (!gpio_class.p)
> > 
> > I think we need a clear statement on how to proceed on this case.
> 
> Don't mess with bus->p.  I can rename it to
> "do_not_touch_this_isnt_for_you" if people think that would make it more
> obvious that a private data structure shouldn't be messed with in any
> way.  Outside of the driver core, you have no knowledge that even if it
> is a pointer, what that means with regards to anything.

I get that, I agree it's ugly and I'm not trying to push it further. If
you follow the thread, you'll see I reviewed and commented a macro would
make more sense than checking the private data directly for the same
reaon you mentioned. But since with Linux development the source code is
part of the documentation, we need a clear state about what's the
correct way to handle it before go telling people "do not do what kernel
is already doing because it's wrong".

Br, David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/