Re: [PATCH v1 4/5] dmaengine: switchtec-dma: Add pmon sysfs attributes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing `.release` callback in `switchtec_ktype` causes Use-After-Free of channel memory when sysfs files are held open during device removal.
- [Low] The `latency_selector_show` function violates sysfs rules by using `strcat()` to build a multi-line menu instead of using `sysfs_emit()`.
--

--- Patch [4]: [PATCH v1 4/5] dmaengine: switchtec-dma: Add pmon sysfs attributes ---
commit 9353b00f22d3df92fe9cfb71beccb84977c35e68
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: Add pmon sysfs attributes

Switchtec hardware exposes some performance monitor registers which
can be used to monitor various statistics of the hardware. Expose
these as a sysfs interface under the switchtec group in a pmon
sub-group.

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 4841134bd7b83..2ac43eb58995a 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c

[ ... ]

> @@ -1156,8 +1163,200 @@ static struct attribute_group switchtec_config_group = {
>  	.attrs = switchtec_config_attrs,
>  };
>  
> +#define pmon_show(chan, page, field, reader) ({				     \
> +	struct switchtec_dma_chan *__swdma_chan =			     \
> +		container_of((chan), struct switchtec_dma_chan, dma_chan);   \

[ ... ]

> +static ssize_t latency_selector_show(struct dma_chan *chan, char *page)
> +{
> +	struct switchtec_dma_chan *swdma_chan =
> +		container_of(chan, struct switchtec_dma_chan, dma_chan);
> +	struct chan_fw_regs __iomem *chan_fw = swdma_chan->mmio_chan_fw;
> +	u32 lat = 0;
> +
> +	rcu_read_lock();
> +	if (!rcu_dereference(swdma_chan->swdma_dev->pdev)) {
> +		rcu_read_unlock();
> +		return -ENODEV;
> +	}
> +
> +	lat = readl(&chan_fw->perf_latency_selector);
> +	rcu_read_unlock();
> +
> +	strcat(page, "To select a latency type, write the type number (1 ~ 5) to latency_selector\n\n");
> +
> +	strcat(page, "Latency Types:\n");
> +	strcat(page, "(1) SE Fetch latency");

[Severity: Low]
Should this code use sysfs_emit() instead of strcat() to construct the menu on
the raw page buffer? Sysfs documentation requires using sysfs_emit() and
generally expects a single value per file rather than a multi-line string.

> +	if (lat & SWITCHTEC_LAT_SE_FETCH)
> +		strcat(page, " (*)\n");
> +	else
> +		strcat(page, "\n");

[ ... ]

>  static const struct attribute_group *switchtec_groups[] = {
>  	&switchtec_config_group,
> +	&switchtec_pmon_group,
>  	NULL,
>  };

[Severity: High]
Does the driver's kobj_type (switchtec_ktype) lack a .release callback?

If a userspace process holds a channel's sysfs file open, keeping the
embedded kobject refcount elevated, and the device is unregistered,
switchtec_dma_release() unconditionally frees the channel memory.

When the file is closed later and the refcount drops to zero, the kernel
would access the already-freed kobject to invoke the missing release
function, leading to a Use-After-Free.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707162045.23910-1-logang@deltatee.com?part=4