Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752315AbdI1Io0 (ORCPT <rfc822;w@1wt.eu>);
        Thu, 28 Sep 2017 04:44:26 -0400
Received: from mail-it0-f42.google.com ([209.85.214.42]:50930 "EHLO
        mail-it0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751920AbdI1IoY (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 28 Sep 2017 04:44:24 -0400
X-Google-Smtp-Source: AOwi7QB0vaKSzdBwTOl7fXK3HnhunThxRyBaRQKpBtVVCRdfcYgzW2eCx+kL/Iohmla7NJ1n4erJ6872vabNoDPmg24=
MIME-Version: 1.0
In-Reply-To: <Pine.LNX.4.44L0.1709271013300.1390-100000@iolanthe.rowland.org>
References: <CAAZ5spCffHnU_EtnhKpP5cDqsEKaGMatpaqAJqorvph+vZm1Sw@mail.gmail.com>
 <Pine.LNX.4.44L0.1709271013300.1390-100000@iolanthe.rowland.org>
From: Jaejoong Kim <climbbb.kim@gmail.com>
Date: Thu, 28 Sep 2017 17:44:22 +0900
Message-ID: <CAL6iAaJQZ4JB6udi3UFyGM0uYYE4NECZ5fK8ujb+ANfwe9tYDw@mail.gmail.com>
Subject: Re: [PATCH] HID: usbhid: fix out-of-bounds bug
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Michel Hermier <michel.hermier@gmail.com>,
        Kostya Serebryany <kcc@google.com>,
        syzkaller <syzkaller@googlegroups.com>, Jiri Kosina <jikos@kernel.org>,
        USB list <linux-usb@vger.kernel.org>,
        Andrey Konovalov <andreyknvl@google.com>,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        Dmitry Vyukov <dvyukov@google.com>, linux-input@vger.kernel.org,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v8S8iWXp001585
Content-Length: 1567
Lines: 54

2017-09-27 23:29 GMT+09:00 Alan Stern <stern@rowland.harvard.edu>:
> On Wed, 27 Sep 2017, Michel Hermier wrote:
>
>> Le 27 sept. 2017 07:42, "Alan Stern" <stern@rowland.harvard.edu> a écrit :
>
>> > -       for (n = 0; n < hdesc->bNumDescriptors; n++)
>> > +       num_descriptors = min_t(int, hdesc->bNumDescriptors,
>> > +                               (hdesc->bLength - 6) / 3);
>> > +       for (n = 0; n < num_descriptors; n++)
>> >                 if (hdesc->desc[n].bDescriptorType == HID_DT_REPORT)
>> >                         rsize = le16_to_cpu(hdesc->desc[n].
>> wDescriptorLength);
>>
>> Yes, this is a lot better.

OK.

>>
>>
>> Is it possible to explicit the magic number 6 and 3 in the code. Currently,
>> it looks like it comes from no where.

I gree with you.

>
> Yes, it is possible.  The 6 is equal to
>
>         offsetof(struct hid_descriptor, desc)
>
> and the 3 is equal to
>
>         sizeof(struct hid_class_descriptor)
>
> (at least, I think it is -- the structure is marked as packed so its
> size should be 3).
>
> In this case I found the numbers to be more readable, but other people
> may have different opinions.

I will post V2 shortly.

>
>> I'm also wondering if this change will not affect some devices in the wild,
>> by rejecting hid descriptors with num descriptors == 0 ?
>
> It's possible, but I doubt it.  If such devices do exist, they should
> never have worked in the first place.  Certainly they would generate
> warnings or errors during enumeration because of their invalid
> descriptors.
>
> Alan Stern
>

Jaejoong