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

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

[Sergey Shtylyov]

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

[PATCH v3 1/2] usb: host: ohci-tmio: add IRQ check

[Sergey Shtylyov]

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>

---
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;

Re: [PATCH v3 1/2] usb: host: ohci-tmio: add IRQ check

[Alan Stern]

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;

[PATCH v3 2/2] usb: phy: tahvo: add IRQ check

[Sergey Shtylyov]

The driver neglects to check the result of platform_get_irq()'s call and
blithely passes the negative error codes to request_threaded_irq() (which
takes *unsigned* IRQ #), causing it to fail with -EINVAL, overriding an
original error code.  Stop calling request_threaded_irq() with the invalid
IRQ #s.

Fixes: 9ba96ae5074c ("usb: omap1: Tahvo USB transceiver driver")
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

---
 drivers/usb/phy/phy-tahvo.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Index: usb/drivers/usb/phy/phy-tahvo.c
===================================================================
--- usb.orig/drivers/usb/phy/phy-tahvo.c
+++ usb/drivers/usb/phy/phy-tahvo.c
@@ -393,7 +393,9 @@ static int tahvo_usb_probe(struct platfo
 
 	dev_set_drvdata(&pdev->dev, tu);
 
-	tu->irq = platform_get_irq(pdev, 0);
+	tu->irq = ret = platform_get_irq(pdev, 0);
+	if (ret < 0)
+		return ret;
 	ret = request_threaded_irq(tu->irq, NULL, tahvo_usb_vbus_interrupt,
 				   IRQF_ONESHOT,
 				   "tahvo-vbus", tu);

Re: [PATCH v3 2/2] usb: phy: tahvo: add IRQ check

[Felipe Balbi]

Sergey Shtylyov <s.shtylyov@omp.ru> writes:

> The driver neglects to check the result of platform_get_irq()'s call and
> blithely passes the negative error codes to request_threaded_irq() (which
> takes *unsigned* IRQ #), causing it to fail with -EINVAL, overriding an
> original error code.  Stop calling request_threaded_irq() with the invalid
> IRQ #s.
>
> Fixes: 9ba96ae5074c ("usb: omap1: Tahvo USB transceiver driver")
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

Acked-by: Felipe Balbi <balbi@kernel.org>

-- 
balbi

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

[Greg Kroah-Hartman]

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...

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

[Sergey Shtylyov]

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

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

[Greg Kroah-Hartman]

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

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

[Sergey Shtylyov]

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