linux-kernel - Re: [PATCH 1/2] leds: simply LED trigger list management

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: <1358448677.26320.22.camel@doorstop.aus.2wire.com>
Date:	Thu, 17 Jan 2013 12:51:17 -0600
From:	Nathan Lynch <ntl@...ox.com>
To:	"Kim, Milo" <Milo.Kim@...com>
Cc:	Bryan Wu <cooloney@...il.com>,
	"linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] leds: simply LED trigger list management

On Thu, 2013-01-17 at 01:06 +0000, Kim, Milo wrote:
> @@ -242,17 +233,15 @@ EXPORT_SYMBOL_GPL(led_trigger_unregister);
>  void led_trigger_event(struct led_trigger *trig,
>  			enum led_brightness brightness)
>  {
> -	struct list_head *entry;
> +	struct led_classdev *led_cdev;
>  
>  	if (!trig)
>  		return;
>  
>  	read_lock(&trig->leddev_list_lock);
> -	list_for_each(entry, &trig->led_cdevs) {
> -		struct led_classdev *led_cdev;
> -
> -		led_cdev = list_entry(entry, struct led_classdev, trig_list);
> -		led_set_brightness(led_cdev, brightness);
> +	list_for_each_entry(led_cdev, &leds_list, node) {
> +		if (led_cdev->trigger == trig)
> +			led_set_brightness(led_cdev, brightness);
>  	}
>  	read_unlock(&trig->leddev_list_lock);

Continuing to use trig->leddev_list_lock doesn't seem right.  Shouldn't
traversal of leds_list be guarded by the leds_list_lock rwsem?  And if
so, is it safe to use a potentially-blocking lock in this context?


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