linux-kernel - Re: [PATCH] hwmon/abituguru: handle sysfs errors

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]

Date:	Tue, 10 Oct 2006 11:08:03 +0200
From:	Jean Delvare <khali@...ux-fr.org>
To:	Hans de Goede <j.w.r.degoede@....nl>
Cc:	Jeff Garzik <jeff@...zik.org>, Andrew Morton <akpm@...l.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] hwmon/abituguru: handle sysfs errors

Hi Hans, Jeff,

> You (Jean) already mailed me about this and it was on my todo list,
> but I'm currently rather busy with work. So it looks like Jeff beat
> me to it.
> 
> Jeff's patch looks fine, please apply. Thanks Jeff!

The patch isn't wrong per se, but it could be made more simple, and is
incomplete in comparison to what was done for all other hardware
monitoring drivers:

* We want to create all the files before registering with the hwmon
  class, this closes a race condition.
* We want to delete all the device files at regular cleanup time (after
  unregistering with the hwmon class).
* It's OK to call device_create_file() on a non-existent file, so the
  error path can be simplified.

I'd like the abituguru driver to behave the same as all other hardware
monitoring drivers to lower the maintenance effort. Can either you
or Jeff work up a compliant patch? 

Thanks.

>  drivers/hwmon/abituguru.c |   30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
> 
> 2b10f648c8ed965369976eb7925b922ee187ce21
> diff --git a/drivers/hwmon/abituguru.c b/drivers/hwmon/abituguru.c
> index e5cb0fd..3ded982 100644
> --- a/drivers/hwmon/abituguru.c
> +++ b/drivers/hwmon/abituguru.c
> @@ -1271,14 +1271,34 @@ static int __devinit abituguru_probe(str
>  		res = PTR_ERR(data->class_dev);
>  		goto abituguru_probe_error;
>  	}
> -	for (i = 0; i < sysfs_attr_i; i++)
> -		device_create_file(&pdev->dev, &data->sysfs_attr[i].dev_attr);
> -	for (i = 0; i < ARRAY_SIZE(abituguru_sysfs_attr); i++)
> -		device_create_file(&pdev->dev,
> -			&abituguru_sysfs_attr[i].dev_attr);
> +	for (i = 0; i < sysfs_attr_i; i++) {
> +		res = device_create_file(&pdev->dev,
> +					 &data->sysfs_attr[i].dev_attr);
> +		if (res) {
> +			for (j = 0; j < i; j++)
> +				device_remove_file(&pdev->dev,
> +					 	&data->sysfs_attr[j].dev_attr);
> +			goto err_devreg;
> +		}
> +	}
> +	for (i = 0; i < ARRAY_SIZE(abituguru_sysfs_attr); i++) {
> +		res = device_create_file(&pdev->dev,
> +					 &abituguru_sysfs_attr[i].dev_attr);
> +		if (res) {
> +			for (j = 0; j < i; j++)
> +				device_remove_file(&pdev->dev,
> +					 &abituguru_sysfs_attr[j].dev_attr);
> +			goto err_attr_i;
> +		}
> +	}
>  
>  	return 0;
>  
> +err_attr_i:
> +	for (i = 0; i < sysfs_attr_i; i++)
> +		device_remove_file(&pdev->dev, &data->sysfs_attr[i].dev_attr);
> +err_devreg:
> +	hwmon_device_unregister(data->class_dev);
>  abituguru_probe_error:
>  	kfree(data);
>  	return res;


-- 
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/