linux-kernel - Re: [PATCH] hwmon: (gpio-fan) Fix fan_ctrl

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: <20101109085914.05589cca@endymion.delvare>
Date:	Tue, 9 Nov 2010 08:59:14 +0100
From:	Jean Delvare <khali@...ux-fr.org>
To:	Axel Lin <axel.lin@...il.com>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	Guenter Roeck <guenter.roeck@...csson.com>,
	Simon Guinot <sguinot@...ie.com>, lm-sensors@...sensors.org
Subject: Re: [PATCH] hwmon: (gpio-fan) Fix fan_ctrl_init error path

Hi Alex,

On Tue, 09 Nov 2010 13:18:39 +0800, Axel Lin wrote:
> Remove sysfs entries before return -ENODEV.
> 
> Signed-off-by: Axel Lin <axel.lin@...il.com>
> ---
>  drivers/hwmon/gpio-fan.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> index aa701a1..d2e66b4 100644
> --- a/drivers/hwmon/gpio-fan.c
> +++ b/drivers/hwmon/gpio-fan.c
> @@ -388,11 +388,13 @@ static int fan_ctrl_init(struct gpio_fan_data *fan_data,
>  	fan_data->speed_index = get_fan_speed_index(fan_data);
>  	if (fan_data->speed_index < 0) {
>  		err = -ENODEV;
> -		goto err_free_gpio;
> +		goto err_remove_sysfs;
>  	}
>  
>  	return 0;
>  
> +err_remove_sysfs:
> +	sysfs_remove_group(&pdev->dev.kobj, &gpio_fan_ctrl_group);
>  err_free_gpio:
>  	for (i = i - 1; i >= 0; i--)
>  		gpio_free(ctrl[i]);

This is certainly better than the current situation, however the fact
that you need to do this strongly suggests that the initialization
order is wrong in the first place. Creating the sysfs attribute should
be the last thing done by the function, after all the rest has been
successful. Otherwise there is a small window during which user-space
can access the attribute but the driver isn't ready to deal with the
requests.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/