The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Daniel Machon" <daniel.machon@microchip.com>
Cc: linux-rt-devel@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net v3 1/2] net: microchip: vcap: use port number instead of netdev name for debugfs
Date: Tue, 18 Aug 2026 15:42:21 +0000	[thread overview]
Message-ID: <20260818154221.D7F8E1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260817-misc-fixes-sparx5-lan969x-v3-1-c7c7fef723a8@microchip.com>

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

  reply	other threads:[~2026-08-18 15:42 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17 15:41 [PATCH net v3 0/2] net: sparx5: misc fixes for sparx5 and lan969x Daniel Machon
2026-08-17 15:41 ` [PATCH net v3 1/2] net: microchip: vcap: use port number instead of netdev name for debugfs Daniel Machon
2026-08-18 15:42   ` sashiko-bot [this message]
2026-08-17 15:41 ` [PATCH net v3 2/2] net: sparx5: fix sleep in atomic context in MAC table access Daniel Machon
2026-08-18 15:42   ` sashiko-bot

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=20260818154221.D7F8E1F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=daniel.machon@microchip.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    /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