* [PATCH v2 1/2] net: usb: cdc_ether: add NCSI passthrough support
2026-09-08 13:01 [PATCH v2 0/2] net: add USB CDC Ethernet NCSI support and fix unregister UAF Potin Lai
@ 2026-09-08 13:01 ` Potin Lai
2026-09-09 18:47 ` Andrew Lunn
2026-09-08 13:01 ` [PATCH v2 2/2] net/ncsi: fix use-after-free in ncsi_unregister_dev() Potin Lai
1 sibling, 1 reply; 4+ messages in thread
From: Potin Lai @ 2026-09-08 13:01 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Oliver Neukum, Samuel Mendoza-Jonas, Paul Fertser,
Simon Horman
Cc: Potin Lai, linux-usb, netdev, linux-kernel, Cosmo Chou,
Mike Hsieh, Mik Lin, Potin Lai, Adrian Ambrozewicz
From: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
Add NCSI (Network Controller Sideband Interface) passthrough support for
USB CDC Ethernet devices. This enables BMC-to-host sideband management
over USB, typically used in DPU platforms where the BMC communicates
with the DPU via a dedicated USB connection.
Key implementation details:
- Register with NCSI subsystem in ndo_open, unregister in ndo_stop
- Override netdev_ops to hook open/stop for NCSI lifecycle management
- Keep carrier always on while interface is up, as NCSI control traffic
shares the USB data path (unlike PHY-based drivers)
- Ignore CDC status notifications since NCSI manages link state
- Forward VLAN operations to NCSI subsystem
The symmetric open/stop lifecycle is critical for USB drivers:
open() -> ncsi_register_dev() + ncsi_start_dev()
stop() -> ncsi_stop_dev() + ncsi_unregister_dev()
This ensures NCSI packet handlers are removed BEFORE unregister_netdev()
checks for them during USB disconnect, avoiding kernel crashes. Unlike
platform drivers where unbind() runs before unregister_netdev(), USB
drivers have the opposite order:
usbnet_disconnect() -> unregister_netdev() -> unbind()
Placing NCSI cleanup in unbind() would be too late.
Supported hardware: NVIDIA DPU USB CDC Ethernet (VID:PID 0955:cf11)
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
Signed-off-by: Potin Lai <potin.lai.pt@gmail.com>
---
drivers/net/usb/Kconfig | 20 +++++
drivers/net/usb/cdc_ether.c | 187 +++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 206 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 52a5c0922c79..00757392128f 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -241,6 +241,26 @@ config USB_NET_CDCETHER
IEEE 802 "local assignment" bit is set in the address, a "usbX"
name is used instead.
+config USB_NET_CDCETHER_NCSI
+ bool "NCSI passthrough support for CDC Ethernet"
+ depends on USB_NET_CDCETHER
+ depends on NET_NCSI
+ help
+ This option enables NCSI (Network Controller Sideband Interface)
+ passthrough support for specific USB CDC Ethernet devices.
+
+ NCSI allows a BMC (Baseboard Management Controller) to share a
+ network interface with the host system for out-of-band management.
+ This is typically used in DPU (Data Processing Unit) platforms
+ where the BMC communicates with the DPU via a dedicated USB
+ connection.
+
+ Currently supported devices:
+ * NVIDIA BlueField DPU (VID:PID 0955:cf11)
+
+ Say Y here if you have a system with USB-based NCSI connectivity
+ between BMC and host. If unsure, say N.
+
config USB_NET_CDC_EEM
tristate "CDC EEM support"
depends on USB_USBNET
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index a0a5740590b9..0a0eb9e0575e 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -538,6 +538,165 @@ static const struct driver_info cdc_info = {
.manage_power = usbnet_manage_power,
};
+/*
+ * NCSI passthrough support for USB CDC Ethernet devices.
+ *
+ * Enables BMC-to-host sideband management over USB, typically used in
+ * DPU (Data Processing Unit) platforms where the BMC communicates with
+ * the DPU via a dedicated USB connection.
+ */
+#ifdef CONFIG_USB_NET_CDCETHER_NCSI
+#include <net/ncsi.h>
+
+/* NCSI operates at 100 Mbps */
+#define NCSI_SPEED_BPS (100 * 1000000)
+
+struct cdc_ncsi_priv {
+ struct ncsi_dev *ndev;
+ struct net_device_ops netdev_ops;
+ const struct net_device_ops *orig_netdev_ops;
+};
+
+static void cdc_ncsi_handler(struct ncsi_dev *nd)
+{
+ if (unlikely(nd->state != ncsi_dev_state_functional))
+ return;
+
+ netdev_dbg(nd->dev, "NCSI interface %s\n",
+ nd->link_up ? "up" : "down");
+
+ /* Don't toggle carrier here - it must stay on for NCSI to
+ * communicate over USB. Carrier was enabled in cdc_ncsi_open().
+ */
+}
+
+static int cdc_ncsi_open(struct net_device *net)
+{
+ struct usbnet *dev = netdev_priv(net);
+ struct cdc_ncsi_priv *priv = dev->driver_priv;
+ int ret;
+
+ ret = usbnet_open(net);
+ if (ret)
+ return ret;
+
+ priv->ndev = ncsi_register_dev(net, cdc_ncsi_handler);
+ if (!priv->ndev) {
+ netdev_err(net, "failed to register NCSI device\n");
+ usbnet_stop(net);
+ return -ENODEV;
+ }
+
+ /* Carrier must stay on for NCSI to transmit/receive its control
+ * packets over USB. Unlike PHY-based drivers, we cannot toggle
+ * carrier based on NCSI link state without breaking USB I/O.
+ */
+ netif_carrier_on(net);
+ ret = ncsi_start_dev(priv->ndev);
+ if (ret) {
+ netdev_err(net, "failed to start NCSI: %d\n", ret);
+ ncsi_unregister_dev(priv->ndev);
+ priv->ndev = NULL;
+ netif_carrier_off(net);
+ usbnet_stop(net);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int cdc_ncsi_stop(struct net_device *net)
+{
+ struct usbnet *dev = netdev_priv(net);
+ struct cdc_ncsi_priv *priv = dev->driver_priv;
+
+ if (priv->ndev) {
+ ncsi_stop_dev(priv->ndev);
+ ncsi_unregister_dev(priv->ndev);
+ priv->ndev = NULL;
+ }
+
+ netif_carrier_off(net);
+ return usbnet_stop(net);
+}
+
+static int cdc_ncsi_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+ struct cdc_ncsi_priv *priv;
+ struct cdc_state *info;
+ int status;
+
+ status = usbnet_ether_cdc_bind(dev, intf);
+ if (status < 0)
+ return status;
+
+ info = (void *)&dev->data;
+ status = usbnet_get_ethernet_addr(dev, info->ether->iMACAddress);
+ if (status < 0)
+ goto err_unbind;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ status = -ENOMEM;
+ goto err_unbind;
+ }
+
+ dev->driver_priv = priv;
+
+ /* Override netdev_ops for NCSI lifecycle management */
+ priv->orig_netdev_ops = dev->net->netdev_ops;
+ priv->netdev_ops = *dev->net->netdev_ops;
+ priv->netdev_ops.ndo_open = cdc_ncsi_open;
+ priv->netdev_ops.ndo_stop = cdc_ncsi_stop;
+ priv->netdev_ops.ndo_vlan_rx_add_vid = ncsi_vlan_rx_add_vid;
+ priv->netdev_ops.ndo_vlan_rx_kill_vid = ncsi_vlan_rx_kill_vid;
+ dev->net->netdev_ops = &priv->netdev_ops;
+
+ dev->net->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+ dev->net->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+
+ dev->rx_speed = NCSI_SPEED_BPS;
+ dev->tx_speed = NCSI_SPEED_BPS;
+
+ netdev_info(dev->net, "NCSI passthrough enabled\n");
+ return 0;
+
+err_unbind:
+ usb_set_intfdata(info->data, NULL);
+ usb_driver_release_interface(driver_of(intf), info->data);
+ return status;
+}
+
+static void cdc_ncsi_unbind(struct usbnet *dev, struct usb_interface *intf)
+{
+ struct cdc_ncsi_priv *priv = dev->driver_priv;
+
+ if (priv) {
+ /* Restore original netdev_ops before freeing priv */
+ dev->net->netdev_ops = priv->orig_netdev_ops;
+ kfree(priv);
+ dev->driver_priv = NULL;
+ }
+
+ usbnet_cdc_unbind(dev, intf);
+}
+
+static void cdc_ncsi_status(struct usbnet *dev, struct urb *urb)
+{
+ /* NCSI manages link state, ignore CDC status notifications */
+}
+
+static const struct driver_info cdc_ncsi_info = {
+ .description = "CDC Ethernet Device (NCSI)",
+ .flags = FLAG_ETHER | FLAG_POINTTOPOINT,
+ .bind = cdc_ncsi_bind,
+ .unbind = cdc_ncsi_unbind,
+ .status = cdc_ncsi_status,
+ .set_rx_mode = usbnet_cdc_update_filter,
+ .manage_power = usbnet_manage_power,
+};
+#endif /* CONFIG_USB_NET_CDCETHER_NCSI */
+
static const struct driver_info zte_cdc_info = {
.description = "ZTE CDC Ethernet Device",
.flags = FLAG_ETHER | FLAG_POINTTOPOINT,
@@ -946,7 +1105,33 @@ static const struct usb_device_id products[] = {
USB_CDC_SUBCLASS_ETHERNET,
USB_CDC_PROTO_NONE),
.driver_info = (unsigned long)&wwan_info,
-}, {
+},
+/*
+ * NCSI passthrough support.
+ *
+ * This implementation enables NCSI unconditionally for matching VID/PID.
+ * Per-driver integration is required because the NCSI subsystem mandates
+ * explicit lifecycle calls (ncsi_register/start/stop/unregister_dev).
+ *
+ * OPEN QUESTION: An alternative approach using DTS "use-ncsi" property
+ * for conditional enablement was considered. This is viable only when
+ * USB topology is fixed and known at build time. Whether DPU deployments
+ * have fixed topologies remains to be determined. Note that DTS-based
+ * control would still require per-driver integration.
+ *
+ * A future generic solution could eliminate per-driver modifications by
+ * extending the NCSI subsystem to hook netdev lifecycle events directly,
+ * with interface selection configured via DTS or sysfs.
+ */
+#ifdef CONFIG_USB_NET_CDCETHER_NCSI
+{
+ USB_DEVICE_AND_INTERFACE_INFO(NVIDIA_VENDOR_ID, 0xcf11,
+ USB_CLASS_COMM, USB_CDC_SUBCLASS_ETHERNET,
+ USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long)&cdc_ncsi_info,
+},
+#endif /* CONFIG_USB_NET_CDCETHER_NCSI */
+{
USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_ETHERNET,
USB_CDC_PROTO_NONE),
.driver_info = (unsigned long) &cdc_info,
--
2.52.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v2 2/2] net/ncsi: fix use-after-free in ncsi_unregister_dev()
2026-09-08 13:01 [PATCH v2 0/2] net: add USB CDC Ethernet NCSI support and fix unregister UAF Potin Lai
2026-09-08 13:01 ` [PATCH v2 1/2] net: usb: cdc_ether: add NCSI passthrough support Potin Lai
@ 2026-09-08 13:01 ` Potin Lai
1 sibling, 0 replies; 4+ messages in thread
From: Potin Lai @ 2026-09-08 13:01 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Oliver Neukum, Samuel Mendoza-Jonas, Paul Fertser,
Simon Horman
Cc: Potin Lai, linux-usb, netdev, linux-kernel, Cosmo Chou,
Mike Hsieh, Mik Lin, Potin Lai, Adrian Ambrozewicz
From: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
ncsi_unregister_dev() frees the ncsi_dev_priv structure while timers
and workqueue may still be accessing it, causing use-after-free.
The problem involves two async mechanisms:
1. Request timers (ncsi_request_timeout) - fire when NCSI responses
are not received in time
2. Workqueue (ncsi_dev_work) - processes NCSI state machine
These can cascade: timer handlers call ncsi_free_request() which may
call schedule_work(), and work can send commands that arm new timers.
The fix adds proper synchronization before kfree():
dev_remove_pack() - stop packet reception
timer_delete_sync() x 256 - cancel all request timers
cancel_work_sync() - wait for workqueue to complete
kfree(ndp)
Order matters: timers must be cancelled before work because timer
handlers may schedule new work via ncsi_free_request().
Note: ncsi_dev_work() is non-blocking - it sends a command, arms a
timer, and returns immediately. It does not wait for timer completion.
The timer firing later triggers schedule_work() for the next state.
So cancel_work_sync() will not hang waiting for cancelled timers.
This relies on ncsi_stop_dev() being called first (guaranteed by the
network device lifecycle). ncsi_stop_dev() sets state to
ncsi_dev_state_functional, which causes ncsi_dev_work() to exit
immediately without sending commands or arming timers. This breaks
the timer<->work cycle and ensures the synchronization terminates.
Timeline showing the race (without fix):
CPU 0 (unregister) CPU 1 (async)
------------------ -------------
ncsi_unregister_dev()
dev_remove_pack()
ncsi_request_timeout()
ncsi_free_request()
schedule_work()
kfree(ndp)
ncsi_dev_work()
ndp->... <- UAF!
With fix:
CPU 0 (unregister) CPU 1 (async)
------------------ -------------
ncsi_unregister_dev()
dev_remove_pack()
timer_delete_sync() x 256 <- waits for timer handlers
ncsi_request_timeout()
ncsi_free_request()
schedule_work()
cancel_work_sync() <- waits for work
ncsi_dev_work()
state=0x100, exits immediately
kfree(ndp) <- safe
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
Signed-off-by: Potin Lai <potin.lai.pt@gmail.com>
---
net/ncsi/ncsi-manage.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 54d0df0a9efe..15d7e4cabf4f 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1957,9 +1957,28 @@ void ncsi_unregister_dev(struct ncsi_dev *nd)
struct ncsi_dev_priv *ndp = TO_NCSI_DEV_PRIV(nd);
struct ncsi_package *np, *tmp;
unsigned long flags;
+ int i;
dev_remove_pack(&ndp->ptype);
+ /*
+ * Synchronize with async operations before freeing ndp.
+ *
+ * Note: The caller must have called ncsi_stop_dev() first, which
+ * sets nd->state to ncsi_dev_state_functional (0x100). This causes
+ * any running or scheduled ncsi_dev_work() to exit immediately
+ * without sending commands or arming new timers, breaking the
+ * potential cycle of: work -> arm timer -> timer -> schedule work.
+ *
+ * Order matters:
+ * 1. timer_delete_sync() - cancel timers, handlers may schedule work
+ * 2. cancel_work_sync() - cancel work scheduled by timer handlers
+ */
+ for (i = 0; i < ARRAY_SIZE(ndp->requests); i++)
+ timer_delete_sync(&ndp->requests[i].timer);
+
+ cancel_work_sync(&ndp->work);
+
list_for_each_entry_safe(np, tmp, &ndp->packages, node)
ncsi_remove_package(np);
--
2.52.0
^ permalink raw reply related [flat|nested] 4+ messages in thread