Skip to content

Commit 64ed437

Browse files
rubycommclaude
authored and
igorpecovnik
committed
rockchip64: fix USB gadget NULL pointer crash in eth_get_drvinfo (6.18 + 7.0)
Port Ondrej Jirman's (megi) dangling pointer fix from sunxi to rockchip64. Without this patch, udev querying ethtool on the USB gadget ethernet interface during DWC3 bind/unbind cycles can hit a NULL pointer dereference in eth_get_drvinfo(), causing intermittent kernel panics at boot. The f_ncm.c hunks from the original patch are excluded because mainline already handles NCM gadget lifecycle properly via the gether_attach_gadget()/gether_detach_gadget() API. Tested on NanoPi Zero2 (RK3528) with 8 consecutive reboot cycles and zero crashes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 84d61ae commit 64ed437

2 files changed

Lines changed: 538 additions & 0 deletions

File tree

‎patch/kernel/archive/rockchip64-6.18/general-usb-gadget-fix-dangling-pointer-in-netdev-private-data.patch‎

Lines changed: 269 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,269 @@
1+
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2+
From: Ondrej Jirman <megi@xff.cz>
3+
Date: Thu, 7 Sep 2023 14:07:26 +0200
4+
Subject: usb: gadget: Fix dangling pointer in netdev private data
5+
6+
Some USB drivers destroy and re-create the gadget regularly. This
7+
leads to stale gadget pointer in gether_* using USB gadget functions
8+
(ecm, eem, rndis, etc.).
9+
10+
Don't parent the netdev to the gadget device, and set gadget to
11+
NULL in function unbind callback, to avoid potential use-after-free
12+
issues.
13+
14+
Note: f_ncm.c is excluded because mainline already handles gadget
15+
lifecycle via gether_attach_gadget()/gether_detach_gadget().
16+
17+
Signed-off-by: Ondrej Jirman <megi@xff.cz>
18+
---
19+
drivers/usb/gadget/function/f_ecm.c | 12 ++---
20+
drivers/usb/gadget/function/f_eem.c | 28 +++++------
21+
drivers/usb/gadget/function/f_rndis.c | 26 ++++------
22+
drivers/usb/gadget/function/f_subset.c | 18 ++++---
23+
drivers/usb/gadget/function/u_ether.c | 10 ++--
24+
5 files changed, 47 insertions(+), 47 deletions(-)
25+
26+
diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
27+
index 111111111111..222222222222 100644
28+
--- a/drivers/usb/gadget/function/f_ecm.c
29+
+++ b/drivers/usb/gadget/function/f_ecm.c
30+
@@ -689,14 +689,12 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
31+
ecm_opts = container_of(f->fi, struct f_ecm_opts, func_inst);
32+
33+
mutex_lock(&ecm_opts->lock);
34+
-
35+
gether_set_gadget(ecm_opts->net, cdev->gadget);
36+
-
37+
if (!ecm_opts->bound) {
38+
status = gether_register_netdev(ecm_opts->net);
39+
- ecm_opts->bound = true;
40+
+ if (!status)
41+
+ ecm_opts->bound = true;
42+
}
43+
-
44+
mutex_unlock(&ecm_opts->lock);
45+
if (status)
46+
return status;
47+
@@ -905,7 +903,9 @@ static void ecm_free(struct usb_function *f)
48+
49+
static void ecm_unbind(struct usb_configuration *c, struct usb_function *f)
50+
{
51+
- struct f_ecm *ecm = func_to_ecm(f);
52+
+ struct f_ecm *ecm = func_to_ecm(f);
53+
+ struct f_ecm_opts *opts = container_of(f->fi, struct f_ecm_opts,
54+
+ func_inst);
55+
56+
DBG(c->cdev, "ecm unbind\n");
57+
58+
@@ -918,6 +918,8 @@ static void ecm_unbind(struct usb_configuration *c, struct usb_function *f)
59+
60+
kfree(ecm->notify_req->buf);
61+
usb_ep_free_request(ecm->notify, ecm->notify_req);
62+
+
63+
+ gether_set_gadget(opts->net, NULL);
64+
}
65+
66+
static struct usb_function *ecm_alloc(struct usb_function_instance *fi)
67+
diff --git a/drivers/usb/gadget/function/f_eem.c b/drivers/usb/gadget/function/f_eem.c
68+
index 111111111111..222222222222 100644
69+
--- a/drivers/usb/gadget/function/f_eem.c
70+
+++ b/drivers/usb/gadget/function/f_eem.c
71+
@@ -247,28 +247,23 @@ static int eem_bind(struct usb_configuration *c, struct usb_function *f)
72+
struct usb_composite_dev *cdev = c->cdev;
73+
struct f_eem *eem = func_to_eem(f);
74+
struct usb_string *us;
75+
- int status;
76+
+ int status = 0;
77+
struct usb_ep *ep;
78+
79+
struct f_eem_opts *eem_opts;
80+
81+
eem_opts = container_of(f->fi, struct f_eem_opts, func_inst);
82+
- /*
83+
- * in drivers/usb/gadget/configfs.c:configfs_composite_bind()
84+
- * configurations are bound in sequence with list_for_each_entry,
85+
- * in each configuration its functions are bound in sequence
86+
- * with list_for_each_entry, so we assume no race condition
87+
- * with regard to eem_opts->bound access
88+
- */
89+
+
90+
+ mutex_lock(&eem_opts->lock);
91+
+ gether_set_gadget(eem_opts->net, cdev->gadget);
92+
if (!eem_opts->bound) {
93+
- mutex_lock(&eem_opts->lock);
94+
- gether_set_gadget(eem_opts->net, cdev->gadget);
95+
status = gether_register_netdev(eem_opts->net);
96+
- mutex_unlock(&eem_opts->lock);
97+
- if (status)
98+
- return status;
99+
- eem_opts->bound = true;
100+
+ if (!status)
101+
+ eem_opts->bound = true;
102+
}
103+
+ mutex_unlock(&eem_opts->lock);
104+
+ if (status)
105+
+ return status;
106+
107+
us = usb_gstrings_attach(cdev, eem_strings,
108+
ARRAY_SIZE(eem_string_defs));
109+
@@ -640,9 +635,14 @@ static void eem_free(struct usb_function *f)
110+
111+
static void eem_unbind(struct usb_configuration *c, struct usb_function *f)
112+
{
113+
+ struct f_eem_opts *opts = container_of(f->fi, struct f_eem_opts,
114+
+ func_inst);
115+
+
116+
DBG(c->cdev, "eem unbind\n");
117+
118+
usb_free_all_descriptors(f);
119+
+
120+
+ gether_set_gadget(opts->net, NULL);
121+
}
122+
123+
static struct usb_function *eem_alloc(struct usb_function_instance *fi)
124+
diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c
125+
index 111111111111..222222222222 100644
126+
--- a/drivers/usb/gadget/function/f_rndis.c
127+
+++ b/drivers/usb/gadget/function/f_rndis.c
128+
@@ -660,7 +660,7 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
129+
struct usb_composite_dev *cdev = c->cdev;
130+
struct f_rndis *rndis = func_to_rndis(f);
131+
struct usb_string *us;
132+
- int status;
133+
+ int status = 0;
134+
struct usb_ep *ep;
135+
136+
struct f_rndis_opts *rndis_opts;
137+
@@ -682,20 +682,16 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
138+
rndis_iad_descriptor.bFunctionSubClass = rndis_opts->subclass;
139+
rndis_iad_descriptor.bFunctionProtocol = rndis_opts->protocol;
140+
141+
- /*
142+
- * in drivers/usb/gadget/configfs.c:configfs_composite_bind()
143+
- * configurations are bound in sequence with list_for_each_entry,
144+
- * in each configuration its functions are bound in sequence
145+
- * with list_for_each_entry, so we assume no race condition
146+
- * with regard to rndis_opts->bound access
147+
- */
148+
+ mutex_lock(&rndis_opts->lock);
149+
+ gether_set_gadget(rndis_opts->net, cdev->gadget);
150+
if (!rndis_opts->bound) {
151+
- gether_set_gadget(rndis_opts->net, cdev->gadget);
152+
status = gether_register_netdev(rndis_opts->net);
153+
- if (status)
154+
- return status;
155+
- rndis_opts->bound = true;
156+
+ if (!status)
157+
+ rndis_opts->bound = true;
158+
}
159+
+ mutex_unlock(&rndis_opts->lock);
160+
+ if (status)
161+
+ return status;
162+
163+
us = usb_gstrings_attach(cdev, rndis_strings,
164+
ARRAY_SIZE(rndis_string_defs));
165+
@@ -939,7 +935,9 @@ static void rndis_free(struct usb_function *f)
166+
167+
static void rndis_unbind(struct usb_configuration *c, struct usb_function *f)
168+
{
169+
- struct f_rndis *rndis = func_to_rndis(f);
170+
+ struct f_rndis *rndis = func_to_rndis(f);
171+
+ struct f_rndis_opts *opts = container_of(f->fi, struct f_rndis_opts,
172+
+ func_inst);
173+
174+
kfree(f->os_desc_table);
175+
f->os_desc_n = 0;
176+
@@ -947,6 +945,8 @@ static void rndis_unbind(struct usb_configuration *c, struct usb_function *f)
177+
178+
kfree(rndis->notify_req->buf);
179+
usb_ep_free_request(rndis->notify, rndis->notify_req);
180+
+
181+
+ gether_set_gadget(opts->net, NULL);
182+
}
183+
184+
static struct usb_function *rndis_alloc(struct usb_function_instance *fi)
185+
diff --git a/drivers/usb/gadget/function/f_subset.c b/drivers/usb/gadget/function/f_subset.c
186+
index 111111111111..222222222222 100644
187+
--- a/drivers/usb/gadget/function/f_subset.c
188+
+++ b/drivers/usb/gadget/function/f_subset.c
189+
@@ -308,15 +308,16 @@ geth_bind(struct usb_configuration *c, struct usb_function *f)
190+
* with list_for_each_entry, so we assume no race condition
191+
* with regard to gether_opts->bound access
192+
*/
193+
+ mutex_lock(&gether_opts->lock);
194+
+ gether_set_gadget(gether_opts->net, cdev->gadget);
195+
if (!gether_opts->bound) {
196+
- mutex_lock(&gether_opts->lock);
197+
- gether_set_gadget(gether_opts->net, cdev->gadget);
198+
status = gether_register_netdev(gether_opts->net);
199+
- mutex_unlock(&gether_opts->lock);
200+
- if (status)
201+
- return status;
202+
- gether_opts->bound = true;
203+
+ if (!status)
204+
+ gether_opts->bound = true;
205+
}
206+
+ mutex_unlock(&gether_opts->lock);
207+
+ if (status)
208+
+ return status;
209+
210+
us = usb_gstrings_attach(cdev, geth_strings,
211+
ARRAY_SIZE(geth_string_defs));
212+
@@ -456,8 +457,13 @@ static void geth_free(struct usb_function *f)
213+
214+
static void geth_unbind(struct usb_configuration *c, struct usb_function *f)
215+
{
216+
+ struct f_gether_opts *opts = container_of(f->fi, struct f_gether_opts,
217+
+ func_inst);
218+
+
219+
geth_string_defs[0].id = 0;
220+
usb_free_all_descriptors(f);
221+
+
222+
+ gether_set_gadget(opts->net, NULL);
223+
}
224+
225+
static struct usb_function *geth_alloc(struct usb_function_instance *fi)
226+
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
227+
index 111111111111..222222222222 100644
228+
--- a/drivers/usb/gadget/function/u_ether.c
229+
+++ b/drivers/usb/gadget/function/u_ether.c
230+
@@ -112,8 +112,10 @@ static void eth_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *p)
231+
232+
strscpy(p->driver, "g_ether", sizeof(p->driver));
233+
strscpy(p->version, UETH__VERSION, sizeof(p->version));
234+
- strscpy(p->fw_version, dev->gadget->name, sizeof(p->fw_version));
235+
- strscpy(p->bus_info, dev_name(&dev->gadget->dev), sizeof(p->bus_info));
236+
+ if (dev->gadget) {
237+
+ strscpy(p->fw_version, dev->gadget->name, sizeof(p->fw_version));
238+
+ strscpy(p->bus_info, dev_name(&dev->gadget->dev), sizeof(p->bus_info));
239+
+ }
240+
}
241+
242+
/* REVISIT can also support:
243+
@@ -787,7 +789,6 @@ struct eth_dev *gether_setup_name(struct usb_gadget *g,
244+
net->max_mtu = GETHER_MAX_MTU_SIZE;
245+
246+
dev->gadget = g;
247+
- SET_NETDEV_DEV(net, &g->dev);
248+
SET_NETDEV_DEVTYPE(net, &gadget_type);
249+
250+
status = register_netdev(net);
251+
@@ -860,8 +861,6 @@ int gether_register_netdev(struct net_device *net)
252+
struct usb_gadget *g;
253+
int status;
254+
255+
- if (!net->dev.parent)
256+
- return -EINVAL;
257+
dev = netdev_priv(net);
258+
g = dev->gadget;
259+
260+
@@ -892,7 +891,6 @@ void gether_set_gadget(struct net_device *net, struct usb_gadget *g)
261+
262+
dev = netdev_priv(net);
263+
dev->gadget = g;
264+
- SET_NETDEV_DEV(net, &g->dev);
265+
}
266+
EXPORT_SYMBOL_GPL(gether_set_gadget);
267+
268+
--
269+
Armbian

0 commit comments

Comments
 (0)