From: Kuen-Han Tsai <khtsai@google.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>,
Faqiang Zhu <faqiang.zhu@nxp.com>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
Kuen-Han Tsai <khtsai@google.com>,
stable@vger.kernel.org
Subject: [PATCH 1/2] usb: gadget: u_ether: Fix NULL pointer deref in debug logging
Date: Wed, 23 Sep 2026 15:58:35 +0800 [thread overview]
Message-ID: <20260923-u-ether-gadget-npe-v1-1-ff466c55ecc0@google.com> (raw)
In-Reply-To: <20260923-u-ether-gadget-npe-v1-0-ff466c55ecc0@google.com>
Commit ec35c1969650 ("usb: gadget: f_ncm: Fix net_device lifecycle with
device_move") and its counterparts reparent the net_device to
/sys/devices/virtual during unbind and clear dev->gadget. However, the
DBG(), VDBG(), ERROR(), and INFO() macros from <linux/usb/composite.h>
dereference &dev->gadget->dev.
When dynamic debug or CONFIG_USB_GADGET_DEBUG is enabled, any logging on
the surviving net_device after unbind causes a NULL pointer dereference,
such as in eth_stop() during function instance teardown:
Unable to handle kernel NULL pointer dereference
Call trace:
dev_driver_string from __dynamic_dev_dbg+0x8c/0x118
__dynamic_dev_dbg from eth_stop+0x70/0x134 [u_ether]
...
unregister_netdev from gether_cleanup+0x14/0x28 [u_ether]
gether_cleanup [u_ether] from rndis_free_inst+0x2c/0x48 [usb_f_rndis]
Replace the composite.h logging macros in u_ether.c with the standard
netdev_*() helpers. Because dev->net remains valid for the entire
lifetime of struct eth_dev and netdev_printk() natively handles
unparented network devices, messages are logged safely both when
attached and when detached from the gadget.
Reported-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Closes: https://lore.kernel.org/all/89e19e6e-7ee7-4bb0-abd6-60971b7fd601@gmail.com/
Fixes: ec35c1969650 ("usb: gadget: f_ncm: Fix net_device lifecycle with device_move")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Kuen-Han Tsai <khtsai@google.com>
---
drivers/usb/gadget/function/u_ether.c | 61 +++++++++++++++++------------------
1 file changed, 30 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index 59d85d6a84a8..043b4ec80808 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -135,9 +135,9 @@ static void defer_kevent(struct eth_dev *dev, int flag)
if (test_and_set_bit(flag, &dev->todo))
return;
if (!schedule_work(&dev->work))
- ERROR(dev, "kevent %d may have been dropped\n", flag);
+ netdev_err(dev->net, "kevent %d may have been dropped\n", flag);
else
- DBG(dev, "kevent %d scheduled\n", flag);
+ netdev_dbg(dev->net, "kevent %d scheduled\n", flag);
}
static void rx_complete(struct usb_ep *ep, struct usb_request *req);
@@ -190,7 +190,7 @@ rx_submit(struct eth_dev *dev, struct usb_request *req, gfp_t gfp_flags)
skb = __netdev_alloc_skb(dev->net, size + NET_IP_ALIGN, gfp_flags);
if (skb == NULL) {
- DBG(dev, "no rx skb\n");
+ netdev_dbg(dev->net, "no rx skb\n");
goto enomem;
}
@@ -211,7 +211,7 @@ rx_submit(struct eth_dev *dev, struct usb_request *req, gfp_t gfp_flags)
enomem:
defer_kevent(dev, WORK_RX_MEMORY);
if (retval) {
- DBG(dev, "rx submit --> %d\n", retval);
+ netdev_dbg(dev->net, "rx submit --> %d\n", retval);
if (skb)
dev_kfree_skb_any(skb);
spin_lock_irqsave(&dev->req_lock, flags);
@@ -258,7 +258,7 @@ static void rx_complete(struct usb_ep *ep, struct usb_request *req)
|| skb2->len > GETHER_MAX_ETH_FRAME_LEN) {
dev->net->stats.rx_errors++;
dev->net->stats.rx_length_errors++;
- DBG(dev, "rx length %d\n", skb2->len);
+ netdev_dbg(dev->net, "rx length %d\n", skb2->len);
dev_kfree_skb_any(skb2);
goto next_frame;
}
@@ -278,12 +278,12 @@ static void rx_complete(struct usb_ep *ep, struct usb_request *req)
/* software-driven interface shutdown */
case -ECONNRESET: /* unlink */
case -ESHUTDOWN: /* disconnect etc */
- VDBG(dev, "rx shutdown, code %d\n", status);
+ netdev_vdbg(dev->net, "rx shutdown, code %d\n", status);
goto quiesce;
/* for hardware automagic (such as pxa) */
case -ECONNABORTED: /* endpoint reset */
- DBG(dev, "rx %s reset\n", ep->name);
+ netdev_dbg(dev->net, "rx %s reset\n", ep->name);
defer_kevent(dev, WORK_RX_MEMORY);
quiesce:
dev_kfree_skb_any(skb);
@@ -296,7 +296,7 @@ static void rx_complete(struct usb_ep *ep, struct usb_request *req)
default:
dev->net->stats.rx_errors++;
- DBG(dev, "rx status %d\n", status);
+ netdev_dbg(dev->net, "rx status %d\n", status);
break;
}
@@ -365,7 +365,7 @@ static int alloc_requests(struct eth_dev *dev, struct gether *link, unsigned n)
goto fail;
goto done;
fail:
- DBG(dev, "can't alloc requests\n");
+ netdev_dbg(dev->net, "can't alloc requests\n");
done:
spin_unlock(&dev->req_lock);
return status;
@@ -403,7 +403,7 @@ static void eth_work(struct work_struct *work)
}
if (dev->todo)
- DBG(dev, "work done, flags = 0x%lx\n", dev->todo);
+ netdev_dbg(dev->net, "work done, flags = 0x%lx\n", dev->todo);
}
static void tx_complete(struct usb_ep *ep, struct usb_request *req)
@@ -414,7 +414,7 @@ static void tx_complete(struct usb_ep *ep, struct usb_request *req)
switch (req->status) {
default:
dev->net->stats.tx_errors++;
- VDBG(dev, "tx err %d\n", req->status);
+ netdev_vdbg(dev->net, "tx err %d\n", req->status);
fallthrough;
case -ECONNRESET: /* unlink */
case -ESHUTDOWN: /* disconnect etc */
@@ -475,7 +475,7 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
}
if (dev->port_usb && dev->port_usb->is_suspend) {
- DBG(dev, "Port suspended. Triggering wakeup\n");
+ netdev_dbg(dev->net, "Port suspended. Triggering wakeup\n");
netif_stop_queue(net);
spin_unlock_irqrestore(&dev->lock, flags);
ether_wakeup_host(dev->port_usb);
@@ -579,7 +579,7 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
retval = usb_ep_queue(in, req, GFP_ATOMIC);
switch (retval) {
default:
- DBG(dev, "tx queue err %d\n", retval);
+ netdev_dbg(dev->net, "tx queue err %d\n", retval);
break;
case 0:
netif_trans_update(net);
@@ -604,7 +604,7 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
static void eth_start(struct eth_dev *dev, gfp_t gfp_flags)
{
- DBG(dev, "%s\n", __func__);
+ netdev_dbg(dev->net, "%s\n", __func__);
/* fill the rx queue */
rx_fill(dev, gfp_flags);
@@ -619,7 +619,7 @@ static int eth_open(struct net_device *net)
struct eth_dev *dev = netdev_priv(net);
struct gether *link;
- DBG(dev, "%s\n", __func__);
+ netdev_dbg(dev->net, "%s\n", __func__);
if (netif_carrier_ok(dev->net))
eth_start(dev, GFP_KERNEL);
@@ -637,13 +637,12 @@ static int eth_stop(struct net_device *net)
struct eth_dev *dev = netdev_priv(net);
unsigned long flags;
- VDBG(dev, "%s\n", __func__);
+ netdev_vdbg(dev->net, "%s\n", __func__);
netif_stop_queue(net);
- DBG(dev, "stop stats: rx/tx %ld/%ld, errs %ld/%ld\n",
- dev->net->stats.rx_packets, dev->net->stats.tx_packets,
- dev->net->stats.rx_errors, dev->net->stats.tx_errors
- );
+ netdev_dbg(dev->net, "stop stats: rx/tx %ld/%ld, errs %ld/%ld\n",
+ dev->net->stats.rx_packets, dev->net->stats.tx_packets,
+ dev->net->stats.rx_errors, dev->net->stats.tx_errors);
/* ensure there are no more active requests */
spin_lock_irqsave(&dev->lock, flags);
@@ -669,7 +668,7 @@ static int eth_stop(struct net_device *net)
usb_ep_disable(link->in_ep);
usb_ep_disable(link->out_ep);
if (netif_carrier_ok(net)) {
- DBG(dev, "host still using in/out endpoints\n");
+ netdev_dbg(dev->net, "host still using in/out endpoints\n");
link->in_ep->desc = in;
link->out_ep->desc = out;
usb_ep_enable(link->in_ep);
@@ -799,8 +798,8 @@ struct eth_dev *gether_setup_name(struct usb_gadget *g,
free_netdev(net);
dev = ERR_PTR(status);
} else {
- INFO(dev, "MAC %pM\n", net->dev_addr);
- INFO(dev, "HOST MAC %pM\n", dev->host_mac);
+ netdev_info(net, "MAC %pM\n", net->dev_addr);
+ netdev_info(net, "HOST MAC %pM\n", dev->host_mac);
/*
* two kinds of host-initiated state changes:
@@ -875,8 +874,8 @@ int gether_register_netdev(struct net_device *net)
dev_dbg(&g->dev, "register_netdev failed, %d\n", status);
return status;
} else {
- INFO(dev, "HOST MAC %pM\n", dev->host_mac);
- INFO(dev, "MAC %pM\n", dev->dev_mac);
+ netdev_info(net, "HOST MAC %pM\n", dev->host_mac);
+ netdev_info(net, "MAC %pM\n", dev->dev_mac);
/* two kinds of host-initiated state changes:
* - iff DATA transfer is active, carrier is "on"
@@ -1147,16 +1146,16 @@ struct net_device *gether_connect(struct gether *link)
link->in_ep->driver_data = dev;
result = usb_ep_enable(link->in_ep);
if (result != 0) {
- DBG(dev, "enable %s --> %d\n",
- link->in_ep->name, result);
+ netdev_dbg(dev->net, "enable %s --> %d\n",
+ link->in_ep->name, result);
goto fail0;
}
link->out_ep->driver_data = dev;
result = usb_ep_enable(link->out_ep);
if (result != 0) {
- DBG(dev, "enable %s --> %d\n",
- link->out_ep->name, result);
+ netdev_dbg(dev->net, "enable %s --> %d\n",
+ link->out_ep->name, result);
goto fail1;
}
@@ -1167,7 +1166,7 @@ struct net_device *gether_connect(struct gether *link)
if (result == 0) {
dev->zlp = link->is_zlp_ok;
dev->no_skb_reserve = gadget_avoids_skb_reserve(dev->gadget);
- DBG(dev, "qlen %d\n", qlen(dev->gadget, dev->qmult));
+ netdev_dbg(dev->net, "qlen %d\n", qlen(dev->gadget, dev->qmult));
dev->header_len = link->header_len;
dev->unwrap = link->unwrap;
@@ -1223,7 +1222,7 @@ void gether_disconnect(struct gether *link)
if (!dev)
return;
- DBG(dev, "%s\n", __func__);
+ netdev_dbg(dev->net, "%s\n", __func__);
spin_lock(&dev->lock);
dev->port_usb = NULL;
--
2.55.0.1082.g2b9226bbc0-goog
next prev parent reply other threads:[~2026-09-23 7:59 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-23 7:58 [PATCH 0/2] usb: gadget: u_ether: Fix NULL pointer dereferences after gadget unbind Kuen-Han Tsai
2026-09-23 7:58 ` Kuen-Han Tsai [this message]
2026-09-23 7:58 ` [PATCH 2/2] usb: gadget: u_ether: Protect dev->gadget access with dev->lock Kuen-Han Tsai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260923-u-ether-gadget-npe-v1-1-ff466c55ecc0@google.com \
--to=khtsai@google.com \
--cc=faqiang.zhu@nxp.com \
--cc=gregkh@linuxfoundation.org \
--cc=ivo.g.dimitrov.75@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®