linux-kernel - Re: [PATCH] usb: gadget: core: Check for unset descriptor

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: <29bc21ae-1f8a-47fd-b361-c761564f483a@rowland.harvard.edu>
Date: Sun, 21 Jul 2024 21:07:21 -0400
From: Alan Stern <stern@...land.harvard.edu>
To: crwulff@...il.com
Cc: linux-usb@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Roy Luo <royluo@...gle.com>,
	Krishna Kurapati <quic_kriskura@...cinc.com>,
	Michael Grzeschik <m.grzeschik@...gutronix.de>,
	yuan linyu <yuanlinyu@...onor.com>,
	Paul Cercueil <paul@...pouillou.net>,
	Felipe Balbi <balbi@...nel.org>, linux-kernel@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH] usb: gadget: core: Check for unset descriptor

On Sun, Jul 21, 2024 at 03:20:49PM -0400, crwulff@...il.com wrote:
> From: Chris Wulff <crwulff@...il.com>
> 
> Make sure the descriptor has been set before looking at maxpacket.
> This fixes a null pointer panic in this case.
> 
> This may happen if the gadget doesn't properly set up the endpoint
> for the current speed, or the gadget descriptors are malformed and
> the descriptor for the speed/endpoint are not found.

If that happens, doesn't it mean there's a bug in the gadget driver?  
And if there's a bug, don't we want to be told about it by a big 
impossible-to-miss error message, so the bug can be fixed?

> Fixes: 54f83b8c8ea9 ("USB: gadget: Reject endpoints with 0 maxpacket value")
> Cc: stable@...r.kernel.org
> Signed-off-by: Chris Wulff <crwulff@...il.com>
> ---
>  drivers/usb/gadget/udc/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index 2dfae7a17b3f..36a5d5935889 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -118,7 +118,7 @@ int usb_ep_enable(struct usb_ep *ep)
>  		goto out;
>  
>  	/* UDC drivers can't handle endpoints with maxpacket size 0 */
> -	if (usb_endpoint_maxp(ep->desc) == 0) {
> +	if (!ep->desc || usb_endpoint_maxp(ep->desc) == 0) {
>  		/*
>  		 * We should log an error message here, but we can't call
>  		 * dev_err() because there's no way to find the gadget

This will just hide the error.  That's not good.

Alan Stern