Commit 47b2116e authored by Kuen-Han Tsai's avatar Kuen-Han Tsai Committed by Greg Kroah-Hartman
Browse files

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



After an bind/unbind cycle, the acm->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
 gs_free_req+0x30/0x44
 acm_bind+0x1b8/0x1f4
 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: 1f1ba11b ("usb gadget: issue notifications from ACM function")
Cc: stable@kernel.org
Signed-off-by: default avatarKuen-Han Tsai <khtsai@google.com>
Link: https://lore.kernel.org/r/20250916-ready-v1-4-4997bf277548@google.com


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Link: https://lore.kernel.org/r/20250916-ready-v1-4-4997bf277548@google.com
parent 75a5b8d4

drivers/usb/gadget/function/f_acm.c

+19 −23
Original line number Diff line number Diff line
@@ -11,12 +11,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/err.h>



#include <linux/usb/gadget.h>



#include "u_serial.h"
@@ -613,6 +616,7 @@ acm_bind(struct usb_configuration *c, struct usb_function *f) 
	struct usb_string	*us;

	int			status;

	struct usb_ep		*ep;

	struct usb_request	*request __free(free_usb_request) = NULL;



	/* REVISIT might want instance-specific strings to help

	 * distinguish instances ...
@@ -630,7 +634,7 @@ acm_bind(struct usb_configuration *c, struct usb_function *f) 
	/* allocate instance-specific interface IDs, and patch descriptors */

	status = usb_interface_id(c, f);

	if (status < 0)

		goto fail;

		return status;

	acm->ctrl_id = status;

	acm_iad_descriptor.bFirstInterface = status;
@@ -639,43 +643,41 @@ acm_bind(struct usb_configuration *c, struct usb_function *f) 


	status = usb_interface_id(c, f);

	if (status < 0)

		goto fail;

		return status;

	acm->data_id = status;



	acm_data_interface_desc.bInterfaceNumber = status;

	acm_union_desc.bSlaveInterface0 = status;

	acm_call_mgmt_descriptor.bDataInterface = status;



	status = -ENODEV;



	/* allocate instance-specific endpoints */

	ep = usb_ep_autoconfig(cdev->gadget, &acm_fs_in_desc);

	if (!ep)

		goto fail;

		return -ENODEV;

	acm->port.in = ep;



	ep = usb_ep_autoconfig(cdev->gadget, &acm_fs_out_desc);

	if (!ep)

		goto fail;

		return -ENODEV;

	acm->port.out = ep;



	ep = usb_ep_autoconfig(cdev->gadget, &acm_fs_notify_desc);

	if (!ep)

		goto fail;

		return -ENODEV;

	acm->notify = ep;



	acm_iad_descriptor.bFunctionProtocol = acm->bInterfaceProtocol;

	acm_control_interface_desc.bInterfaceProtocol = acm->bInterfaceProtocol;



	/* allocate notification */

	acm->notify_req = gs_alloc_req(ep,

	request = gs_alloc_req(ep,

			       sizeof(struct usb_cdc_notification) + 2,

			       GFP_KERNEL);

	if (!acm->notify_req)

		goto fail;

	if (!request)

		return -ENODEV;



	acm->notify_req->complete = acm_cdc_notify_complete;

	acm->notify_req->context = acm;

	request->complete = acm_cdc_notify_complete;

	request->context = acm;



	/* support all relevant hardware speeds... we expect that when

	 * hardware is dual speed, all bulk-capable endpoints work at
@@ -692,7 +694,9 @@ acm_bind(struct usb_configuration *c, struct usb_function *f) 
	status = usb_assign_descriptors(f, acm_fs_function, acm_hs_function,

			acm_ss_function, acm_ss_function);

	if (status)

		goto fail;

		return status;



	acm->notify_req = no_free_ptr(request);



	dev_dbg(&cdev->gadget->dev,

		"acm ttyGS%d: IN/%s OUT/%s NOTIFY/%s\n",
@@ -700,14 +704,6 @@ acm_bind(struct usb_configuration *c, struct usb_function *f) 
		acm->port.in->name, acm->port.out->name,

		acm->notify->name);

	return 0;



fail:

	if (acm->notify_req)

		gs_free_req(acm->notify, acm->notify_req);



	ERROR(cdev, "%s/%p: can't bind, err %d\n", f->name, f, status);



	return status;

}



static void acm_unbind(struct usb_configuration *c, struct usb_function *f)