Skip to content

Commit 75a5b8d

Browse files
Kuen-Han Tsaigregkh
Kuen-Han Tsai
authored and
gregkh
committed
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: 9f6ce42 ("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
1 parent 201c53c commit 75a5b8d

1 file changed

Lines changed: 33 additions & 45 deletions

File tree

‎drivers/usb/gadget/function/f_ncm.c‎

Lines changed: 33 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
* Copyright (C) 2008 Nokia Corporation
1212
*/
1313

14+
#include <linux/cleanup.h>
1415
#include <linux/kernel.h>
1516
#include <linux/interrupt.h>
1617
#include <linux/module.h>
@@ -20,6 +21,7 @@
2021
#include <linux/string_choices.h>
2122

2223
#include <linux/usb/cdc.h>
24+
#include <linux/usb/gadget.h>
2325

2426
#include "u_ether.h"
2527
#include "u_ether_configfs.h"
@@ -1436,18 +1438,18 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
14361438
struct usb_ep *ep;
14371439
struct f_ncm_opts *ncm_opts;
14381440

1441+
struct usb_os_desc_table *os_desc_table __free(kfree) = NULL;
1442+
struct usb_request *request __free(free_usb_request) = NULL;
1443+
14391444
if (!can_support_ecm(cdev->gadget))
14401445
return -EINVAL;
14411446

14421447
ncm_opts = container_of(f->fi, struct f_ncm_opts, func_inst);
14431448

14441449
if (cdev->use_os_string) {
1445-
f->os_desc_table = kzalloc(sizeof(*f->os_desc_table),
1446-
GFP_KERNEL);
1447-
if (!f->os_desc_table)
1450+
os_desc_table = kzalloc(sizeof(*os_desc_table), GFP_KERNEL);
1451+
if (!os_desc_table)
14481452
return -ENOMEM;
1449-
f->os_desc_n = 1;
1450-
f->os_desc_table[0].os_desc = &ncm_opts->ncm_os_desc;
14511453
}
14521454

14531455
mutex_lock(&ncm_opts->lock);
@@ -1459,16 +1461,15 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
14591461
mutex_unlock(&ncm_opts->lock);
14601462

14611463
if (status)
1462-
goto fail;
1464+
return status;
14631465

14641466
ncm_opts->bound = true;
14651467

14661468
us = usb_gstrings_attach(cdev, ncm_strings,
14671469
ARRAY_SIZE(ncm_string_defs));
1468-
if (IS_ERR(us)) {
1469-
status = PTR_ERR(us);
1470-
goto fail;
1471-
}
1470+
if (IS_ERR(us))
1471+
return PTR_ERR(us);
1472+
14721473
ncm_control_intf.iInterface = us[STRING_CTRL_IDX].id;
14731474
ncm_data_nop_intf.iInterface = us[STRING_DATA_IDX].id;
14741475
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)
14781479
/* allocate instance-specific interface IDs */
14791480
status = usb_interface_id(c, f);
14801481
if (status < 0)
1481-
goto fail;
1482+
return status;
14821483
ncm->ctrl_id = status;
14831484
ncm_iad_desc.bFirstInterface = status;
14841485

14851486
ncm_control_intf.bInterfaceNumber = status;
14861487
ncm_union_desc.bMasterInterface0 = status;
14871488

1488-
if (cdev->use_os_string)
1489-
f->os_desc_table[0].if_id =
1490-
ncm_iad_desc.bFirstInterface;
1491-
14921489
status = usb_interface_id(c, f);
14931490
if (status < 0)
1494-
goto fail;
1491+
return status;
14951492
ncm->data_id = status;
14961493

14971494
ncm_data_nop_intf.bInterfaceNumber = status;
@@ -1500,35 +1497,31 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
15001497

15011498
ecm_desc.wMaxSegmentSize = cpu_to_le16(ncm_opts->max_segment_size);
15021499

1503-
status = -ENODEV;
1504-
15051500
/* allocate instance-specific endpoints */
15061501
ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_in_desc);
15071502
if (!ep)
1508-
goto fail;
1503+
return -ENODEV;
15091504
ncm->port.in_ep = ep;
15101505

15111506
ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_out_desc);
15121507
if (!ep)
1513-
goto fail;
1508+
return -ENODEV;
15141509
ncm->port.out_ep = ep;
15151510

15161511
ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_notify_desc);
15171512
if (!ep)
1518-
goto fail;
1513+
return -ENODEV;
15191514
ncm->notify = ep;
15201515

1521-
status = -ENOMEM;
1522-
15231516
/* allocate notification request and buffer */
1524-
ncm->notify_req = usb_ep_alloc_request(ep, GFP_KERNEL);
1525-
if (!ncm->notify_req)
1526-
goto fail;
1527-
ncm->notify_req->buf = kmalloc(NCM_STATUS_BYTECOUNT, GFP_KERNEL);
1528-
if (!ncm->notify_req->buf)
1529-
goto fail;
1530-
ncm->notify_req->context = ncm;
1531-
ncm->notify_req->complete = ncm_notify_complete;
1517+
request = usb_ep_alloc_request(ep, GFP_KERNEL);
1518+
if (!request)
1519+
return -ENOMEM;
1520+
request->buf = kmalloc(NCM_STATUS_BYTECOUNT, GFP_KERNEL);
1521+
if (!request->buf)
1522+
return -ENOMEM;
1523+
request->context = ncm;
1524+
request->complete = ncm_notify_complete;
15321525

15331526
/*
15341527
* 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)
15481541
status = usb_assign_descriptors(f, ncm_fs_function, ncm_hs_function,
15491542
ncm_ss_function, ncm_ss_function);
15501543
if (status)
1551-
goto fail;
1544+
return status;
15521545

15531546
/*
15541547
* 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)
15611554

15621555
hrtimer_setup(&ncm->task_timer, ncm_tx_timeout, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
15631556

1557+
if (cdev->use_os_string) {
1558+
os_desc_table[0].os_desc = &ncm_opts->ncm_os_desc;
1559+
os_desc_table[0].if_id = ncm_iad_desc.bFirstInterface;
1560+
f->os_desc_table = no_free_ptr(os_desc_table);
1561+
f->os_desc_n = 1;
1562+
}
1563+
ncm->notify_req = no_free_ptr(request);
1564+
15641565
DBG(cdev, "CDC Network: IN/%s OUT/%s NOTIFY/%s\n",
15651566
ncm->port.in_ep->name, ncm->port.out_ep->name,
15661567
ncm->notify->name);
15671568
return 0;
1568-
1569-
fail:
1570-
kfree(f->os_desc_table);
1571-
f->os_desc_n = 0;
1572-
1573-
if (ncm->notify_req) {
1574-
kfree(ncm->notify_req->buf);
1575-
usb_ep_free_request(ncm->notify, ncm->notify_req);
1576-
}
1577-
1578-
ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
1579-
1580-
return status;
15811569
}
15821570

15831571
static inline struct f_ncm_opts *to_f_ncm_opts(struct config_item *item)

0 commit comments

Comments
 (0)