linux-kernel - Re: [RFC 3/5] hid: Parse the device before adding it

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: <CAN+gG=G9KSjSFmWpxpPLzMzz14ePAhQKTJZjQBsPeq2iztZ17A@mail.gmail.com>
Date:	Tue, 3 Apr 2012 10:41:56 +0200
From:	Benjamin Tissoires <benjamin.tissoires@...il.com>
To:	Henrik Rydberg <rydberg@...omail.se>
Cc:	Jiri Kosina <jkosina@...e.cz>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Stephane Chatty <chatty@...c.fr>, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC 3/5] hid: Parse the device before adding it

Hi Henrik,

On Tue, Apr 3, 2012 at 09:05, Henrik Rydberg <rydberg@...omail.se> wrote:
> The hid bus is populated by devices created by the usb and bluetooth
> subsystems. The hid device is then broadcast to userland via uevents.
> Currently, the parsing of the hid reports is done during probe of
> the hid device, after the device has been broadcast. In order to
> allow for the report descriptors to influence the device properties,
> it is desirable to parse the device _before_ it is broadcast to
> userland. In actuality, the parsing depends only accidentally on
> the driver being present, so it can be trivially achieved.
>
> Something also needs to be done for the report_fixup handler, which
> seems to be the only hard device-driver coupling in the code.
>
> Signed-off-by: Henrik Rydberg <rydberg@...omail.se>
> ---
>  drivers/hid/hid-core.c        |    2 +-
>  drivers/hid/usbhid/hid-core.c |    5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 35ba9d9..8a7b59e 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -658,7 +658,7 @@ int hid_parse_report(struct hid_device *device, __u8 *start,
>                hid_parser_reserved
>        };
>
> -       if (device->driver->report_fixup)
> +       if (device->driver && device->driver->report_fixup)
>                start = device->driver->report_fixup(device, start, &size);

Well, this is not a good solution for the following drivers:
hid-zydacron
hid-prodikeys
hid-uclogic
hid-samsung
hid-sony, etc...

they all use report_fixup and as the parse is made only once, they
won't be able to work properly.


>
>        device->rdesc = kmemdup(start, size, GFP_KERNEL);
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index 5bf91db..e63613b 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -1266,6 +1266,11 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
>        if (usb_string(dev, dev->descriptor.iSerialNumber, hid->uniq, 64) <= 0)
>                hid->uniq[0] = 0;
>
> +       ret = hid_parse(hid);
> +       if (ret)
> +               goto err;

shouldn't we do the same for hidp?

Thanks,
Benjamin

> +
> +
>        usbhid = kzalloc(sizeof(*usbhid), GFP_KERNEL);
>        if (usbhid == NULL) {
>                ret = -ENOMEM;
> --
> 1.7.9.5
>
--
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/