Re: [PATCH v1] dmaengine: qcom: hidma-mgmt: Fix sysfs cleanup on setup failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Reliance on `kobject_put` without `kobject_del` for sysfs cleanup leads to use-after-free
--

commit a6dc90d05cf8a69c255834fbadc8ca4eb60dbe56
Author: Yuho Choi <dbgh9129@gmail.com>

dmaengine: qcom: hidma-mgmt: Fix sysfs cleanup on setup failure

hidma_mgmt_init_sys() creates the chanops kobject, per-channel
kobjects and sysfs files incrementally. If a later creation step fails,
the function returns without tearing down the objects already created.

> diff --git a/drivers/dma/qcom/hidma_mgmt_sys.c b/drivers/dma/qcom/hidma_mgmt_sys.c
> --- a/drivers/dma/qcom/hidma_mgmt_sys.c
> +++ b/drivers/dma/qcom/hidma_mgmt_sys.c

[ ... ]

> @@ -231,20 +231,52 @@ static int create_sysfs_entry_channel(struct hidma_mgmt_dev *mdev, char *name,
>  	return sysfs_create_file(parent, &chattr->attr.attr);
>  }
>  
> +static void hidma_mgmt_uninit_sys(struct hidma_mgmt_dev *mdev,
> +				  unsigned int sysfs_count,
> +				  unsigned int chroot_count)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < sysfs_count; i++) {
> +		struct attribute attr = { .name = hidma_mgmt_files[i].name };
> +
> +		sysfs_remove_file(&mdev->pdev->dev.kobj, &attr);
> +	}
> +
> +	for (i = 0; i < chroot_count; i++) {
> +		kobject_put(mdev->chroots[i]);

[Severity: High]
This is a pre-existing issue, but does this teardown logic still leave a
potential use-after-free?

Since the sysfs attributes (chattr) and their names are allocated using
devm_kmalloc() and devm_kstrdup() in create_sysfs_entry_channel(), devres
will free them immediately upon probe failure or device unbind.

If userspace holds an open file descriptor to any of the per-channel sysfs
files, or if CONFIG_DEBUG_KOBJECT_RELEASE is enabled, kobject_put() will
only decrement the reference count and the implicit kobject_del() inside
kobject_release() will be delayed. A subsequent read or write to the open
sysfs file would then dereference the already freed chattr memory.

Should kobject_del() be explicitly called before kobject_put() here to
synchronously remove the sysfs directory and files before devres frees the
backing memory?

> +		mdev->chroots[i] = NULL;
> +	}
> +
> +	if (mdev->chanops) {
> +		kobject_put(mdev->chanops);
> +		mdev->chanops = NULL;
> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608030846.2602111-1-dbgh9129@gmail.com?part=1