From: Damien Le Moal dlemoal@kernel.org
[ Upstream commit a2f54ff15c3bdc0132e20aae041607e2320dbd73 ]
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 Link: https://lore.kernel.org/r/20250728041700.76660-1-dlemoal@kernel.org Reviewed-by: John Garry john.g.garry@oracle.com Reviewed-by: Johannes Thumshin johannes.thumshirn@wdc.com Signed-off-by: Martin K. Petersen martin.petersen@oracle.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
**Backport Status: YES**
This commit should be backported to stable kernel trees for the following reasons:
## Bug Fix Nature The commit fixes a clear bug where sysfs attributes `supported_mode` and `active_mode` incorrectly have write permissions (S_IWUSR) despite having no store method defined (NULL is passed as the store parameter to DEVICE_ATTR). This is a longstanding bug dating back to 2007 when these attributes were first introduced in commit 5dc2b89e1242.
## Security and Stability Implications 1. **Misleading permissions**: The incorrect S_IWUSR permission suggests these attributes are writable when they are not, which could confuse userspace tools and administrators.
2. **Potential crashes**: While the kernel's sysfs framework likely handles the NULL store method gracefully, having write permissions on read-only attributes is incorrect and could potentially lead to issues if userspace attempts to write to these files.
3. **Permission principle violation**: This violates the principle of least privilege - files should only have the permissions they actually support.
## Small and Contained Fix The fix is extremely minimal - it simply removes the S_IWUSR flag from two DEVICE_ATTR declarations. The changes are: - Line 268: `S_IRUGO | S_IWUSR` → `S_IRUGO` for supported_mode - Line 282: `S_IRUGO | S_IWUSR` → `S_IRUGO` for active_mode
## No Architectural Changes This is a straightforward permission correction with: - No functional changes to the SCSI subsystem - No changes to data structures or APIs - No new features added - Minimal risk of regression
## Long-standing Issue This bug has existed since 2007 (commit 5dc2b89e1242), meaning it affects all stable kernels that include the SCSI host mode attributes. The fact that it went unnoticed for so long suggests it's not causing critical failures, but it's still incorrect behavior that should be fixed.
## Stable Tree Criteria This meets the stable tree criteria as it: - Fixes an obvious bug (incorrect permissions) - Is a trivial fix (2 lines changed) - Has essentially zero risk of introducing regressions - Corrects a violation of kernel coding standards (attributes without store methods should not have write permissions)
The commit is an ideal candidate for stable backporting as it's a simple, safe correction of a longstanding permission bug in the SCSI sysfs interface.
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) {