usb: gadget: f_ncm: Refactor bind path to use __free()

[ 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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 00995d65b54c7dba86545d9d039bb6492a3be5dc..4fe6a1efe098620c7759988e82dae1df585101cf 100644 (file)
--- 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>
@@ -19,6 +20,7 @@
 #include <linux/crc32.h>
 
 #include <linux/usb/cdc.h>
+#include <linux/usb/gadget.h>
 
 #include "u_ether.h"
 #include "u_ether_configfs.h"
@@ -1441,18 +1443,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);
@@ -1462,16 +1464,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;
@@ -1481,55 +1482,47 @@ 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;
        ncm_data_intf.bInterfaceNumber = status;
        ncm_union_desc.bSlaveInterface0 = status;
 
-       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
@@ -1549,7 +1542,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
@@ -1563,25 +1556,20 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
        hrtimer_init(&ncm->task_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
        ncm->task_timer.function = ncm_tx_timeout;
 
+       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: %s speed IN/%s OUT/%s NOTIFY/%s\n",
                        gadget_is_superspeed(c->cdev->gadget) ? "super" :
                        gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full",
                        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)