From: Kuen-Han Tsai khtsai@google.com
[ Upstream commit 42988380ac67c76bb9dff8f77d7ef3eefd50b7b5 ]
After an bind/unbind cycle, the ecm->notify_req is left stale. If a subsequent bind fails, the unified error label attempts to free this stale request, leading to a NULL pointer dereference when accessing ep->ops->free_request.
Refactor the error handling in the bind path to use the __free() automatic cleanup mechanism.
Fixes: da741b8c56d6 ("usb ethernet gadget: split CDC Ethernet function") Cc: stable@kernel.org Signed-off-by: Kuen-Han Tsai khtsai@google.com Link: https://lore.kernel.org/r/20250916-ready-v1-5-4997bf277548@google.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Link: https://lore.kernel.org/r/20250916-ready-v1-5-4997bf277548@google.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/usb/gadget/function/f_ecm.c | 48 ++++++++++++----------------- 1 file changed, 20 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c index ffe2486fce71c..35ec50685b499 100644 --- a/drivers/usb/gadget/function/f_ecm.c +++ b/drivers/usb/gadget/function/f_ecm.c @@ -8,12 +8,15 @@
/* #define VERBOSE_DEBUG */
+#include <linux/cleanup.h> #include <linux/slab.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/device.h> #include <linux/etherdevice.h>
+#include <linux/usb/gadget.h> + #include "u_ether.h" #include "u_ether_configfs.h" #include "u_ecm.h" @@ -689,6 +692,7 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f) struct usb_ep *ep;
struct f_ecm_opts *ecm_opts; + struct usb_request *request __free(free_usb_request) = NULL;
if (!can_support_ecm(cdev->gadget)) return -EINVAL; @@ -726,7 +730,7 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f) /* allocate instance-specific interface IDs */ status = usb_interface_id(c, f); if (status < 0) - goto fail; + return status; ecm->ctrl_id = status; ecm_iad_descriptor.bFirstInterface = status;
@@ -735,24 +739,22 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
status = usb_interface_id(c, f); if (status < 0) - goto fail; + return status; ecm->data_id = status;
ecm_data_nop_intf.bInterfaceNumber = status; ecm_data_intf.bInterfaceNumber = status; ecm_union_desc.bSlaveInterface0 = status;
- status = -ENODEV; - /* allocate instance-specific endpoints */ ep = usb_ep_autoconfig(cdev->gadget, &fs_ecm_in_desc); if (!ep) - goto fail; + return -ENODEV; ecm->port.in_ep = ep;
ep = usb_ep_autoconfig(cdev->gadget, &fs_ecm_out_desc); if (!ep) - goto fail; + return -ENODEV; ecm->port.out_ep = ep;
/* NOTE: a status/notification endpoint is *OPTIONAL* but we @@ -761,20 +763,18 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f) */ ep = usb_ep_autoconfig(cdev->gadget, &fs_ecm_notify_desc); if (!ep) - goto fail; + return -ENODEV; ecm->notify = ep;
- status = -ENOMEM; - /* allocate notification request and buffer */ - ecm->notify_req = usb_ep_alloc_request(ep, GFP_KERNEL); - if (!ecm->notify_req) - goto fail; - ecm->notify_req->buf = kmalloc(ECM_STATUS_BYTECOUNT, GFP_KERNEL); - if (!ecm->notify_req->buf) - goto fail; - ecm->notify_req->context = ecm; - ecm->notify_req->complete = ecm_notify_complete; + request = usb_ep_alloc_request(ep, GFP_KERNEL); + if (!request) + return -ENOMEM; + request->buf = kmalloc(ECM_STATUS_BYTECOUNT, GFP_KERNEL); + if (!request->buf) + return -ENOMEM; + request->context = ecm; + request->complete = ecm_notify_complete;
/* support all relevant hardware speeds... we expect that when * hardware is dual speed, all bulk-capable endpoints work at @@ -793,7 +793,7 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f) status = usb_assign_descriptors(f, ecm_fs_function, ecm_hs_function, ecm_ss_function, ecm_ss_function); if (status) - goto fail; + return status;
/* NOTE: all that is done without knowing or caring about * the network link ... which is unavailable to this code @@ -803,22 +803,14 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f) ecm->port.open = ecm_open; ecm->port.close = ecm_close;
+ ecm->notify_req = no_free_ptr(request); + DBG(cdev, "CDC Ethernet: %s speed IN/%s OUT/%s NOTIFY/%s\n", gadget_is_superspeed(c->cdev->gadget) ? "super" : gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full", ecm->port.in_ep->name, ecm->port.out_ep->name, ecm->notify->name); return 0; - -fail: - if (ecm->notify_req) { - kfree(ecm->notify_req->buf); - usb_ep_free_request(ecm->notify, ecm->notify_req); - } - - ERROR(cdev, "%s: can't bind, err %d\n", f->name, status); - - return status; }
static inline struct f_ecm_opts *to_f_ecm_opts(struct config_item *item)