[PATCH 0/2] net: add USB CDC Ethernet NCSI support and fix unregister UAF

This series introduces NCSI (Network Controller Sideband Interface)
passthrough support for USB CDC Ethernet devices and fixes a
use-after-free race condition in the NCSI core unregistration path.

In DPU (Data Processing Unit) platforms such as the NVIDIA BlueField
series, the Baseboard Management Controller (BMC) communicates with the
host or DPU via a dedicated USB CDC Ethernet connection for out-of-band
management traffic.

Unlike traditional platform Ethernet devices where NCSI is initialized
statically at probe time, USB devices require dynamic lifecycle
management within ndo_open() and ndo_stop():

1. NCSI control packets share the USB data path, requiring the link
   carrier to remain enabled while the interface is up.
2. In USB drivers, usbnet_disconnect() invokes unregister_netdev()
   before unbind(). Performing NCSI registration in ndo_open() and
   cleanup in ndo_stop() ensures NCSI packet handlers are removed before
   netdevice teardown occurs.
3. Dynamic unregistration of NCSI devices revealed a race in the NCSI
   core: ncsi_unregister_dev() freed the ncsi_dev_priv structure while
   asynchronous request timers and workqueue items were still active.

Signed-off-by: Potin Lai <redacted>
---
Adrian Ambrozewicz (2):
      net: usb: cdc_ether: add NCSI passthrough support
      net/ncsi: fix use-after-free in ncsi_unregister_dev()

 drivers/net/usb/Kconfig     |  20 +++++
 drivers/net/usb/cdc_ether.c | 190 +++++++++++++++++++++++++++++++++++++++++++-
 net/ncsi/ncsi-manage.c      |  19 +++++
 3 files changed, 228 insertions(+), 1 deletion(-)
---
base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
change-id: 20260907-ncsi-over-usb-3e786f4686c8

Best regards,
--  
Potin Lai [off-list ref]

From: Adrian Ambrozewicz <redacted>

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 <redacted>
Signed-off-by: Potin Lai <redacted>
---
 drivers/net/usb/Kconfig     |  20 +++++
 drivers/net/usb/cdc_ether.c | 190 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 209 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..e5d3924e6a2e 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c

@@ -538,6 +538,168 @@ 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 int cdc_ncsi_open(struct net_device *net);
+static int cdc_ncsi_stop(struct net_device *net);
+
+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_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 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 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 +1108,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

From: Adrian Ambrozewicz <redacted>

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
  del_timer_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()
    del_timer_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 <redacted>
Signed-off-by: Potin Lai <redacted>
---
 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..dc5f2a76a0a5 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. del_timer_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++)
+		del_timer_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

+/* NCSI operates at 100 Mbps */
+#define NCSI_SPEED_BPS		(100 * 1000000)

Why 100Mbps? RGMII is often used, running at 1G.

No forward declarations. Move the code around so they are not needed.

   Andrew

On Mon, Sep 07, 2026 at 08:15:54PM +0800, Potin Lai wrote:

This series introduces NCSI (Network Controller Sideband Interface)
passthrough support for USB CDC Ethernet devices and fixes a
use-after-free race condition in the NCSI core unregistration path.

In DPU (Data Processing Unit) platforms such as the NVIDIA BlueField
series, the Baseboard Management Controller (BMC) communicates with the
host or DPU via a dedicated USB CDC Ethernet connection for out-of-band
management traffic.

I'm confused with the architecture here. What normally happens is the
BMC has a standard Ethernet interface which outputs RGMII. That goes
into what is in effect a little 3 port switch in the host
interface. That switch allows packets to flow to the host, to the BMC,
or out the PHY to the medium.

Because the PHY is in effect being shared by two interfaces,
management of that PHY becomes "Interesting". You don't want either of
the interfaces setting the PHY down because it would cut the other off
from the medium.

What is the architecture then USB is used?

     Andrew

On Tue, Sep 8, 2026 at 2:23 AM Andrew Lunn [off-list ref] wrote:

On Mon, Sep 07, 2026 at 08:15:54PM +0800, Potin Lai wrote:

quoted

This series introduces NCSI (Network Controller Sideband Interface)
passthrough support for USB CDC Ethernet devices and fixes a
use-after-free race condition in the NCSI core unregistration path.

In DPU (Data Processing Unit) platforms such as the NVIDIA BlueField
series, the Baseboard Management Controller (BMC) communicates with the
host or DPU via a dedicated USB CDC Ethernet connection for out-of-band
management traffic.

I'm confused with the architecture here. What normally happens is the
BMC has a standard Ethernet interface which outputs RGMII. That goes
into what is in effect a little 3 port switch in the host
interface. That switch allows packets to flow to the host, to the BMC,
or out the PHY to the medium.

Because the PHY is in effect being shared by two interfaces,
management of that PHY becomes "Interesting". You don't want either of
the interfaces setting the PHY down because it would cut the other off
from the medium.

What is the architecture then USB is used?

     Andrew

Hi Andrew,

Thanks for the review. To clarify the architecture, it looks like this:

[ BMC ] <--(USB)--> [ SMA Controller ] <--(Internal)--> [ Shared NIC (CX9) ]

1. The USB Interface: Between the BMC and the shared NIC, there is an
   SMA controller. This controller emulates a standard USB CDC Ethernet
   device facing the BMC.

2. Passthrough (Bypass): The SMA controller acts as a transparent bridge.
   It simply passes through both standard network packets and NCSI
   control packets between the BMC's USB interface and the actual
   shared NIC.

3. PHY Management: Because of this architecture, the actual 3-port switch
   logic and the physical PHY management are handled entirely by the
   NIC/SMA hardware firmware on the DPU side.

From the BMC's perspective, it only sees a point-to-point USB CDC Ethernet
device. The BMC driver does not have direct access to manage the shared
PHY's link state.

Best regards,
Potin

On Tue, Sep 8, 2026 at 2:17 AM Andrew Lunn [off-list ref] wrote:

quoted

+/* NCSI operates at 100 Mbps */
+#define NCSI_SPEED_BPS               (100 * 1000000)

Why 100Mbps? RGMII is often used, running at 1G.

Since the original NC-SI specification was designed based on the
RMII protocol, simply set the speed to 100 Mbps as the default
for this virtual USB link.

quoted

+struct cdc_ncsi_priv {
+     struct ncsi_dev *ndev;
+     struct net_device_ops netdev_ops;
+     const struct net_device_ops *orig_netdev_ops;
+};
+
+static int cdc_ncsi_open(struct net_device *net);
+static int cdc_ncsi_stop(struct net_device *net);

No forward declarations. Move the code around so they are not needed.

Thanks for the suggestion, I will move the code to remove forward declarations.

   Andrew

Best regards,
Potin

On Tue, Sep 08, 2026 at 08:45:36PM +0800, Potin Lai wrote:

On Tue, Sep 8, 2026 at 2:17 AM Andrew Lunn [off-list ref] wrote:

quoted

quoted

+/* NCSI operates at 100 Mbps */
+#define NCSI_SPEED_BPS               (100 * 1000000)

Why 100Mbps? RGMII is often used, running at 1G.

Since the original NC-SI specification was designed based on the
RMII protocol, simply set the speed to 100 Mbps as the default
for this virtual USB link.

How is this used? You could look at the USB link and return 1.5Mbps,
12Mbps, 480MBps, 5Gbps, 10Gbps, ...

	Andrew

Hi Andrew,

Thanks for the review. To clarify the architecture, it looks like this:

[ BMC ] <--(USB)--> [ SMA Controller ] <--(Internal)--> [ Shared NIC (CX9) ]

1. The USB Interface: Between the BMC and the shared NIC, there is an
   SMA controller. This controller emulates a standard USB CDC Ethernet
   device facing the BMC.

2. Passthrough (Bypass): The SMA controller acts as a transparent bridge.
   It simply passes through both standard network packets and NCSI
   control packets between the BMC's USB interface and the actual
   shared NIC.

3. PHY Management: Because of this architecture, the actual 3-port switch
   logic and the physical PHY management are handled entirely by the
   NIC/SMA hardware firmware on the DPU side.

This is the first time i've seen this sort of setup. You should
explain this in the commit message, because it is probably new to
others as well.

quoted

From the BMC's perspective, it only sees a point-to-point USB CDC Ethernet

device. The BMC driver does not have direct access to manage the shared
PHY's link state.

So why is the SMA controller emulator actually reporting the true link
state? Since it is an emulator, why not just say the link is always
up? And ignore any requests from the BMC to change its state? You then
don't need any quirks in the CDC driver. It seems silly to emulator
something and not actually use the fact it is an emulation to hide
away differences to a real device.

      Andrew