Skip to content

rockchip64: fix USB gadget NULL pointer crash in eth_get_drvinfo (6.18 + 7.0) #9608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rubycomm wants to merge 1 commit into
base: main
Choose a base branch
from
+538 −0
Open

rockchip64: fix USB gadget NULL pointer crash in eth_get_drvinfo (6.18 + 7.0) #9608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
269 changes: 269 additions & 0 deletions ...hive/rockchip64-6.18/general-usb-gadget-fix-dangling-pointer-in-netdev-private-data.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,269 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Ondrej Jirman <megi@xff.cz>
Date: Thu, 7 Sep 2023 14:07:26 +0200
Subject: usb: gadget: Fix dangling pointer in netdev private data

Some USB drivers destroy and re-create the gadget regularly. This
leads to stale gadget pointer in gether_* using USB gadget functions
(ecm, eem, rndis, etc.).

Don't parent the netdev to the gadget device, and set gadget to
NULL in function unbind callback, to avoid potential use-after-free
issues.

Note: f_ncm.c is excluded because mainline already handles gadget
lifecycle via gether_attach_gadget()/gether_detach_gadget().

Signed-off-by: Ondrej Jirman <megi@xff.cz>
---
drivers/usb/gadget/function/f_ecm.c | 12 ++---
drivers/usb/gadget/function/f_eem.c | 28 +++++------
drivers/usb/gadget/function/f_rndis.c | 26 ++++------
drivers/usb/gadget/function/f_subset.c | 18 ++++---
drivers/usb/gadget/function/u_ether.c | 10 ++--
5 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index 111111111111..222222222222 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -689,14 +689,12 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
ecm_opts = container_of(f->fi, struct f_ecm_opts, func_inst);

mutex_lock(&ecm_opts->lock);
-
gether_set_gadget(ecm_opts->net, cdev->gadget);
-
if (!ecm_opts->bound) {
status = gether_register_netdev(ecm_opts->net);
- ecm_opts->bound = true;
+ if (!status)
+ ecm_opts->bound = true;
}
-
mutex_unlock(&ecm_opts->lock);
if (status)
return status;
@@ -905,7 +903,9 @@ static void ecm_free(struct usb_function *f)

static void ecm_unbind(struct usb_configuration *c, struct usb_function *f)
{
- struct f_ecm *ecm = func_to_ecm(f);
+ struct f_ecm *ecm = func_to_ecm(f);
+ struct f_ecm_opts *opts = container_of(f->fi, struct f_ecm_opts,
+ func_inst);

DBG(c->cdev, "ecm unbind\n");

@@ -918,6 +918,8 @@ static void ecm_unbind(struct usb_configuration *c, struct usb_function *f)

kfree(ecm->notify_req->buf);
usb_ep_free_request(ecm->notify, ecm->notify_req);
+
+ gether_set_gadget(opts->net, NULL);
}

static struct usb_function *ecm_alloc(struct usb_function_instance *fi)
diff --git a/drivers/usb/gadget/function/f_eem.c b/drivers/usb/gadget/function/f_eem.c
index 111111111111..222222222222 100644
--- a/drivers/usb/gadget/function/f_eem.c
+++ b/drivers/usb/gadget/function/f_eem.c
@@ -247,28 +247,23 @@ static int eem_bind(struct usb_configuration *c, struct usb_function *f)
struct usb_composite_dev *cdev = c->cdev;
struct f_eem *eem = func_to_eem(f);
struct usb_string *us;
- int status;
+ int status = 0;
struct usb_ep *ep;

struct f_eem_opts *eem_opts;

eem_opts = container_of(f->fi, struct f_eem_opts, func_inst);
- /*
- * in drivers/usb/gadget/configfs.c:configfs_composite_bind()
- * configurations are bound in sequence with list_for_each_entry,
- * in each configuration its functions are bound in sequence
- * with list_for_each_entry, so we assume no race condition
- * with regard to eem_opts->bound access
- */
+
+ mutex_lock(&eem_opts->lock);
+ gether_set_gadget(eem_opts->net, cdev->gadget);
if (!eem_opts->bound) {
- mutex_lock(&eem_opts->lock);
- gether_set_gadget(eem_opts->net, cdev->gadget);
status = gether_register_netdev(eem_opts->net);
- mutex_unlock(&eem_opts->lock);
- if (status)
- return status;
- eem_opts->bound = true;
+ if (!status)
+ eem_opts->bound = true;
}
+ mutex_unlock(&eem_opts->lock);
+ if (status)
+ return status;

us = usb_gstrings_attach(cdev, eem_strings,
ARRAY_SIZE(eem_string_defs));
@@ -640,9 +635,14 @@ static void eem_free(struct usb_function *f)

static void eem_unbind(struct usb_configuration *c, struct usb_function *f)
{
+ struct f_eem_opts *opts = container_of(f->fi, struct f_eem_opts,
+ func_inst);
+
DBG(c->cdev, "eem unbind\n");

usb_free_all_descriptors(f);
+
+ gether_set_gadget(opts->net, NULL);
}

static struct usb_function *eem_alloc(struct usb_function_instance *fi)
diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c
index 111111111111..222222222222 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -660,7 +660,7 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
struct usb_composite_dev *cdev = c->cdev;
struct f_rndis *rndis = func_to_rndis(f);
struct usb_string *us;
- int status;
+ int status = 0;
struct usb_ep *ep;

struct f_rndis_opts *rndis_opts;
@@ -682,20 +682,16 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
rndis_iad_descriptor.bFunctionSubClass = rndis_opts->subclass;
rndis_iad_descriptor.bFunctionProtocol = rndis_opts->protocol;

- /*
- * in drivers/usb/gadget/configfs.c:configfs_composite_bind()
- * configurations are bound in sequence with list_for_each_entry,
- * in each configuration its functions are bound in sequence
- * with list_for_each_entry, so we assume no race condition
- * with regard to rndis_opts->bound access
- */
+ mutex_lock(&rndis_opts->lock);
+ gether_set_gadget(rndis_opts->net, cdev->gadget);
if (!rndis_opts->bound) {
- gether_set_gadget(rndis_opts->net, cdev->gadget);
status = gether_register_netdev(rndis_opts->net);
- if (status)
- return status;
- rndis_opts->bound = true;
+ if (!status)
+ rndis_opts->bound = true;
}
+ mutex_unlock(&rndis_opts->lock);
+ if (status)
+ return status;

us = usb_gstrings_attach(cdev, rndis_strings,
ARRAY_SIZE(rndis_string_defs));
@@ -939,7 +935,9 @@ static void rndis_free(struct usb_function *f)

static void rndis_unbind(struct usb_configuration *c, struct usb_function *f)
{
- struct f_rndis *rndis = func_to_rndis(f);
+ struct f_rndis *rndis = func_to_rndis(f);
+ struct f_rndis_opts *opts = container_of(f->fi, struct f_rndis_opts,
+ func_inst);

kfree(f->os_desc_table);
f->os_desc_n = 0;
@@ -947,6 +945,8 @@ static void rndis_unbind(struct usb_configuration *c, struct usb_function *f)

kfree(rndis->notify_req->buf);
usb_ep_free_request(rndis->notify, rndis->notify_req);
+
+ gether_set_gadget(opts->net, NULL);
}

static struct usb_function *rndis_alloc(struct usb_function_instance *fi)
diff --git a/drivers/usb/gadget/function/f_subset.c b/drivers/usb/gadget/function/f_subset.c
index 111111111111..222222222222 100644
--- a/drivers/usb/gadget/function/f_subset.c
+++ b/drivers/usb/gadget/function/f_subset.c
@@ -308,15 +308,16 @@ geth_bind(struct usb_configuration *c, struct usb_function *f)
* with list_for_each_entry, so we assume no race condition
* with regard to gether_opts->bound access
*/
+ mutex_lock(&gether_opts->lock);
+ gether_set_gadget(gether_opts->net, cdev->gadget);
if (!gether_opts->bound) {
- mutex_lock(&gether_opts->lock);
- gether_set_gadget(gether_opts->net, cdev->gadget);
status = gether_register_netdev(gether_opts->net);
- mutex_unlock(&gether_opts->lock);
- if (status)
- return status;
- gether_opts->bound = true;
+ if (!status)
+ gether_opts->bound = true;
}
+ mutex_unlock(&gether_opts->lock);
+ if (status)
+ return status;

us = usb_gstrings_attach(cdev, geth_strings,
ARRAY_SIZE(geth_string_defs));
@@ -456,8 +457,13 @@ static void geth_free(struct usb_function *f)

static void geth_unbind(struct usb_configuration *c, struct usb_function *f)
{
+ struct f_gether_opts *opts = container_of(f->fi, struct f_gether_opts,
+ func_inst);
+
geth_string_defs[0].id = 0;
usb_free_all_descriptors(f);
+
+ gether_set_gadget(opts->net, NULL);
}

static struct usb_function *geth_alloc(struct usb_function_instance *fi)
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index 111111111111..222222222222 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -112,8 +112,10 @@ static void eth_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *p)

strscpy(p->driver, "g_ether", sizeof(p->driver));
strscpy(p->version, UETH__VERSION, sizeof(p->version));
- strscpy(p->fw_version, dev->gadget->name, sizeof(p->fw_version));
- strscpy(p->bus_info, dev_name(&dev->gadget->dev), sizeof(p->bus_info));
+ if (dev->gadget) {
+ strscpy(p->fw_version, dev->gadget->name, sizeof(p->fw_version));
+ strscpy(p->bus_info, dev_name(&dev->gadget->dev), sizeof(p->bus_info));
+ }
}

/* REVISIT can also support:
@@ -787,7 +789,6 @@ struct eth_dev *gether_setup_name(struct usb_gadget *g,
net->max_mtu = GETHER_MAX_MTU_SIZE;

dev->gadget = g;
- SET_NETDEV_DEV(net, &g->dev);
SET_NETDEV_DEVTYPE(net, &gadget_type);

status = register_netdev(net);
@@ -860,8 +861,6 @@ int gether_register_netdev(struct net_device *net)
struct usb_gadget *g;
int status;

- if (!net->dev.parent)
- return -EINVAL;
dev = netdev_priv(net);
g = dev->gadget;

@@ -892,7 +891,6 @@ void gether_set_gadget(struct net_device *net, struct usb_gadget *g)

dev = netdev_priv(net);
dev->gadget = g;
- SET_NETDEV_DEV(net, &g->dev);
}
EXPORT_SYMBOL_GPL(gether_set_gadget);

--
Armbian
Loading
Loading