[PATCH] power: supply: sysfs: enable is_writeable check during sysfs creation

[PATCH] power: supply: sysfs: enable is_writeable check during sysfs creation

[Andreas Kemnade]

The files in sysfs are created during device_add(). psy->use_cnt
is not incremented yet. So attributes are created readonly
without checking desc->property_is_writeable() and writeable
files are readonly.

To fix this, revert back to calling desc->property_is_writeable()
directly without using the helper.

Fixes: be6299c6e55e ("power: supply: sysfs: use power_supply_property_is_writeable()")
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Cc: stable@vger.kernel.org # 6.11
---
 drivers/power/supply/power_supply_sysfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index 3e63d165b2f70..b86e11bdc07ef 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -379,7 +379,8 @@ static umode_t power_supply_attr_is_visible(struct kobject *kobj,
 		int property = psy->desc->properties[i];
 
 		if (property == attrno) {
-			if (power_supply_property_is_writeable(psy, property) > 0)
+			if (psy->desc->property_is_writeable &&
+			    psy->desc->property_is_writeable(psy, property) > 0)
 				mode |= S_IWUSR;
 
 			return mode;
-- 
2.39.2

Re: [PATCH] power: supply: sysfs: enable is_writeable check during sysfs creation

[Thomas Weißschuh]

Hi Andreas,

On 2024-09-14 10:15:23+0000, Andreas Kemnade wrote:
> The files in sysfs are created during device_add(). psy->use_cnt
> is not incremented yet. So attributes are created readonly
> without checking desc->property_is_writeable() and writeable
> files are readonly.
> 
> To fix this, revert back to calling desc->property_is_writeable()
> directly without using the helper.

Hans noticed the same issue, but fixed it differently [0].
The problem is that the hwmon registration also uses
power_supply_property_is_writeable() and has the same issue.
(Independently from my change)

IMO this is the better fix.

(Plus the renaming and unexporting of the function that I'll add if it
won't be part of the first fix)

[0] https://lore.kernel.org/all/20240908185337.103696-1-hdegoede@redhat.com/

> Fixes: be6299c6e55e ("power: supply: sysfs: use power_supply_property_is_writeable()")
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> Cc: stable@vger.kernel.org # 6.11
> ---
>  drivers/power/supply/power_supply_sysfs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index 3e63d165b2f70..b86e11bdc07ef 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -379,7 +379,8 @@ static umode_t power_supply_attr_is_visible(struct kobject *kobj,
>  		int property = psy->desc->properties[i];
>  
>  		if (property == attrno) {
> -			if (power_supply_property_is_writeable(psy, property) > 0)
> +			if (psy->desc->property_is_writeable &&
> +			    psy->desc->property_is_writeable(psy, property) > 0)
>  				mode |= S_IWUSR;
>  
>  			return mode;
> -- 
> 2.39.2
>