[PATCH 1/1] usb: ulpi: don't register drivers if bus doesn't exist

[PATCH 1/1] usb: ulpi: don't register drivers if bus doesn't exist

[Lu Baolu]

ULPI registers its bus at module_init, so if the bus fails to register, the
module will fail to load and all will be well in the world.

However, if the ULPI code is built-in rather than a module, the bus
initialization may fail, but we'd still try to register drivers later onto
a non-existent bus, which will panic the kernel.

Fix that by checking that the bus was indeed initialized before trying to
register drivers on top of it.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reported-by: Zhuo Qiuxu <qiuxu.zhuo@intel.com>
Reviewed-by: David Cohen <david.a.cohen@linux.intel.com>
---
 drivers/usb/common/ulpi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..af52b46 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv)
 	if (!drv->probe)
 		return -EINVAL;
 
+	/* Was the bus registered successfully? */
+	if (unlikely(WARN_ON(!ulpi_bus.p)))
+		return -ENODEV;
+
 	drv->driver.bus = &ulpi_bus;
 
 	return driver_register(&drv->driver);
@@ -242,7 +246,7 @@ static int __init ulpi_init(void)
 {
 	return bus_register(&ulpi_bus);
 }
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);
 
 static void __exit ulpi_exit(void)
 {
-- 
2.1.4

Re: [PATCH 1/1] usb: ulpi: don't register drivers if bus doesn't exist

[Lu, Baolu]

Hi Heikki, Sasha and others,

Please ignore this patch. I should not squash these two patches into one and
sign it off behalf of other people. I apologize for this and I'm sorry 
if this lets
you feel affronted.

Thanks,
Baolu


On 05/21/2015 04:57 PM, Lu Baolu wrote:
> ULPI registers its bus at module_init, so if the bus fails to register, the
> module will fail to load and all will be well in the world.
>
> However, if the ULPI code is built-in rather than a module, the bus
> initialization may fail, but we'd still try to register drivers later onto
> a non-existent bus, which will panic the kernel.
>
> Fix that by checking that the bus was indeed initialized before trying to
> register drivers on top of it.
>
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reported-by: Zhuo Qiuxu <qiuxu.zhuo@intel.com>
> Reviewed-by: David Cohen <david.a.cohen@linux.intel.com>
> ---
>   drivers/usb/common/ulpi.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> index 0e6f968..af52b46 100644
> --- a/drivers/usb/common/ulpi.c
> +++ b/drivers/usb/common/ulpi.c
> @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv)
>   	if (!drv->probe)
>   		return -EINVAL;
>   
> +	/* Was the bus registered successfully? */
> +	if (unlikely(WARN_ON(!ulpi_bus.p)))
> +		return -ENODEV;
> +
>   	drv->driver.bus = &ulpi_bus;
>   
>   	return driver_register(&drv->driver);
> @@ -242,7 +246,7 @@ static int __init ulpi_init(void)
>   {
>   	return bus_register(&ulpi_bus);
>   }
> -module_init(ulpi_init);
> +subsys_initcall(ulpi_init);
>   
>   static void __exit ulpi_exit(void)
>   {

Re: [PATCH 1/1] usb: ulpi: don't register drivers if bus doesn't exist

[Heikki Krogerus]

On Thu, May 21, 2015 at 06:07:25PM +0800, Lu, Baolu wrote:
> Hi Heikki, Sasha and others,
> 
> Please ignore this patch. I should not squash these two patches into one and
> sign it off behalf of other people. I apologize for this and I'm sorry if
> this lets
> you feel affronted.

No worries. So we'll use patch from Sasha to fix the panic when the
bus is missing, and your's to fix the execution order.

If you prepare v2 of your original patch, I'll ACK it.


Thanks,

-- 
heikki

Re: [PATCH 1/1] usb: ulpi: don't register drivers if bus doesn't exist

[Greg Kroah-Hartman]

On Thu, May 21, 2015 at 04:57:52PM +0800, Lu Baolu wrote:
> ULPI registers its bus at module_init, so if the bus fails to register, the
> module will fail to load and all will be well in the world.
> 
> However, if the ULPI code is built-in rather than a module, the bus
> initialization may fail, but we'd still try to register drivers later onto
> a non-existent bus, which will panic the kernel.
> 
> Fix that by checking that the bus was indeed initialized before trying to
> register drivers on top of it.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reported-by: Zhuo Qiuxu <qiuxu.zhuo@intel.com>
> Reviewed-by: David Cohen <david.a.cohen@linux.intel.com>
> ---
>  drivers/usb/common/ulpi.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> index 0e6f968..af52b46 100644
> --- a/drivers/usb/common/ulpi.c
> +++ b/drivers/usb/common/ulpi.c
> @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv)
>  	if (!drv->probe)
>  		return -EINVAL;
>  
> +	/* Was the bus registered successfully? */
> +	if (unlikely(WARN_ON(!ulpi_bus.p)))

never use unlikely/likely unless you can actually measure the difference
if the marking is not used.

Hint, on driver probe time, you can't.

thanks,

greg k-h