The patch below does not apply to the 6.17-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.17.y git checkout FETCH_HEAD git cherry-pick -x 75a5b8d4ddd4eb6b16cb0b475d14ff4ae64295ef # <resolve conflicts, build, test, etc.> git commit -s git send-email --to 'stable@vger.kernel.org' --in-reply-to '2025101601-custard-enchilada-decc@gregkh' --subject-prefix 'PATCH 6.17.y' HEAD^..
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 75a5b8d4ddd4eb6b16cb0b475d14ff4ae64295ef Mon Sep 17 00:00:00 2001 From: Kuen-Han Tsai khtsai@google.com Date: Tue, 16 Sep 2025 16:21:34 +0800 Subject: [PATCH] usb: gadget: f_ncm: Refactor bind path to use __free()
After an bind/unbind cycle, the ncm->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.
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000020 Call trace: usb_ep_free_request+0x2c/0xec ncm_bind+0x39c/0x3dc usb_add_function+0xcc/0x1f0 configfs_composite_bind+0x468/0x588 gadget_bind_driver+0x104/0x270 really_probe+0x190/0x374 __driver_probe_device+0xa0/0x12c driver_probe_device+0x3c/0x218 __device_attach_driver+0x14c/0x188 bus_for_each_drv+0x10c/0x168 __device_attach+0xfc/0x198 device_initial_probe+0x14/0x24 bus_probe_device+0x94/0x11c device_add+0x268/0x48c usb_add_gadget+0x198/0x28c dwc3_gadget_init+0x700/0x858 __dwc3_set_mode+0x3cc/0x664 process_scheduled_works+0x1d8/0x488 worker_thread+0x244/0x334 kthread+0x114/0x1bc ret_from_fork+0x10/0x20
Fixes: 9f6ce4240a2b ("usb: gadget: f_ncm.c added") Cc: stable@kernel.org Signed-off-by: Kuen-Han Tsai khtsai@google.com Link: https://lore.kernel.org/r/20250916-ready-v1-3-4997bf277548@google.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Link: https://lore.kernel.org/r/20250916-ready-v1-3-4997bf277548@google.com
diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c index 58b0dd575af3..0148d60926dc 100644 --- a/drivers/usb/gadget/function/f_ncm.c +++ b/drivers/usb/gadget/function/f_ncm.c @@ -11,6 +11,7 @@ * Copyright (C) 2008 Nokia Corporation */
+#include <linux/cleanup.h> #include <linux/kernel.h> #include <linux/interrupt.h> #include <linux/module.h> @@ -20,6 +21,7 @@ #include <linux/string_choices.h>
#include <linux/usb/cdc.h> +#include <linux/usb/gadget.h>
#include "u_ether.h" #include "u_ether_configfs.h" @@ -1436,18 +1438,18 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f) struct usb_ep *ep; struct f_ncm_opts *ncm_opts;
+ struct usb_os_desc_table *os_desc_table __free(kfree) = NULL; + struct usb_request *request __free(free_usb_request) = NULL; + if (!can_support_ecm(cdev->gadget)) return -EINVAL;
ncm_opts = container_of(f->fi, struct f_ncm_opts, func_inst);
if (cdev->use_os_string) { - f->os_desc_table = kzalloc(sizeof(*f->os_desc_table), - GFP_KERNEL); - if (!f->os_desc_table) + os_desc_table = kzalloc(sizeof(*os_desc_table), GFP_KERNEL); + if (!os_desc_table) return -ENOMEM; - f->os_desc_n = 1; - f->os_desc_table[0].os_desc = &ncm_opts->ncm_os_desc; }
mutex_lock(&ncm_opts->lock); @@ -1459,16 +1461,15 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f) mutex_unlock(&ncm_opts->lock);
if (status) - goto fail; + return status;
ncm_opts->bound = true;
us = usb_gstrings_attach(cdev, ncm_strings, ARRAY_SIZE(ncm_string_defs)); - if (IS_ERR(us)) { - status = PTR_ERR(us); - goto fail; - } + if (IS_ERR(us)) + return PTR_ERR(us); + ncm_control_intf.iInterface = us[STRING_CTRL_IDX].id; ncm_data_nop_intf.iInterface = us[STRING_DATA_IDX].id; ncm_data_intf.iInterface = us[STRING_DATA_IDX].id; @@ -1478,20 +1479,16 @@ static int ncm_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; ncm->ctrl_id = status; ncm_iad_desc.bFirstInterface = status;
ncm_control_intf.bInterfaceNumber = status; ncm_union_desc.bMasterInterface0 = status;
- if (cdev->use_os_string) - f->os_desc_table[0].if_id = - ncm_iad_desc.bFirstInterface; - status = usb_interface_id(c, f); if (status < 0) - goto fail; + return status; ncm->data_id = status;
ncm_data_nop_intf.bInterfaceNumber = status; @@ -1500,35 +1497,31 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
ecm_desc.wMaxSegmentSize = cpu_to_le16(ncm_opts->max_segment_size);
- status = -ENODEV; - /* allocate instance-specific endpoints */ ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_in_desc); if (!ep) - goto fail; + return -ENODEV; ncm->port.in_ep = ep;
ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_out_desc); if (!ep) - goto fail; + return -ENODEV; ncm->port.out_ep = ep;
ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_notify_desc); if (!ep) - goto fail; + return -ENODEV; ncm->notify = ep;
- status = -ENOMEM; - /* allocate notification request and buffer */ - ncm->notify_req = usb_ep_alloc_request(ep, GFP_KERNEL); - if (!ncm->notify_req) - goto fail; - ncm->notify_req->buf = kmalloc(NCM_STATUS_BYTECOUNT, GFP_KERNEL); - if (!ncm->notify_req->buf) - goto fail; - ncm->notify_req->context = ncm; - ncm->notify_req->complete = ncm_notify_complete; + request = usb_ep_alloc_request(ep, GFP_KERNEL); + if (!request) + return -ENOMEM; + request->buf = kmalloc(NCM_STATUS_BYTECOUNT, GFP_KERNEL); + if (!request->buf) + return -ENOMEM; + request->context = ncm; + request->complete = ncm_notify_complete;
/* * support all relevant hardware speeds... we expect that when @@ -1548,7 +1541,7 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f) status = usb_assign_descriptors(f, ncm_fs_function, ncm_hs_function, ncm_ss_function, ncm_ss_function); if (status) - goto fail; + return status;
/* * NOTE: all that is done without knowing or caring about @@ -1561,23 +1554,18 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
hrtimer_setup(&ncm->task_timer, ncm_tx_timeout, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
+ if (cdev->use_os_string) { + os_desc_table[0].os_desc = &ncm_opts->ncm_os_desc; + os_desc_table[0].if_id = ncm_iad_desc.bFirstInterface; + f->os_desc_table = no_free_ptr(os_desc_table); + f->os_desc_n = 1; + } + ncm->notify_req = no_free_ptr(request); + DBG(cdev, "CDC Network: IN/%s OUT/%s NOTIFY/%s\n", ncm->port.in_ep->name, ncm->port.out_ep->name, ncm->notify->name); return 0; - -fail: - kfree(f->os_desc_table); - f->os_desc_n = 0; - - if (ncm->notify_req) { - kfree(ncm->notify_req->buf); - usb_ep_free_request(ncm->notify, ncm->notify_req); - } - - ERROR(cdev, "%s: can't bind, err %d\n", f->name, status); - - return status; }
static inline struct f_ncm_opts *to_f_ncm_opts(struct config_item *item)
From: Kuen-Han Tsai khtsai@google.com
[ Upstream commit bfb1d99d969fe3b892db30848aeebfa19d21f57f ]
Gadget function drivers often have goto-based error handling in their bind paths, which can be bug-prone. Refactoring these paths to use __free() scope-based cleanup is desirable, but currently blocked.
The blocker is that usb_ep_free_request(ep, req) requires two parameters, while the __free() mechanism can only pass a pointer to the request itself.
Store an endpoint pointer in the struct usb_request. The pointer is populated centrally in usb_ep_alloc_request() on every successful allocation, making the request object self-contained.
Signed-off-by: Kuen-Han Tsai khtsai@google.com Link: https://lore.kernel.org/r/20250916-ready-v1-1-4997bf277548@google.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Link: https://lore.kernel.org/r/20250916-ready-v1-1-4997bf277548@google.com Stable-dep-of: 75a5b8d4ddd4 ("usb: gadget: f_ncm: Refactor bind path to use __free()") Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/usb/gadget/udc/core.c | 3 +++ include/linux/usb/gadget.h | 2 ++ 2 files changed, 5 insertions(+)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index d709e24c1fd42..e3d63b8fa0f4c 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -194,6 +194,9 @@ struct usb_request *usb_ep_alloc_request(struct usb_ep *ep,
req = ep->ops->alloc_request(ep, gfp_flags);
+ if (req) + req->ep = ep; + trace_usb_ep_alloc_request(ep, req, req ? 0 : -ENOMEM);
return req; diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index 0f28c5512fcb6..0f20794760887 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -32,6 +32,7 @@ struct usb_ep;
/** * struct usb_request - describes one i/o request + * @ep: The associated endpoint set by usb_ep_alloc_request(). * @buf: Buffer used for data. Always provide this; some controllers * only use PIO, or don't use DMA for some endpoints. * @dma: DMA address corresponding to 'buf'. If you don't set this @@ -98,6 +99,7 @@ struct usb_ep; */
struct usb_request { + struct usb_ep *ep; void *buf; unsigned length; dma_addr_t dma;
From: Kuen-Han Tsai khtsai@google.com
[ Upstream commit 201c53c687f2b55a7cc6d9f4000af4797860174b ]
Introduce the free_usb_request() function that frees both the request's buffer and the request itself.
This function serves as the cleanup callback for DEFINE_FREE() to enable automatic, scope-based cleanup for usb_request pointers.
Signed-off-by: Kuen-Han Tsai khtsai@google.com Link: https://lore.kernel.org/r/20250916-ready-v1-2-4997bf277548@google.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Link: https://lore.kernel.org/r/20250916-ready-v1-2-4997bf277548@google.com Stable-dep-of: 75a5b8d4ddd4 ("usb: gadget: f_ncm: Refactor bind path to use __free()") Signed-off-by: Sasha Levin sashal@kernel.org --- include/linux/usb/gadget.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index 0f20794760887..3aaf19e775580 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -15,6 +15,7 @@ #ifndef __LINUX_USB_GADGET_H #define __LINUX_USB_GADGET_H
+#include <linux/cleanup.h> #include <linux/configfs.h> #include <linux/device.h> #include <linux/errno.h> @@ -293,6 +294,28 @@ static inline void usb_ep_fifo_flush(struct usb_ep *ep)
/*-------------------------------------------------------------------------*/
+/** + * free_usb_request - frees a usb_request object and its buffer + * @req: the request being freed + * + * This helper function frees both the request's buffer and the request object + * itself by calling usb_ep_free_request(). Its signature is designed to be used + * with DEFINE_FREE() to enable automatic, scope-based cleanup for usb_request + * pointers. + */ +static inline void free_usb_request(struct usb_request *req) +{ + if (!req) + return; + + kfree(req->buf); + usb_ep_free_request(req->ep, req); +} + +DEFINE_FREE(free_usb_request, struct usb_request *, free_usb_request(_T)) + +/*-------------------------------------------------------------------------*/ + struct usb_dcd_config_params { __u8 bU1devExitLat; /* U1 Device exit Latency */ #define USB_DEFAULT_U1_DEV_EXIT_LAT 0x01 /* Less then 1 microsec */
From: Kuen-Han Tsai khtsai@google.com
[ Upstream commit 75a5b8d4ddd4eb6b16cb0b475d14ff4ae64295ef ]
After an bind/unbind cycle, the ncm->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.
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000020 Call trace: usb_ep_free_request+0x2c/0xec ncm_bind+0x39c/0x3dc usb_add_function+0xcc/0x1f0 configfs_composite_bind+0x468/0x588 gadget_bind_driver+0x104/0x270 really_probe+0x190/0x374 __driver_probe_device+0xa0/0x12c driver_probe_device+0x3c/0x218 __device_attach_driver+0x14c/0x188 bus_for_each_drv+0x10c/0x168 __device_attach+0xfc/0x198 device_initial_probe+0x14/0x24 bus_probe_device+0x94/0x11c device_add+0x268/0x48c usb_add_gadget+0x198/0x28c dwc3_gadget_init+0x700/0x858 __dwc3_set_mode+0x3cc/0x664 process_scheduled_works+0x1d8/0x488 worker_thread+0x244/0x334 kthread+0x114/0x1bc ret_from_fork+0x10/0x20
Fixes: 9f6ce4240a2b ("usb: gadget: f_ncm.c added") Cc: stable@kernel.org Signed-off-by: Kuen-Han Tsai khtsai@google.com Link: https://lore.kernel.org/r/20250916-ready-v1-3-4997bf277548@google.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Link: https://lore.kernel.org/r/20250916-ready-v1-3-4997bf277548@google.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/usb/gadget/function/f_ncm.c | 78 ++++++++++++----------------- 1 file changed, 33 insertions(+), 45 deletions(-)
diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c index 58b0dd575af32..0148d60926dcf 100644 --- a/drivers/usb/gadget/function/f_ncm.c +++ b/drivers/usb/gadget/function/f_ncm.c @@ -11,6 +11,7 @@ * Copyright (C) 2008 Nokia Corporation */
+#include <linux/cleanup.h> #include <linux/kernel.h> #include <linux/interrupt.h> #include <linux/module.h> @@ -20,6 +21,7 @@ #include <linux/string_choices.h>
#include <linux/usb/cdc.h> +#include <linux/usb/gadget.h>
#include "u_ether.h" #include "u_ether_configfs.h" @@ -1436,18 +1438,18 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f) struct usb_ep *ep; struct f_ncm_opts *ncm_opts;
+ struct usb_os_desc_table *os_desc_table __free(kfree) = NULL; + struct usb_request *request __free(free_usb_request) = NULL; + if (!can_support_ecm(cdev->gadget)) return -EINVAL;
ncm_opts = container_of(f->fi, struct f_ncm_opts, func_inst);
if (cdev->use_os_string) { - f->os_desc_table = kzalloc(sizeof(*f->os_desc_table), - GFP_KERNEL); - if (!f->os_desc_table) + os_desc_table = kzalloc(sizeof(*os_desc_table), GFP_KERNEL); + if (!os_desc_table) return -ENOMEM; - f->os_desc_n = 1; - f->os_desc_table[0].os_desc = &ncm_opts->ncm_os_desc; }
mutex_lock(&ncm_opts->lock); @@ -1459,16 +1461,15 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f) mutex_unlock(&ncm_opts->lock);
if (status) - goto fail; + return status;
ncm_opts->bound = true;
us = usb_gstrings_attach(cdev, ncm_strings, ARRAY_SIZE(ncm_string_defs)); - if (IS_ERR(us)) { - status = PTR_ERR(us); - goto fail; - } + if (IS_ERR(us)) + return PTR_ERR(us); + ncm_control_intf.iInterface = us[STRING_CTRL_IDX].id; ncm_data_nop_intf.iInterface = us[STRING_DATA_IDX].id; ncm_data_intf.iInterface = us[STRING_DATA_IDX].id; @@ -1478,20 +1479,16 @@ static int ncm_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; ncm->ctrl_id = status; ncm_iad_desc.bFirstInterface = status;
ncm_control_intf.bInterfaceNumber = status; ncm_union_desc.bMasterInterface0 = status;
- if (cdev->use_os_string) - f->os_desc_table[0].if_id = - ncm_iad_desc.bFirstInterface; - status = usb_interface_id(c, f); if (status < 0) - goto fail; + return status; ncm->data_id = status;
ncm_data_nop_intf.bInterfaceNumber = status; @@ -1500,35 +1497,31 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
ecm_desc.wMaxSegmentSize = cpu_to_le16(ncm_opts->max_segment_size);
- status = -ENODEV; - /* allocate instance-specific endpoints */ ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_in_desc); if (!ep) - goto fail; + return -ENODEV; ncm->port.in_ep = ep;
ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_out_desc); if (!ep) - goto fail; + return -ENODEV; ncm->port.out_ep = ep;
ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_notify_desc); if (!ep) - goto fail; + return -ENODEV; ncm->notify = ep;
- status = -ENOMEM; - /* allocate notification request and buffer */ - ncm->notify_req = usb_ep_alloc_request(ep, GFP_KERNEL); - if (!ncm->notify_req) - goto fail; - ncm->notify_req->buf = kmalloc(NCM_STATUS_BYTECOUNT, GFP_KERNEL); - if (!ncm->notify_req->buf) - goto fail; - ncm->notify_req->context = ncm; - ncm->notify_req->complete = ncm_notify_complete; + request = usb_ep_alloc_request(ep, GFP_KERNEL); + if (!request) + return -ENOMEM; + request->buf = kmalloc(NCM_STATUS_BYTECOUNT, GFP_KERNEL); + if (!request->buf) + return -ENOMEM; + request->context = ncm; + request->complete = ncm_notify_complete;
/* * support all relevant hardware speeds... we expect that when @@ -1548,7 +1541,7 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f) status = usb_assign_descriptors(f, ncm_fs_function, ncm_hs_function, ncm_ss_function, ncm_ss_function); if (status) - goto fail; + return status;
/* * NOTE: all that is done without knowing or caring about @@ -1561,23 +1554,18 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
hrtimer_setup(&ncm->task_timer, ncm_tx_timeout, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
+ if (cdev->use_os_string) { + os_desc_table[0].os_desc = &ncm_opts->ncm_os_desc; + os_desc_table[0].if_id = ncm_iad_desc.bFirstInterface; + f->os_desc_table = no_free_ptr(os_desc_table); + f->os_desc_n = 1; + } + ncm->notify_req = no_free_ptr(request); + DBG(cdev, "CDC Network: IN/%s OUT/%s NOTIFY/%s\n", ncm->port.in_ep->name, ncm->port.out_ep->name, ncm->notify->name); return 0; - -fail: - kfree(f->os_desc_table); - f->os_desc_n = 0; - - if (ncm->notify_req) { - kfree(ncm->notify_req->buf); - usb_ep_free_request(ncm->notify, ncm->notify_req); - } - - ERROR(cdev, "%s: can't bind, err %d\n", f->name, status); - - return status; }
static inline struct f_ncm_opts *to_f_ncm_opts(struct config_item *item)
linux-stable-mirror@lists.linaro.org