Skip to content

Commit 4298838

Browse files
Kuen-Han Tsaigregkh
authored andcommitted
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: da741b8 ("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
1 parent 47b2116 commit 4298838

1 file changed

Lines changed: 20 additions & 28 deletions

File tree

drivers/usb/gadget/function/f_ecm.c

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,16 @@
88

99
/* #define VERBOSE_DEBUG */
1010

11+
#include <linux/cleanup.h>
1112
#include <linux/slab.h>
1213
#include <linux/kernel.h>
1314
#include <linux/module.h>
1415
#include <linux/device.h>
1516
#include <linux/etherdevice.h>
1617
#include <linux/string_choices.h>
1718

19+
#include <linux/usb/gadget.h>
20+
1821
#include "u_ether.h"
1922
#include "u_ether_configfs.h"
2023
#include "u_ecm.h"
@@ -678,6 +681,7 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
678681
struct usb_ep *ep;
679682

680683
struct f_ecm_opts *ecm_opts;
684+
struct usb_request *request __free(free_usb_request) = NULL;
681685

682686
if (!can_support_ecm(cdev->gadget))
683687
return -EINVAL;
@@ -711,7 +715,7 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
711715
/* allocate instance-specific interface IDs */
712716
status = usb_interface_id(c, f);
713717
if (status < 0)
714-
goto fail;
718+
return status;
715719
ecm->ctrl_id = status;
716720
ecm_iad_descriptor.bFirstInterface = status;
717721

@@ -720,24 +724,22 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
720724

721725
status = usb_interface_id(c, f);
722726
if (status < 0)
723-
goto fail;
727+
return status;
724728
ecm->data_id = status;
725729

726730
ecm_data_nop_intf.bInterfaceNumber = status;
727731
ecm_data_intf.bInterfaceNumber = status;
728732
ecm_union_desc.bSlaveInterface0 = status;
729733

730-
status = -ENODEV;
731-
732734
/* allocate instance-specific endpoints */
733735
ep = usb_ep_autoconfig(cdev->gadget, &fs_ecm_in_desc);
734736
if (!ep)
735-
goto fail;
737+
return -ENODEV;
736738
ecm->port.in_ep = ep;
737739

738740
ep = usb_ep_autoconfig(cdev->gadget, &fs_ecm_out_desc);
739741
if (!ep)
740-
goto fail;
742+
return -ENODEV;
741743
ecm->port.out_ep = ep;
742744

743745
/* NOTE: a status/notification endpoint is *OPTIONAL* but we
@@ -746,20 +748,18 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
746748
*/
747749
ep = usb_ep_autoconfig(cdev->gadget, &fs_ecm_notify_desc);
748750
if (!ep)
749-
goto fail;
751+
return -ENODEV;
750752
ecm->notify = ep;
751753

752-
status = -ENOMEM;
753-
754754
/* allocate notification request and buffer */
755-
ecm->notify_req = usb_ep_alloc_request(ep, GFP_KERNEL);
756-
if (!ecm->notify_req)
757-
goto fail;
758-
ecm->notify_req->buf = kmalloc(ECM_STATUS_BYTECOUNT, GFP_KERNEL);
759-
if (!ecm->notify_req->buf)
760-
goto fail;
761-
ecm->notify_req->context = ecm;
762-
ecm->notify_req->complete = ecm_notify_complete;
755+
request = usb_ep_alloc_request(ep, GFP_KERNEL);
756+
if (!request)
757+
return -ENOMEM;
758+
request->buf = kmalloc(ECM_STATUS_BYTECOUNT, GFP_KERNEL);
759+
if (!request->buf)
760+
return -ENOMEM;
761+
request->context = ecm;
762+
request->complete = ecm_notify_complete;
763763

764764
/* support all relevant hardware speeds... we expect that when
765765
* hardware is dual speed, all bulk-capable endpoints work at
@@ -778,7 +778,7 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
778778
status = usb_assign_descriptors(f, ecm_fs_function, ecm_hs_function,
779779
ecm_ss_function, ecm_ss_function);
780780
if (status)
781-
goto fail;
781+
return status;
782782

783783
/* NOTE: all that is done without knowing or caring about
784784
* the network link ... which is unavailable to this code
@@ -788,20 +788,12 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
788788
ecm->port.open = ecm_open;
789789
ecm->port.close = ecm_close;
790790

791+
ecm->notify_req = no_free_ptr(request);
792+
791793
DBG(cdev, "CDC Ethernet: IN/%s OUT/%s NOTIFY/%s\n",
792794
ecm->port.in_ep->name, ecm->port.out_ep->name,
793795
ecm->notify->name);
794796
return 0;
795-
796-
fail:
797-
if (ecm->notify_req) {
798-
kfree(ecm->notify_req->buf);
799-
usb_ep_free_request(ecm->notify, ecm->notify_req);
800-
}
801-
802-
ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
803-
804-
return status;
805797
}
806798

807799
static inline struct f_ecm_opts *to_f_ecm_opts(struct config_item *item)

0 commit comments

Comments
 (0)