[PATCH net v3 0/2] net: sparx5: misc fixes for sparx5 and lan969x

[PATCH net v3 0/2] net: sparx5: misc fixes for sparx5 and lan969x

[Daniel Machon]

This series fixes various issues in the sparx5 driver, which also
serves lan969x.

Details are in the individual commit descriptions.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
Changes in v3:
- Patch #1: retitled to net: microchip: vcap:, since the fix lives in
  the shared VCAP debugfs helper; name the VCAP debugfs entry after
  the stable port index instead of the netdev name, which can change
  at runtime (Jakub). Added as a new vcap_port_debugfs_portno()
  helper, so lan966x's existing per-port debugfs files are untouched.
- Patch #2: update sparx5_set_rx_mode() to return int, matching
  .ndo_set_rx_mode_async's return type, and propagate
  __hw_addr_sync_dev()'s return value instead of always returning 0
- Drop patches #3 and #4, already merged upstream
- Link to v2: https://lore.kernel.org/r/20260506-misc-fixes-sparx5-lan969x-v2-0-fb236aa96908@microchip.com

Changes in v2:
- Rework patch #2 to use ndo_set_rx_mode_async (Jakub)
- Link to v1: https://lore.kernel.org/r/20260504-misc-fixes-sparx5-lan969x-v1-0-6604306b5743@microchip.com

---
Daniel Machon (2):
      net: microchip: vcap: use port number instead of netdev name for debugfs
      net: sparx5: fix sleep in atomic context in MAC table access

 .../net/ethernet/microchip/sparx5/sparx5_netdev.c   | 11 ++++++++---
 .../ethernet/microchip/sparx5/sparx5_vcap_impl.c    |  5 +++--
 .../net/ethernet/microchip/vcap/vcap_api_debugfs.c  | 21 +++++++++++++++++++++
 .../net/ethernet/microchip/vcap/vcap_api_debugfs.h  | 14 ++++++++++++++
 4 files changed, 46 insertions(+), 5 deletions(-)
---
base-commit: e6a5d573d24cd375e09d24f136523cb3cc85c9d3
change-id: 20260428-misc-fixes-sparx5-lan969x-bc2961a570fb

Best regards,
-- 
Daniel Machon <daniel.machon@microchip.com>

[PATCH net v3 1/2] net: microchip: vcap: use port number instead of netdev name for debugfs

[Daniel Machon]

sparx5_vcap_init() runs before sparx5_register_netdevs() in probe, and
its debugfs setup calls vcap_port_debugfs() for every port using
netdev_name(ndev) as the debugfs file name. At that point the netdevs
have only been allocated, not registered, so dev->name still holds the
"eth%d" template and netdev_name() returns "(unnamed net_device)".
Every port tries to create the same file under vcaps/, producing a
flood of warnings at boot:

  debugfs: '(unnamed net_device)' already exists in 'vcaps'
  debugfs: '(unnamed net_device)' already exists in 'vcaps'
  ...

Add vcap_port_debugfs_portno(), a variant of vcap_port_debugfs() that
takes the port's stable hardware port number and uses "p%u" as the
debugfs file name instead of netdev_name(ndev). This makes the file
name independent of registration order; the file still stores and
later dereferences the netdev itself, same as before. sparx5 already
reports the same "p%d" string via ndo_get_phys_port_name(), so the
debugfs name now matches that.

Only sparx5 (and lan969x, which shares this code) is switched to the
new function. lan966x keeps calling vcap_port_debugfs() unchanged, so
this fix does not rename any of its existing debugfs files.

Fixes: b8909aad5b8d ("net: sparx5: move netdev and notifier block registration to probe")
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 .../ethernet/microchip/sparx5/sparx5_vcap_impl.c    |  5 +++--
 .../net/ethernet/microchip/vcap/vcap_api_debugfs.c  | 21 +++++++++++++++++++++
 .../net/ethernet/microchip/vcap/vcap_api_debugfs.h  | 14 ++++++++++++++
 3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c b/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
index cf332de6bf73..2dee2ce19fce 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
@@ -2077,8 +2077,9 @@ int sparx5_vcap_init(struct sparx5 *sparx5)
 	dir = vcap_debugfs(sparx5->dev, sparx5->debugfs_root, ctrl);
 	for (idx = 0; idx < consts->n_ports; ++idx)
 		if (sparx5->ports[idx])
-			vcap_port_debugfs(sparx5->dev, dir, ctrl,
-					  sparx5->ports[idx]->ndev);
+			vcap_port_debugfs_portno(sparx5->dev, dir, ctrl,
+						 sparx5->ports[idx]->ndev,
+						 sparx5->ports[idx]->portno);
 
 	return err;
 }
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
index e0c65c7ab23e..476f7496a9d4 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
@@ -400,6 +400,27 @@ void vcap_port_debugfs(struct device *dev, struct dentry *parent,
 }
 EXPORT_SYMBOL_GPL(vcap_port_debugfs);
 
+void vcap_port_debugfs_portno(struct device *dev,
+			      struct dentry *parent,
+			      struct vcap_control *vctrl,
+			      struct net_device *ndev,
+			      unsigned int portno)
+{
+	struct vcap_port_debugfs_info *info;
+	char name[16];
+
+	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return;
+
+	info->vctrl = vctrl;
+	info->ndev = ndev;
+
+	snprintf(name, sizeof(name), "p%u", portno);
+	debugfs_create_file(name, 0444, parent, info, &vcap_port_debugfs_fops);
+}
+EXPORT_SYMBOL_GPL(vcap_port_debugfs_portno);
+
 /* Show the full VCAP instance data (rules with all fields) */
 static int vcap_debugfs_show(struct seq_file *m, void *unused)
 {
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.h b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.h
index 9f2c59b5f6f5..7dc6e3411a4d 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.h
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.h
@@ -18,6 +18,12 @@ void vcap_port_debugfs(struct device *dev, struct dentry *parent,
 		       struct vcap_control *vctrl,
 		       struct net_device *ndev);
 
+void vcap_port_debugfs_portno(struct device *dev,
+			      struct dentry *parent,
+			      struct vcap_control *vctrl,
+			      struct net_device *ndev,
+			      unsigned int portno);
+
 /* Create a debugFS entry for a vcap instance */
 struct dentry *vcap_debugfs(struct device *dev, struct dentry *parent,
 			    struct vcap_control *vctrl);
@@ -30,6 +36,14 @@ static inline void vcap_port_debugfs(struct device *dev, struct dentry *parent,
 {
 }
 
+static inline void vcap_port_debugfs_portno(struct device *dev,
+					    struct dentry *parent,
+					    struct vcap_control *vctrl,
+					    struct net_device *ndev,
+					    unsigned int portno)
+{
+}
+
 static inline struct dentry *vcap_debugfs(struct device *dev,
 					  struct dentry *parent,
 					  struct vcap_control *vctrl)

-- 
2.34.1

Re: [PATCH net v3 1/2] net: microchip: vcap: use port number instead of netdev name for debugfs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing debugfs_remove_recursive() on the probe error path leaves stranded debugfs files that trigger Use-After-Free when accessed.
- [High] Failure in vcap_debugfs() drops per-port debugfs files into the global debugfs root, leading to UAF on module unload.
--

--- Patch [1]: [PATCH net v3 1/2] net: microchip: vcap: use port number instead of netdev name for debugfs ---
commit 8d862146f79389830b4bf1733e4f4af6bbff212d
Author: Daniel Machon <daniel.machon@microchip.com>

net: microchip: vcap: use port number instead of netdev name for debugfs

This commit introduces vcap_port_debugfs_portno() to create debugfs
entries using the hardware port number instead of the netdev name. This
resolves boot warnings in sparx5_vcap_init() when netdev names are not
yet registered.

> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c b/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
> index cf332de6bf736..2dee2ce19fce7 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
> @@ -2077,8 +2077,9 @@ int sparx5_vcap_init(struct sparx5 *sparx5)
>  	dir = vcap_debugfs(sparx5->dev, sparx5->debugfs_root, ctrl);
>  	for (idx = 0; idx < consts->n_ports; ++idx)
>  		if (sparx5->ports[idx])
> -			vcap_port_debugfs(sparx5->dev, dir, ctrl,
> -					  sparx5->ports[idx]->ndev);
> +			vcap_port_debugfs_portno(sparx5->dev, dir, ctrl,
> +						 sparx5->ports[idx]->ndev,
> +						 sparx5->ports[idx]->portno);

[Severity: High]
This isn't a bug introduced by this patch, but what happens if vcap_debugfs()
fails to allocate memory and returns NULL?

In drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c:vcap_debugfs():

	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
	if (!info)
		return NULL;

If dir is NULL here, passing it as the parent to vcap_port_debugfs_portno()
will cause debugfs_create_file() to create the port debugfs files in the
global debugfs root (e.g., /sys/kernel/debug/p0).

When the module is unloaded, the driver only cleans up sparx5->debugfs_root,
abandoning the files in the global root. The devm-managed backing data
is freed, so reading these abandoned files afterward will cause a
use-after-free.

>  	return err;
>  }
> diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
> index e0c65c7ab23e1..476f7496a9d44 100644
> --- a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
> +++ b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c

[ ... ]

> @@ -400,6 +400,27 @@ void vcap_port_debugfs(struct device *dev, struct dentry *parent,
>  }
>  EXPORT_SYMBOL_GPL(vcap_port_debugfs);
>  
> +void vcap_port_debugfs_portno(struct device *dev,
> +			      struct dentry *parent,
> +			      struct vcap_control *vctrl,
> +			      struct net_device *ndev,
> +			      unsigned int portno)
> +{
> +	struct vcap_port_debugfs_info *info;
> +	char name[16];
> +
> +	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);

[Severity: High]
This is a pre-existing issue, but does the probe error path properly
clean up these debugfs files?

During initialization, mchp_sparx5_probe() creates sparx5->debugfs_root and
populates it with files backed by this devm_kzalloc() memory.

If a subsequent probe step fails (such as sparx5_register_netdevs()), the
error cleanup path does not seem to call debugfs_remove_recursive().

Since the devres framework automatically frees this memory on probe failure
but the files remain in the filesystem, could this lead to a use-after-free
if a user later reads one of these leaked debugfs files?

> +	if (!info)
> +		return;
> +
> +	info->vctrl = vctrl;
> +	info->ndev = ndev;
> +
> +	snprintf(name, sizeof(name), "p%u", portno);
> +	debugfs_create_file(name, 0444, parent, info, &vcap_port_debugfs_fops);
> +}
> +EXPORT_SYMBOL_GPL(vcap_port_debugfs_portno);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-misc-fixes-sparx5-lan969x-v3-0-c7c7fef723a8@microchip.com?part=1

[PATCH net v3 2/2] net: sparx5: fix sleep in atomic context in MAC table access

[Daniel Machon]

sparx5_set_rx_mode() runs with netif_addr_lock_bh held and iterates
dev->mc via __dev_mc_sync(), which per address calls sparx5_mc_sync() /
sparx5_mc_unsync() -> sparx5_mact_learn() / sparx5_mact_forget().  These
take sparx5->lock, a mutex, and then poll the MAC access command
register with readx_poll_timeout(). A mutex may block, which is not
allowed from atomic context.

Convert the driver to the new .ndo_set_rx_mode_async callback introduced
in commit 3554b4345d85 ("net: introduce ndo_set_rx_mode_async and
netdev_rx_mode_work"). The async callback is invoked from process
context, so the mutex and sleeping completion poll can remain.

Observed with CONFIG_PROVE_LOCKING, CONFIG_DEBUG_SPINLOCK,
CONFIG_DEBUG_MUTEXES and CONFIG_DEBUG_ATOMIC_SLEEP enabled:

  BUG: sleeping function called from invalid context at kernel/locking/mutex.c:591
  in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 217, name: ip
  preempt_count: 201, expected: 0
  Call trace:
   __might_resched+0x144/0x248
   __might_sleep+0x48/0x7c
   __mutex_lock+0x74/0x850
   mutex_lock_nested+0x24/0x30
   sparx5_mact_learn+0x78/0x100
   sparx5_mc_sync+0x40/0x54
   __hw_addr_sync_dev+0xc4/0x170
   sparx5_set_rx_mode+0x4c/0x58
   __dev_set_rx_mode+0x64/0xa4
   __dev_open+0x1ec/0x26c

Fixes: d6fce5141929 ("net: sparx5: add switching support")
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
index 1d34af78166a..4dedf5d92641 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
@@ -162,13 +162,18 @@ static int sparx5_port_stop(struct net_device *ndev)
 	return 0;
 }
 
-static void sparx5_set_rx_mode(struct net_device *dev)
+static int sparx5_set_rx_mode(struct net_device *dev,
+			      struct netdev_hw_addr_list *uc,
+			      struct netdev_hw_addr_list *mc)
 {
 	struct sparx5_port *port = netdev_priv(dev);
 	struct sparx5 *sparx5 = port->sparx5;
 
 	if (!test_bit(port->portno, sparx5->bridge_mask))
-		__dev_mc_sync(dev, sparx5_mc_sync, sparx5_mc_unsync);
+		return __hw_addr_sync_dev(mc, dev, sparx5_mc_sync,
+					  sparx5_mc_unsync);
+
+	return 0;
 }
 
 static int sparx5_port_get_phys_port_name(struct net_device *dev,
@@ -249,7 +254,7 @@ static const struct net_device_ops sparx5_port_netdev_ops = {
 	.ndo_open               = sparx5_port_open,
 	.ndo_stop               = sparx5_port_stop,
 	.ndo_start_xmit         = sparx5_port_xmit_impl,
-	.ndo_set_rx_mode        = sparx5_set_rx_mode,
+	.ndo_set_rx_mode_async  = sparx5_set_rx_mode,
 	.ndo_get_phys_port_name = sparx5_port_get_phys_port_name,
 	.ndo_set_mac_address    = sparx5_set_mac_address,
 	.ndo_validate_addr      = eth_validate_addr,

-- 
2.34.1

Re: [PATCH net v3 2/2] net: sparx5: fix sleep in atomic context in MAC table access

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The switchdev bridge join/leave handlers manually call `__dev_mc_sync` and `__dev_mc_unsync` without holding `netif_addr_lock_bh`, corrupting `dev->mc` list.
- [High] The driver destroys its workqueue prior to unregistering the notifiers that submit work to it, causing a use-after-free.
- [High] Dynamic FDB MAC entries use `devm_kzalloc` and `devm_kfree`, causing severe O(N) devres list traversal overhead leading to lockups.
--

--- Patch [2]: [PATCH net v3 2/2] net: sparx5: fix sleep in atomic context in MAC table access ---
commit 0271557455a8033457131b5edf4c50d634d7c1d7
Author: Daniel Machon <daniel.machon@microchip.com>

net: sparx5: fix sleep in atomic context in MAC table access

This commit converts the sparx5 driver to use the ndo_set_rx_mode_async
callback to avoid sleeping in atomic context. The previous ndo_set_rx_mode
implementation polled the MAC access command register under a mutex, which
is not allowed from atomic context.

> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
> index 1d34af78166a3..4dedf5d926416 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
> @@ -162,13 +162,18 @@ static int sparx5_port_stop(struct net_device *ndev)
>  	return 0;
>  }
>  
> -static void sparx5_set_rx_mode(struct net_device *dev)
> +static int sparx5_set_rx_mode(struct net_device *dev,
> +			      struct netdev_hw_addr_list *uc,
> +			      struct netdev_hw_addr_list *mc)
>  {
>  	struct sparx5_port *port = netdev_priv(dev);
>  	struct sparx5 *sparx5 = port->sparx5;
>  
>  	if (!test_bit(port->portno, sparx5->bridge_mask))
> -		__dev_mc_sync(dev, sparx5_mc_sync, sparx5_mc_unsync);

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the
__dev_mc_sync changes, I noticed a pre-existing issue in the switchdev code.
The bridge join/leave handlers manually call __dev_mc_sync() and
__dev_mc_unsync() without holding netif_addr_lock_bh:

drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c:sparx5_port_bridge_join() {
    ...
    __dev_mc_unsync(ndev, sparx5_mc_unsync);
    ...
}

drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c:sparx5_port_bridge_leave() {
    ...
    __dev_mc_sync(port->ndev, sparx5_mc_sync, sparx5_mc_unsync);
    ...
}

Since these are called under rtnl_lock but without netif_addr_lock_bh, could
this corrupt the dev->mc linked list if there are concurrent unprivileged
multicast mutations (like IGMP/MLD)?

> +		return __hw_addr_sync_dev(mc, dev, sparx5_mc_sync,
> +					  sparx5_mc_unsync);
> +
> +	return 0;
>  }

[ ... ]

[Severity: High]
This is a pre-existing issue, but does module removal destroy the workqueue
while the switchdev notifier is still active?

drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c:sparx5_unregister_notifier_blocks() {
    ...
    destroy_workqueue(sparx5_owq);
    
    unregister_switchdev_blocking_notifier(&s5->switchdev_blocking_nb);
    unregister_switchdev_notifier(&s5->switchdev_nb);
    ...
}

If a concurrent switchdev FDB event occurs after destroy_workqueue() but
before unregister_switchdev_notifier(), sparx5_switchdev_event() might
queue work on the destroyed workqueue. Can this sequence result in a
use-after-free and kernel panic?

[Severity: High]
This isn't a bug introduced by this patch, but does the driver use devres
allocations for highly dynamic network state?

drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c:alloc_mact_entry() {
    ...
    mact_entry = devm_kzalloc(sparx5->dev,
                              sizeof(*mact_entry), GFP_ATOMIC);
    ...
}

drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c:sparx5_del_mact_entry() {
    ...
    devm_kfree(sparx5->dev, mact_entry);
    ...
}

Because devm_kfree() linearly scans the entire devres list under the
dev->devres_lock spinlock with IRQs disabled, could this cause severe O(N)
traversal overhead and lockups when handling thousands of dynamic FDB MAC
entries?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-misc-fixes-sparx5-lan969x-v3-0-c7c7fef723a8@microchip.com?part=2