summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKuen-Han Tsai <khtsai@google.com>2025-09-16 16:21:36 +0800
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2025-09-17 12:44:53 +0200
commit42988380ac67c76bb9dff8f77d7ef3eefd50b7b5 (patch)
tree7a2d5a4d6f8937299e5345c58486702b3ad42c76
parent47b2116e54b4a854600341487e8b55249e926324 (diff)
usb: gadget: f_ecm: Refactor bind path to use __free()
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
-rw-r--r--drivers/usb/gadget/function/f_ecm.c48
1 files changed, 20 insertions, 28 deletions
diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index 027226325039..675d2bc538a4 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -8,6 +8,7 @@
/* #define VERBOSE_DEBUG */
+#include <linux/cleanup.h>
#include <linux/slab.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -15,6 +16,8 @@
#include <linux/etherdevice.h>
#include <linux/string_choices.h>
+#include <linux/usb/gadget.h>
+
#include "u_ether.h"
#include "u_ether_configfs.h"
#include "u_ecm.h"
@@ -678,6 +681,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;
@@ -711,7 +715,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;
@@ -720,24 +724,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
@@ -746,20 +748,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
@@ -778,7 +778,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
@@ -788,20 +788,12 @@ 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: IN/%s OUT/%s NOTIFY/%s\n",
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)