mbox series

[v3,0/2] Stop calling request_irq(), etc. with invalid IRQs in the USB drivers`

Message ID fb92857f-3120-9a20-65ba-f21aeb4b9020@omp.ru
Headers show
Series Stop calling request_irq(), etc. with invalid IRQs in the USB drivers` | expand

Message

Sergey Shtylyov Aug. 13, 2021, 8:23 p.m. UTC 
Here are 2 patches against the 'usb-linus' branch of GregKH's 'usb.git' repo.
The affected drivers call platform_get_irq() but largely ignore its result --
they blithely pass the negative error codes to request_irq() (and its ilk)
which expects *unsinged* IRQ #s. Stop doing that by checking what exactly
platform_get_irq() returns.

I've removed 7 patches that GregKH has applied to the usb-testing' branch
(holler if this isn't enough).

[1/2] usb: host: ohci-tmio: add IRQ check
[2/2] usb: phy: tahvo: add IRQ check

Comments

Alan Stern Aug. 14, 2021, 1:45 a.m. UTC | #1 
On Fri, Aug 13, 2021 at 11:30:18PM +0300, Sergey Shtylyov wrote:
> The driver neglects to check the  result of platform_get_irq()'s call and

> blithely passes the negative error codes to usb_add_hcd() (which takes

> *unsigned* IRQ #), causing request_irq() that it calls to fail with

> -EINVAL, overriding an original error code. Stop calling usb_add_hcd()

> with the invalid IRQ #s.

> 

> Fixes: 78c73414f4f6 ("USB: ohci: add support for tmio-ohci cell")

> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

> 

> ---


Acked-by: Alan Stern <stern@rowland.harvard.edu>


> Changes in version 3:

> - move the IRQ check higher in ohci_hcd_tmio_drv_probe(), to be closer to

>   platfrom_get_irq()'s call.

> 

>  drivers/usb/host/ohci-tmio.c |    3 +++

>  1 file changed, 3 insertions(+)

> 

> Index: usb/drivers/usb/host/ohci-tmio.c

> ===================================================================

> --- usb.orig/drivers/usb/host/ohci-tmio.c

> +++ usb/drivers/usb/host/ohci-tmio.c

> @@ -202,6 +202,9 @@ static int ohci_hcd_tmio_drv_probe(struc

>  	if (!cell)

>  		return -EINVAL;

>  

> +	if (irq < 0)

> +		return irq;

> +

>  	hcd = usb_create_hcd(&ohci_tmio_hc_driver, &dev->dev, dev_name(&dev->dev));

>  	if (!hcd) {

>  		ret = -ENOMEM;
Greg Kroah-Hartman Aug. 14, 2021, 6:39 a.m. UTC | #2 
On Fri, Aug 13, 2021 at 11:23:43PM +0300, Sergey Shtylyov wrote:
> Here are 2 patches against the 'usb-linus' branch of GregKH's 'usb.git' repo.


Wait, why that branch?  Please make them against the branch you want
them applied to.  Hopefully they will apply to the usb-next branch...
Sergey Shtylyov Aug. 14, 2021, 11:59 a.m. UTC | #3 
On 14.08.2021 9:39, Greg Kroah-Hartman wrote:

>> Here are 2 patches against the 'usb-linus' branch of GregKH's 'usb.git' repo.

> 

> Wait, why that branch?


    What branch I'd use for the fixes then?

>  Please make them against the branch you want

> them applied to.  Hopefully they will apply to the usb-next branch...


    I didn't intend them for usb-next but looks like they apply there too.

MBR, Sergey
Greg Kroah-Hartman Aug. 14, 2021, 1:34 p.m. UTC | #4 
On Sat, Aug 14, 2021 at 02:59:09PM +0300, Sergey Shtylyov wrote:
> On 14.08.2021 9:39, Greg Kroah-Hartman wrote:

> 

> > > Here are 2 patches against the 'usb-linus' branch of GregKH's 'usb.git' repo.

> > 

> > Wait, why that branch?

> 

>    What branch I'd use for the fixes then?


Ah, you really want this in for 5.14-final?  People are hitting this
issue now?

> >  Please make them against the branch you want

> > them applied to.  Hopefully they will apply to the usb-next branch...

> 

>    I didn't intend them for usb-next but looks like they apply there too.


I think it belongs there as a "nice cleanup to have", right?

thanks,

greg k-h
Sergey Shtylyov Aug. 14, 2021, 6:16 p.m. UTC | #5 
On 8/14/21 4:34 PM, Greg Kroah-Hartman wrote:

>>>> Here are 2 patches against the 'usb-linus' branch of GregKH's 'usb.git' repo.

>>>

>>> Wait, why that branch?

>>

>>    What branch I'd use for the fixes then?

> 

> Ah, you really want this in for 5.14-final?


   Not necessarily, it's your call. But all the patches are fixes.

> People are hitting this issue now?


   No, the patches ware all the result of the code reviews...

>>>  Please make them against the branch you want

>>> them applied to.  Hopefully they will apply to the usb-next branch...

>>

>>    I didn't intend them for usb-next but looks like they apply there too.

> 

> I think it belongs there as a "nice cleanup to have", right?


   No, they're definitely not cleanups and all have the "Fixes:" tags, so going
to end up in the stable trees (some already have).
   There's going to be 10-patch series soon, all fixing the deferred probing due
to platfrorm_get_irq()...
   Luckily, the USB tree doesn't shave the 3rd kind of platfrorm_get_irq() bugs:
treating 0 as error and returning it immediately along with the negative values,
without doing the remaining part of probe...

> thanks,

> 

> greg k-h


MBR, Sergey