linux-kernel - Re: [v3.11][Regression] HID: hyperv: convert alloc+memcpy to memdup

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, 19 Dec 2013 11:08:10 +0100
From:	David Herrmann <dh.herrmann@...il.com>
To:	Jiri Kosina <jkosina@...e.cz>
Cc:	Joseph Salisbury <joseph.salisbury@...onical.com>,
	Dan Carpenter <dan.carpenter@...cle.com>, thomas@...3r.de,
	list@...osl.org, Haiyang Zhang <haiyangz@...rosoft.com>,
	LKML <linux-kernel@...r.kernel.org>, open@...osl.org,
	HID CORE LAYER <linux-input@...r.kernel.org>,
	"driverdev-devel@...uxdriverproject.org" 
	<devel@...uxdriverproject.org>
Subject: Re: [v3.11][Regression] HID: hyperv: convert alloc+memcpy to memdup

Hi

On Thu, Dec 19, 2013 at 10:59 AM, Jiri Kosina <jkosina@...e.cz> wrote:
> On Thu, 19 Dec 2013, David Herrmann wrote:
>
>> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> > index 253fe23..81eacd3 100644
>> > --- a/drivers/hid/hid-core.c
>> > +++ b/drivers/hid/hid-core.c
>> > @@ -1334,7 +1334,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
>> >                 csize--;
>> >         }
>> >
>> > -       rsize = ((report->size - 1) >> 3) + 1;
>> > +       rsize = ((report->size - 1) >> 3) + 1 + (report->id > 0) + 7;
>>
>> Isn't "report->id" already covered by "if (report_enum->numbered)"
>> above? The test for "id > 0" won't work here as in this case
>> "report_enum->numbered" must already be set to true by the hid-desc
>> parser, doesn't it?
>
> Right, that one is not correct here, thanks.
>
>> Where exactly did you get the +7 from?
>
> Please see commit (the one I am not really proud of) 27ce405039bfe6d3.

Eh, I remember.. Ok, but the magic-mouse is BT right? So this commit
really breaks BT drivers:

commit b1a1442a23776756b254b69786848a94d92445ba
Author: Jiri Kosina <jkosina@...e.cz>
Date:   Mon Jun 3 11:27:48 2013 +0200

    HID: core: fix reporting of raw events

Problem is, if the raw_event() callback returned 0 earlier, we just
skipped raw input reports. However, we now always call the
hid_report_raw_event() helper. Which is normally fine, but the helper
expects the input buffer to be of size HID_MAX_REPORT_SIZE, which is
*not* true for HIDP. So the memset() writes over some random memory.

I don't know exactly how to fix it. HID_MAX_BUFFER_SIZE is too big to
be allocated on the stack, but we're in atomic-context here so a
kzalloc(rsize, GFP_ATOMIC) seems overkill. So I guess we'd have to
look into HIDP to make the skb big enough, but I'm not sure how we can
achieve that.

So off the top of my head, the best idea is to add "char
inbuf[HID_MAX_BUFFER_SIZE];" to the hidp_session object in HIDP and
copy every input-report into the buffer before passing to
hid_input_report().

Ideas?
David
--
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/