linux-kernel - Re: [PATCH 1/3] HID: Use existing parser for pre-scanning the report descriptors

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: <520BE280.6010407@ahsoftware.de>
Date:	Wed, 14 Aug 2013 22:03:12 +0200
From:	Alexander Holler <holler@...oftware.de>
To:	Benjamin Tissoires <benjamin.tissoires@...hat.com>
CC:	rydberg@...omail.se,
	Benjamin Tissoires <benjamin.tissoires@...il.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Stephane Chatty <chatty@...c.fr>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] HID: Use existing parser for pre-scanning the report
 descriptors

Am 14.08.2013 17:38, schrieb Benjamin Tissoires:

>>>   {
>>>   	if (usage == HID_DG_CONTACTID)
>>> -		hid->group = HID_GROUP_MULTITOUCH;
>>> +		parser->flags |= HID_FLAG_MULTITOUCH;
>>
>> Did you consider reusing the group flags, e.g., parser->groups |= (1
>> << HID_GROUP_MULTITOUCH)? This change could be made regardless of the
>> parser logic.
>
> If nobody is against changing the values of the different groups across
> kernel version (which should be harmless), then I fully agree, we can
> use the group item as a bit field (but we would be able to only have 16
> different device groups).

Hmm, that might become a problem. E.g. all the HID sensors might be used 
stand alone (without a sensor-hub, if someone modifies the drivers). But 
I agree that currently the flags are just confusing and would introduce 
them only if the number of groups reaches the limit.


>>> -	hid->group = HID_GROUP_GENERIC;
>>> +	parser = vzalloc(sizeof(struct hid_parser));
>>
>> Argh, I realize it is inevitable for this patch, but it still makes my
>> eyes bleed. The parser takes quite a bit of memory...
>
> Yep, my first attempt was to remove it, then I re-added it with a small
> tear...

So you actually create a new parser and the subject (that existing) of 
this patch is misleading.

Regards,

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