drivers/cdx/cdx.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c index 07371cb653d3..4af1901c9d52 100644 --- a/drivers/cdx/cdx.c +++ b/drivers/cdx/cdx.c @@ -470,8 +470,12 @@ static ssize_t driver_override_show(struct device *dev, struct device_attribute *attr, char *buf) { struct cdx_device *cdx_dev = to_cdx_device(dev);
ssize_t len;
return sysfs_emit(buf, "%s\n", cdx_dev->driver_override);
device_lock(dev);len = sysfs_emit(buf, "%s\n", cdx_dev->driver_override);device_unlock(dev);No, you should not need to lock a device in a sysfs callback like this, especially for just printing out a string.
This function is part of DEVICE_ATTR_RW, which includes both driver_override_show() and driver_override_store(). These functions can be executed concurrently in sysfs.
The driver_override_store() function uses driver_set_override() to update the driver_override value, and driver_set_override() internally locks the device (device_lock(dev)). If driver_override_show() reads cdx_dev->driver_override without locking, it could potentially access a freed pointer if driver_override_store() frees the string concurrently. This could lead to printing a kernel address, which is a security risk since DEVICE_ATTR can be read by all users.
Additionally, a similar pattern is used in drivers/amba/bus.c, as well as many other bus drivers, where device_lock() is taken in the show function, and it has been working without issues.
Regards, Qiu-ji Chen