linux-kernel - Re: [patch 2/2] Add a driver for the Winbond WPCD376I Consumer IR hardware

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:	Thu, 13 Aug 2009 19:14:01 +0200
From:	David Härdeman <david@...deman.nu>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:	linux-kernel@...r.kernel.org, linux-input@...r.kernel.org
Subject: Re: [patch 2/2] Add a driver for the Winbond WPCD376I Consumer IR
 hardware

On Wed, Aug 12, 2009 at 11:58:22PM -0700, Dmitry Torokhov wrote:
>> +/* Registers and other state is protected by wbcir_lock */
>> +struct wbcir_data {
>> +	unsigned long wbase;        /* Wake-Up Baseaddr		*/
>> +	unsigned long ebase;        /* Enhanced Func. Baseaddr	*/
>> +	unsigned long sbase;        /* Serial Port Baseaddr	*/
>> +	unsigned int  irq;          /* Serial Port IRQ		*/
>> +
>> +	struct input_dev *input_dev;
>> +	struct timer_list timer_keyup;
>> +	struct led_trigger *rxtrigger;
>> +	struct led_trigger *txtrigger;
>> +	struct led_classdev led;
>> +
>> +	u32 last_scancode;
>> +	unsigned int last_keycode;
>> +	u8 last_toggle;
>> +	u8 keypressed;
>> +	unsigned long keyup_jiffies;
>> +	unsigned int idle_count;
>> +
>> +	/* RX irdata and parsing state */
>> +	unsigned long irdata[30];
>> +	unsigned int irdata_count;
>> +	unsigned int irdata_idle;
>> +	unsigned int irdata_off;
>> +	unsigned int irdata_error;
>> +
>> +	/* Protected by keytable_lock */
>> +	struct list_head keytable;
>
>I think this has a potential to deadlock... Set and get keycodes are
>called with event lock taken, and then your implementations acquire
>keytable lock. When you emit input events the opposite happens - you
>take the keytable lock and then input core takes event lock.

I've taken a look at it, and I think it's ok. set/getkeycode take the 
event lock and then the keytable lock.

The irqhandler call sequence (which generates the input events) will 
take the keytable lock, copy the keycode, release the lock and report 
the event (which will take the event lock).

-- 
David Härdeman
--
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/