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]

Message-ID: <20150528132458.GA12315@kuha.fi.intel.com>
Date:	Thu, 28 May 2015 16:24:58 +0300
From:	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:	Sasha Levin <sasha.levin@...cle.com>
Cc:	Sudip Mukherjee <sudipm.mukherjee@...il.com>,
	Greg KH <gregkh@...uxfoundation.org>,
	Alan Stern <stern@...land.harvard.edu>,
	Tal Shorer <tal.shorer@...il.com>,
	USB list <linux-usb@...r.kernel.org>,
	"<linux-kernel@...r.kernel.org>" <linux-kernel@...r.kernel.org>,
	David Cohen <david.a.cohen@...ux.intel.com>,
	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

On Thu, May 28, 2015 at 08:36:46AM -0400, Sasha Levin wrote:
> On 05/28/2015 01:39 AM, Sudip Mukherjee wrote:
> > diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> > index 4eabfe2..1acae5b 100644
> > --- a/drivers/base/driver.c
> > +++ b/drivers/base/driver.c
> > @@ -150,6 +150,11 @@ int driver_register(struct device_driver *drv)
> >  	int ret;
> >  	struct device_driver *other;
> >  
> > +	if (!drv->bus->registered) {
> > +		pr_err("Driver %s registration failed. bus not yet registered\n",
> > +		       drv->name);
> > +		return -ENODEV;
> > +	}
> >  	BUG_ON(!drv->bus->p);
> 
> This is a design issue with the code in the layer above, there's no reason
> driver_register() should be called with a bus that wasn't registered to
> begin with.
> 
> This is why there's a BUG_ON there to catch these issues - it's a bug, not
> a desired behaviour.

Unfortunately problems with the design are not the only cases why we
could end up here before the bus has been registered. If the bus has
failed to register, we definitely should not trigger a BUG here. The
bus management driver has in that case already made the decision to
not BUG. Or if the user is allowed to disable a bus somehow, for
example with something like nousb parameter, but we still manage do
get here, we should again not trigger BUG().

I don't think BUG_ON here is ever the correct thing to do. This
function can see that the bus doesn't exist or possibly that something
has gone wrong by checking the "p", but it does not know any details,
nor should it. This function should trigger a warning in those cases
and return failure, and not make any extra decisions.

Thanks,

-- 
heikki
--
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/