[PATCH] dmaengine: qcom: hidma: avoid world-writable channel sysfs files

[PATCH] dmaengine: qcom: hidma: avoid world-writable channel sysfs files

[Hungyu Lin]

hidma_mgmt_init_sys() creates the per-channel priority and weight
attributes with S_IRUGO | S_IWUGO, so any local user can write them.
A write stores the new value and calls hidma_mgmt_setup(), which
programs it into the channel's QoS register.

Attributes declared with __ATTR() are checked at build time by
VERIFY_OCTAL_PERMISSIONS(), which rejects world-writable modes. These
two are created at runtime with a mode passed to
create_sysfs_entry_channel(), so the check does not reach them.

Make them writable by the owner only, the 0644 that __ATTR_RW() uses.

Fixes: 7f8f209fd6e0 ("dmaengine: add Qualcomm Technologies HIDMA management driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 drivers/dma/qcom/hidma_mgmt_sys.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/qcom/hidma_mgmt_sys.c b/drivers/dma/qcom/hidma_mgmt_sys.c
index 9672ef9ee8fc..53b991752e80 100644
--- a/drivers/dma/qcom/hidma_mgmt_sys.c
+++ b/drivers/dma/qcom/hidma_mgmt_sys.c
@@ -265,13 +265,13 @@ int hidma_mgmt_init_sys(struct hidma_mgmt_dev *mdev)
 	/* populate parameters that are per channel */
 	for (i = 0; i < mdev->dma_channels; i++) {
 		rc = create_sysfs_entry_channel(mdev, "priority",
-						(S_IRUGO | S_IWUGO), i,
+						0644, i,
 						mdev->chroots[i]);
 		if (rc)
 			return rc;
 
 		rc = create_sysfs_entry_channel(mdev, "weight",
-						(S_IRUGO | S_IWUGO), i,
+						0644, i,
 						mdev->chroots[i]);
 		if (rc)
 			return rc;
-- 
2.43.0

Re: [PATCH] dmaengine: qcom: hidma: avoid world-writable channel sysfs files

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] dmaengine: qcom: hidma_mgmt: kobject leak and UAF in
  hidma_mgmt_init_sys() [1]
--

commit 44ad6eea44504d623e4ff527b7da4680661c450d
Author: Hungyu Lin <dennylin0707@gmail.com>

dmaengine: qcom: hidma: avoid world-writable channel sysfs files

This commit restricts the per-channel priority and weight sysfs attributes
in hidma_mgmt_init_sys() from world-writable (S_IRUGO | S_IWUGO) to 0644.

> diff --git a/drivers/dma/qcom/hidma_mgmt_sys.c b/drivers/dma/qcom/hidma_mgmt_sys.c
> index 9672ef9ee8fc0..53b991752e805 100644
> --- a/drivers/dma/qcom/hidma_mgmt_sys.c
> +++ b/drivers/dma/qcom/hidma_mgmt_sys.c
> @@ -265,13 +265,13 @@ int hidma_mgmt_init_sys(struct hidma_mgmt_dev *mdev)
>  	/* populate parameters that are per channel */
>  	for (i = 0; i < mdev->dma_channels; i++) {
>  		rc = create_sysfs_entry_channel(mdev, "priority",
> -						(S_IRUGO | S_IWUGO), i,
> +						0644, i,
>  						mdev->chroots[i]);
>  		if (rc)
>  			return rc;
>  
>  		rc = create_sysfs_entry_channel(mdev, "weight",
> -						(S_IRUGO | S_IWUGO), i,
> +						0644, i,
>  						mdev->chroots[i]);
>  		if (rc)
>  			return rc;

[Severity: Critical]
This problem wasn't introduced by this patch, but does hidma_mgmt_init_sys()
leak kobjects and leave dangling sysfs attributes on failure or driver
unbind [1]?

If any step in hidma_mgmt_init_sys() fails, it returns without calling
kobject_put() on chanops or mdev->chroots[i] or removing previously created
sysfs files, and hidma_mgmt_driver has no .remove callback to clean them up
on unbind either.  Because the attribute structs, names, and mdev itself are
allocated with devm_kmalloc(), devres frees them on probe failure or device
unbind while the sysfs files remain exposed, causing a use-after-free when
userspace reads or writes those attributes.

[1] https://sashiko.dev/#/bug/linux-68f0e3d9-2e4a-4922-9d34-c92a40fad2aa

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