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@xxxxxxxxxxxxx>

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@xxxxxxxxxxxxx?part=1