linux-kernel - Re: [PATCH] driver core: bus: Return -EIO instead of 0 when show/store invalid bus attribute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <70ec36ff-1be9-4fc3-9792-4950a483b272@quicinc.com>
Date: Wed, 24 Jul 2024 20:56:18 +0800
From: quic_zijuhu <quic_zijuhu@...cinc.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Zijun Hu
	<zijun_hu@...oud.com>
CC: "Rafael J. Wysocki" <rafael@...nel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] driver core: bus: Return -EIO instead of 0 when
 show/store invalid bus attribute

On 7/24/2024 1:31 AM, Greg Kroah-Hartman wrote:
> On Tue, Jul 23, 2024 at 10:55:43PM +0800, Zijun Hu wrote:
>> From: Zijun Hu <quic_zijuhu@...cinc.com>
>>
>> Return -EIO instead of 0 when show/store invalid bus attribute as
>> class/device/driver/kobject attribute.
> 
> Why?  What is this now going to break?  You are changing a user-visable
> api that has been this way for 20+ years, how was this tested?
> 
this change should break nothing.

tested by wc a writing only bus attribute, for example

root@...-Q35:/sys/bus/gpio# ls -l
--w------- 1 root root 4096  7月 24 20:20 drivers_probe
root@...-Q35:/sys/bus/gpio# chmod u+r drivers_probe
root@...-Q35:/sys/bus/gpio# wc -c drivers_probe
0 drivers_probe                  // for current design

root@...un-kvm-Q35:/sys/bus/gpio# wc -c drivers_probe
wc: drivers_probe: Input/output error  // for this change

>>
>> Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
>> ---
>>  drivers/base/bus.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
>> index ffea0728b8b2..e84522a90102 100644
>> --- a/drivers/base/bus.c
>> +++ b/drivers/base/bus.c
>> @@ -152,7 +152,7 @@ static ssize_t bus_attr_show(struct kobject *kobj, struct attribute *attr,
>>  {
>>  	struct bus_attribute *bus_attr = to_bus_attr(attr);
>>  	struct subsys_private *subsys_priv = to_subsys_private(kobj);
>> -	ssize_t ret = 0;
>> +	ssize_t ret = -EIO;
>>  
>>  	if (bus_attr->show)
>>  		ret = bus_attr->show(subsys_priv->bus, buf);
>> @@ -164,7 +164,7 @@ static ssize_t bus_attr_store(struct kobject *kobj, struct attribute *attr,
>>  {
>>  	struct bus_attribute *bus_attr = to_bus_attr(attr);
>>  	struct subsys_private *subsys_priv = to_subsys_private(kobj);
>> -	ssize_t ret = 0;
>> +	ssize_t ret = -EIO;
> 
> Why -EIO?  What is wrong with returning an empty string?
>

for this error case, all class/device/driver/kobject attributes return
-EIO, i just follow them for bus attribute.

for empty string, the value returned by attribute store() is returned.

> I think I know why you picked this but you better document the heck out
> of it AND test it very very well first.
> 
sure.
> thanks,
> 
> greg k-h