[PATCH] scsi: Correct sysfs attributes access rights

[PATCH] scsi: Correct sysfs attributes access rights

[Damien Le Moal]

The scsi sysfs attributes "supported_mode" and "active_mode" do not
define a store method and thus cannot be modified. Correct the
DEVICE_ATTR() call for these two attributes to not include S_IWUSR to
allow write access as they are read-only.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/scsi/scsi_sysfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index d772258e29ad..e6464b998960 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -265,7 +265,7 @@ show_shost_supported_mode(struct device *dev, struct device_attribute *attr,
 	return show_shost_mode(supported_mode, buf);
 }
 
-static DEVICE_ATTR(supported_mode, S_IRUGO | S_IWUSR, show_shost_supported_mode, NULL);
+static DEVICE_ATTR(supported_mode, S_IRUGO, show_shost_supported_mode, NULL);
 
 static ssize_t
 show_shost_active_mode(struct device *dev,
@@ -279,7 +279,7 @@ show_shost_active_mode(struct device *dev,
 		return show_shost_mode(shost->active_mode, buf);
 }
 
-static DEVICE_ATTR(active_mode, S_IRUGO | S_IWUSR, show_shost_active_mode, NULL);
+static DEVICE_ATTR(active_mode, S_IRUGO, show_shost_active_mode, NULL);
 
 static int check_reset_type(const char *str)
 {
-- 
2.50.1

Re: [PATCH] scsi: Correct sysfs attributes access rights

[John Garry]

On 28/07/2025 05:17, Damien Le Moal wrote:
> The scsi sysfs attributes "supported_mode" and "active_mode" do not
> define a store method and thus cannot be modified. Correct the
> DEVICE_ATTR() call for these two attributes to not include S_IWUSR to
> allow write access as they are read-only.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>

Reviewed-by: John Garry <john.g.garry@oracle.com>

Note that class_attr_store() returns -EIO for no store method. Does the 
same happen after this change?

> ---
>   drivers/scsi/scsi_sysfs.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index d772258e29ad..e6464b998960 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -265,7 +265,7 @@ show_shost_supported_mode(struct device *dev, struct device_attribute *attr,
>   	return show_shost_mode(supported_mode, buf);
>   }
>   
> -static DEVICE_ATTR(supported_mode, S_IRUGO | S_IWUSR, show_shost_supported_mode, NULL);
> +static DEVICE_ATTR(supported_mode, S_IRUGO, show_shost_supported_mode, NULL);
>   
>   static ssize_t
>   show_shost_active_mode(struct device *dev,
> @@ -279,7 +279,7 @@ show_shost_active_mode(struct device *dev,
>   		return show_shost_mode(shost->active_mode, buf);
>   }
>   
> -static DEVICE_ATTR(active_mode, S_IRUGO | S_IWUSR, show_shost_active_mode, NULL);
> +static DEVICE_ATTR(active_mode, S_IRUGO, show_shost_active_mode, NULL);
>   
>   static int check_reset_type(const char *str)
>   {

Re: [PATCH] scsi: Correct sysfs attributes access rights

[Damien Le Moal]

On 7/28/25 6:40 PM, John Garry wrote:
> On 28/07/2025 05:17, Damien Le Moal wrote:
>> The scsi sysfs attributes "supported_mode" and "active_mode" do not
>> define a store method and thus cannot be modified. Correct the
>> DEVICE_ATTR() call for these two attributes to not include S_IWUSR to
>> allow write access as they are read-only.
>>
>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> 
> Reviewed-by: John Garry <john.g.garry@oracle.com>
> 
> Note that class_attr_store() returns -EIO for no store method. Does the same
> happen after this change?

The regular Permission Denied (-EPERM) is returned.
Do you think that is a problem ?

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] scsi: Correct sysfs attributes access rights

[John Garry]

On 28/07/2025 10:52, Damien Le Moal wrote:
>> Note that class_attr_store() returns -EIO for no store method. Does the same
>> happen after this change?
> The regular Permission Denied (-EPERM) is returned.
> Do you think that is a problem ?

Probably not. I can't imagine that any sane application would rely on 
that behavior (of returning -EIO).

Re: [PATCH] scsi: Correct sysfs attributes access rights

[Johannes Thumshirn]

Looks good,

Reviewed-by: Johannes Thumshin <johannes.thumshirn@wdc.com>

Re: [PATCH] scsi: Correct sysfs attributes access rights

[Martin K. Petersen]

Damien,

> The scsi sysfs attributes "supported_mode" and "active_mode" do not
> define a store method and thus cannot be modified. Correct the
> DEVICE_ATTR() call for these two attributes to not include S_IWUSR to
> allow write access as they are read-only.

Applied to 6.17/scsi-staging, thanks!

-- 
Martin K. Petersen

Re: [PATCH] scsi: Correct sysfs attributes access rights

[Martin K. Petersen]

On Mon, 28 Jul 2025 13:17:00 +0900, Damien Le Moal wrote:

> The scsi sysfs attributes "supported_mode" and "active_mode" do not
> define a store method and thus cannot be modified. Correct the
> DEVICE_ATTR() call for these two attributes to not include S_IWUSR to
> allow write access as they are read-only.
> 
> 

Applied to 6.17/scsi-queue, thanks!

[1/1] scsi: Correct sysfs attributes access rights
      https://git.kernel.org/mkp/scsi/c/a2f54ff15c3b

-- 
Martin K. Petersen	Oracle Linux Engineering