From: Chris Wulff crwulff@gmail.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.
No current gadget driver is known to have this problem, but this may cause a hard-to-find bug during development of new gadgets.
Fixes: 54f83b8c8ea9 ("USB: gadget: Reject endpoints with 0 maxpacket value") Cc: stable@vger.kernel.org Signed-off-by: Chris Wulff crwulff@gmail.com --- v2: Added WARN_ONCE message & clarification on causes v1: https://lore.kernel.org/all/20240721192048.3530097-2-crwulff@gmail.com/ --- drivers/usb/gadget/udc/core.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index 2dfae7a17b3f..81f9140f3681 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -118,12 +118,10 @@ 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) { - /* - * We should log an error message here, but we can't call - * dev_err() because there's no way to find the gadget - * given only ep. - */ + if (!ep->desc || usb_endpoint_maxp(ep->desc) == 0) { + WARN_ONCE(1, "%s: ep%d (%s) has %s\n", __func__, ep->address, ep->name, + (!ep->desc) ? "NULL descriptor" : "maxpacket 0"); + ret = -EINVAL; goto out; }
On Wed, Jul 24, 2024 at 09:04:20PM -0400, crwulff@gmail.com wrote:
From: Chris Wulff crwulff@gmail.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.
No current gadget driver is known to have this problem, but this may cause a hard-to-find bug during development of new gadgets.
Fixes: 54f83b8c8ea9 ("USB: gadget: Reject endpoints with 0 maxpacket value") Cc: stable@vger.kernel.org Signed-off-by: Chris Wulff crwulff@gmail.com
v2: Added WARN_ONCE message & clarification on causes v1: https://lore.kernel.org/all/20240721192048.3530097-2-crwulff@gmail.com/
drivers/usb/gadget/udc/core.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index 2dfae7a17b3f..81f9140f3681 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -118,12 +118,10 @@ 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) {
/** We should log an error message here, but we can't call* dev_err() because there's no way to find the gadget* given only ep.*/
- if (!ep->desc || usb_endpoint_maxp(ep->desc) == 0) {
WARN_ONCE(1, "%s: ep%d (%s) has %s\n", __func__, ep->address, ep->name,(!ep->desc) ? "NULL descriptor" : "maxpacket 0");
So you just rebooted a machine that hit this, that's not good at all. Please log the error and recover, don't crash a system (remember, panic-on-warn is enabled in billions of Linux systems.)
thanks,
greg k-h
On Thu, Jul 25, 2024 at 06:56:19AM +0200, Greg Kroah-Hartman wrote:
On Wed, Jul 24, 2024 at 09:04:20PM -0400, crwulff@gmail.com wrote:
From: Chris Wulff crwulff@gmail.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.
No current gadget driver is known to have this problem, but this may cause a hard-to-find bug during development of new gadgets.
Fixes: 54f83b8c8ea9 ("USB: gadget: Reject endpoints with 0 maxpacket value") Cc: stable@vger.kernel.org Signed-off-by: Chris Wulff crwulff@gmail.com
v2: Added WARN_ONCE message & clarification on causes v1: https://lore.kernel.org/all/20240721192048.3530097-2-crwulff@gmail.com/
drivers/usb/gadget/udc/core.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index 2dfae7a17b3f..81f9140f3681 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -118,12 +118,10 @@ 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) {
/** We should log an error message here, but we can't call* dev_err() because there's no way to find the gadget* given only ep.*/
- if (!ep->desc || usb_endpoint_maxp(ep->desc) == 0) {
WARN_ONCE(1, "%s: ep%d (%s) has %s\n", __func__, ep->address, ep->name,(!ep->desc) ? "NULL descriptor" : "maxpacket 0");So you just rebooted a machine that hit this, that's not good at all. Please log the error and recover, don't crash a system (remember, panic-on-warn is enabled in billions of Linux systems.)
That should not be a problem. This WARN_ONCE is expected never to be triggered except by a buggy gadget driver. It's a debugging tool; the developer will get an indication in the kernel log of where the problem is instead of just a panic.
However, if you feel strongly about this, Chris probably won't mind changing it to pr_err() plus dump_stack() instead of WARN_ONCE().
Alan Stern
On Thu, Jul 25, 2024 at 02:23:25PM -0400, Alan Stern wrote:
On Thu, Jul 25, 2024 at 06:56:19AM +0200, Greg Kroah-Hartman wrote:
On Wed, Jul 24, 2024 at 09:04:20PM -0400, crwulff@gmail.com wrote:
From: Chris Wulff crwulff@gmail.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.
No current gadget driver is known to have this problem, but this may cause a hard-to-find bug during development of new gadgets.
Fixes: 54f83b8c8ea9 ("USB: gadget: Reject endpoints with 0 maxpacket value") Cc: stable@vger.kernel.org Signed-off-by: Chris Wulff crwulff@gmail.com
v2: Added WARN_ONCE message & clarification on causes v1: https://lore.kernel.org/all/20240721192048.3530097-2-crwulff@gmail.com/
drivers/usb/gadget/udc/core.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index 2dfae7a17b3f..81f9140f3681 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -118,12 +118,10 @@ 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) {
/** We should log an error message here, but we can't call* dev_err() because there's no way to find the gadget* given only ep.*/
- if (!ep->desc || usb_endpoint_maxp(ep->desc) == 0) {
WARN_ONCE(1, "%s: ep%d (%s) has %s\n", __func__, ep->address, ep->name,(!ep->desc) ? "NULL descriptor" : "maxpacket 0");So you just rebooted a machine that hit this, that's not good at all. Please log the error and recover, don't crash a system (remember, panic-on-warn is enabled in billions of Linux systems.)
That should not be a problem. This WARN_ONCE is expected never to be triggered except by a buggy gadget driver. It's a debugging tool; the developer will get an indication in the kernel log of where the problem is instead of just a panic.
Ok, if this can never be hit by a user action, then it's ok to leave as-is, it wasn't obvious to me that this is the case, sorry. I'll queue this up after -rc1 is out.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Alan Stern -
crwulff＠gmail.com -
Greg Kroah-Hartman