Skip to content

Commit 0822894

Browse files
Kuen-Han Tsaigregkh
authored andcommitted
usb: gadget: f_rndis: Refactor bind path to use __free()
After an bind/unbind cycle, the rndis->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: 45fe3b8 ("usb ethernet gadget: split RNDIS function") Cc: stable@kernel.org Signed-off-by: Kuen-Han Tsai <khtsai@google.com> Link: https://lore.kernel.org/r/20250916-ready-v1-6-4997bf277548@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Link: https://lore.kernel.org/r/20250916-ready-v1-6-4997bf277548@google.com
1 parent 4298838 commit 0822894

1 file changed

Lines changed: 35 additions & 50 deletions

File tree

drivers/usb/gadget/function/f_rndis.c

Lines changed: 35 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
#include <linux/atomic.h>
2121

22+
#include <linux/usb/gadget.h>
23+
2224
#include "u_ether.h"
2325
#include "u_ether_configfs.h"
2426
#include "u_rndis.h"
@@ -662,19 +664,18 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
662664
struct usb_ep *ep;
663665

664666
struct f_rndis_opts *rndis_opts;
667+
struct usb_os_desc_table *os_desc_table __free(kfree) = NULL;
668+
struct usb_request *request __free(free_usb_request) = NULL;
665669

666670
if (!can_support_rndis(c))
667671
return -EINVAL;
668672

669673
rndis_opts = container_of(f->fi, struct f_rndis_opts, func_inst);
670674

671675
if (cdev->use_os_string) {
672-
f->os_desc_table = kzalloc(sizeof(*f->os_desc_table),
673-
GFP_KERNEL);
674-
if (!f->os_desc_table)
676+
os_desc_table = kzalloc(sizeof(*os_desc_table), GFP_KERNEL);
677+
if (!os_desc_table)
675678
return -ENOMEM;
676-
f->os_desc_n = 1;
677-
f->os_desc_table[0].os_desc = &rndis_opts->rndis_os_desc;
678679
}
679680

680681
rndis_iad_descriptor.bFunctionClass = rndis_opts->class;
@@ -692,53 +693,45 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
692693
gether_set_gadget(rndis_opts->net, cdev->gadget);
693694
status = gether_register_netdev(rndis_opts->net);
694695
if (status)
695-
goto fail;
696+
return status;
696697
rndis_opts->bound = true;
697698
}
698699

699700
us = usb_gstrings_attach(cdev, rndis_strings,
700701
ARRAY_SIZE(rndis_string_defs));
701-
if (IS_ERR(us)) {
702-
status = PTR_ERR(us);
703-
goto fail;
704-
}
702+
if (IS_ERR(us))
703+
return PTR_ERR(us);
705704
rndis_control_intf.iInterface = us[0].id;
706705
rndis_data_intf.iInterface = us[1].id;
707706
rndis_iad_descriptor.iFunction = us[2].id;
708707

709708
/* allocate instance-specific interface IDs */
710709
status = usb_interface_id(c, f);
711710
if (status < 0)
712-
goto fail;
711+
return status;
713712
rndis->ctrl_id = status;
714713
rndis_iad_descriptor.bFirstInterface = status;
715714

716715
rndis_control_intf.bInterfaceNumber = status;
717716
rndis_union_desc.bMasterInterface0 = status;
718717

719-
if (cdev->use_os_string)
720-
f->os_desc_table[0].if_id =
721-
rndis_iad_descriptor.bFirstInterface;
722-
723718
status = usb_interface_id(c, f);
724719
if (status < 0)
725-
goto fail;
720+
return status;
726721
rndis->data_id = status;
727722

728723
rndis_data_intf.bInterfaceNumber = status;
729724
rndis_union_desc.bSlaveInterface0 = status;
730725

731-
status = -ENODEV;
732-
733726
/* allocate instance-specific endpoints */
734727
ep = usb_ep_autoconfig(cdev->gadget, &fs_in_desc);
735728
if (!ep)
736-
goto fail;
729+
return -ENODEV;
737730
rndis->port.in_ep = ep;
738731

739732
ep = usb_ep_autoconfig(cdev->gadget, &fs_out_desc);
740733
if (!ep)
741-
goto fail;
734+
return -ENODEV;
742735
rndis->port.out_ep = ep;
743736

744737
/* NOTE: a status/notification endpoint is, strictly speaking,
@@ -747,21 +740,19 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
747740
*/
748741
ep = usb_ep_autoconfig(cdev->gadget, &fs_notify_desc);
749742
if (!ep)
750-
goto fail;
743+
return -ENODEV;
751744
rndis->notify = ep;
752745

753-
status = -ENOMEM;
754-
755746
/* allocate notification request and buffer */
756-
rndis->notify_req = usb_ep_alloc_request(ep, GFP_KERNEL);
757-
if (!rndis->notify_req)
758-
goto fail;
759-
rndis->notify_req->buf = kmalloc(STATUS_BYTECOUNT, GFP_KERNEL);
760-
if (!rndis->notify_req->buf)
761-
goto fail;
762-
rndis->notify_req->length = STATUS_BYTECOUNT;
763-
rndis->notify_req->context = rndis;
764-
rndis->notify_req->complete = rndis_response_complete;
747+
request = usb_ep_alloc_request(ep, GFP_KERNEL);
748+
if (!request)
749+
return -ENOMEM;
750+
request->buf = kmalloc(STATUS_BYTECOUNT, GFP_KERNEL);
751+
if (!request->buf)
752+
return -ENOMEM;
753+
request->length = STATUS_BYTECOUNT;
754+
request->context = rndis;
755+
request->complete = rndis_response_complete;
765756

766757
/* support all relevant hardware speeds... we expect that when
767758
* hardware is dual speed, all bulk-capable endpoints work at
@@ -778,7 +769,7 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
778769
status = usb_assign_descriptors(f, eth_fs_function, eth_hs_function,
779770
eth_ss_function, eth_ss_function);
780771
if (status)
781-
goto fail;
772+
return status;
782773

783774
rndis->port.open = rndis_open;
784775
rndis->port.close = rndis_close;
@@ -789,9 +780,18 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
789780
if (rndis->manufacturer && rndis->vendorID &&
790781
rndis_set_param_vendor(rndis->params, rndis->vendorID,
791782
rndis->manufacturer)) {
792-
status = -EINVAL;
793-
goto fail_free_descs;
783+
usb_free_all_descriptors(f);
784+
return -EINVAL;
785+
}
786+
787+
if (cdev->use_os_string) {
788+
os_desc_table[0].os_desc = &rndis_opts->rndis_os_desc;
789+
os_desc_table[0].if_id = rndis_iad_descriptor.bFirstInterface;
790+
f->os_desc_table = no_free_ptr(os_desc_table);
791+
f->os_desc_n = 1;
792+
794793
}
794+
rndis->notify_req = no_free_ptr(request);
795795

796796
/* NOTE: all that is done without knowing or caring about
797797
* the network link ... which is unavailable to this code
@@ -802,21 +802,6 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
802802
rndis->port.in_ep->name, rndis->port.out_ep->name,
803803
rndis->notify->name);
804804
return 0;
805-
806-
fail_free_descs:
807-
usb_free_all_descriptors(f);
808-
fail:
809-
kfree(f->os_desc_table);
810-
f->os_desc_n = 0;
811-
812-
if (rndis->notify_req) {
813-
kfree(rndis->notify_req->buf);
814-
usb_ep_free_request(rndis->notify, rndis->notify_req);
815-
}
816-
817-
ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
818-
819-
return status;
820805
}
821806

822807
void rndis_borrow_net(struct usb_function_instance *f, struct net_device *net)

0 commit comments

Comments
 (0)