From patchwork Tue Oct 24 16:39:43 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Torokhov X-Patchwork-Id: 10025087 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E60AE6035E for ; Tue, 24 Oct 2017 16:39:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D3EEA22230 for ; Tue, 24 Oct 2017 16:39:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C494C28A3E; Tue, 24 Oct 2017 16:39:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5A83E22230 for ; Tue, 24 Oct 2017 16:39:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751318AbdJXQjs (ORCPT ); Tue, 24 Oct 2017 12:39:48 -0400 Received: from mail-oi0-f67.google.com ([209.85.218.67]:53958 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751330AbdJXQjr (ORCPT ); Tue, 24 Oct 2017 12:39:47 -0400 Received: by mail-oi0-f67.google.com with SMTP id h6so38140122oia.10; Tue, 24 Oct 2017 09:39:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=BSG1x69Od/AI27zGgs7OW3APtdWoe1YwPrYrJdkb/Dw=; b=Yz+NUSEo9SGcBPx0y337JgF9wjywP+yHCoXAZJcnlT/4Sh0XMRRWq0OobMhzYpPPMj xMUsx1rb1dFgwNKBEoFpDbc2cXOWBmMkeyT+Ez2IoUZcPqq0gjBDgtk2AhTHEsV6j687 bWzMn97tyDmoaPR9DN9w+UPxTOl8kI6ZXIKQGhCNW8i/8sgLn5TQR6+SKJ8mylK1pDUH DxOrcLm3mGGGhsjgv5G1BMQnKyAewgEUXy6ZgbTjlstCJoNuUWiobCec9CaPyC3Q+/VB 3fvTitpWgDIJ0otVZ8GXeoKhWC9g6zMIvJ881DzUH2o35CsWb+dvJLK0hJIAmh9YI8zj 1icA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=BSG1x69Od/AI27zGgs7OW3APtdWoe1YwPrYrJdkb/Dw=; b=G7/SrisrXU6es0qNITbPyqYhcSlyii6y8/vYHWkVlG1Tjzb525pX1rVCgOp6lrdLe2 l5SE49oA90vKIiLVrvqxdplYKYw5vE/3ily1sq/7UbsqSVV7feQytPhKVFBgZLYKWAcI LYHWB1yHkRI3BwPD/p1PZ49D9Vg8l0KKZz0bxEfCGvO+6u+dnm9KD4PJF2Ym8z/6QKfJ UHEyL9roLO1Jgyz9dkEWKefGUwt8ehWrMp7GCWKmLaswwJoYNxvdMikU6pEoWmOp9j1h Wgd3GMpseJxQuMFwDB4tLwjKuqBBZOWyrPm0/7SYSMP1ODBSPB2yv9g0joB8rb0al2eK hnqQ== X-Gm-Message-State: AMCzsaUuBx/0460Bq5glnK3IGJQFwgroy42jtjIb2Zy83hwiWV59trkn Vklo7FSGBVUSfOb/frMyW/ib1Bpd X-Google-Smtp-Source: ABhQp+R8irxI3pHqLNqLS5HVA2ZYMhz3z5jIAvzuhMaJJFNJdcUko4W04RDevL4UMUBM9SlgWxU6ZQ== X-Received: by 10.157.15.178 with SMTP id d47mr9406556otd.85.1508863186218; Tue, 24 Oct 2017 09:39:46 -0700 (PDT) Received: from dtor-ws ([2620:0:1000:1611:da80:8749:c06f:9515]) by smtp.gmail.com with ESMTPSA id c135sm296674oih.7.2017.10.24.09.39.45 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 24 Oct 2017 09:39:45 -0700 (PDT) Date: Tue, 24 Oct 2017 09:39:43 -0700 From: Dmitry Torokhov To: Andrey Konovalov Cc: linux-input@vger.kernel.org, LKML Subject: Re: [PATCH] Input: gtco - fix potential out-of-bound access Message-ID: <20171024163943.gpkmpgexfuqe5vun@dtor-ws> References: <20171024052823.jpetpk2bfazfvfah@dtor-ws> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Oct 24, 2017 at 01:04:03PM +0200, Andrey Konovalov wrote: > On Tue, Oct 24, 2017 at 7:28 AM, Dmitry Torokhov > wrote: > > parse_hid_report_descriptor() has a while (i < length) loop, which > > only guarantees that there's at least 1 byte in the buffer, but the > > loop body can read multiple bytes which causes out-of-bounds access. > > > > Reported-by: Andrey Konovalov > > Signed-off-by: Dmitry Torokhov > > --- > > drivers/input/tablet/gtco.c | 24 +++++++++++++++++------- > > 1 file changed, 17 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/input/tablet/gtco.c b/drivers/input/tablet/gtco.c > > index b796e891e2ee..0351203b8c24 100644 > > --- a/drivers/input/tablet/gtco.c > > +++ b/drivers/input/tablet/gtco.c > > @@ -230,13 +230,24 @@ static void parse_hid_report_descriptor(struct gtco *device, char * report, > > > > /* Walk this report and pull out the info we need */ > > while (i < length) { > > - prefix = report[i]; > > - > > - /* Skip over prefix */ > > - i++; > > + prefix = report[i++]; > > > > /* Determine data size and save the data in the proper variable */ > > - size = PREF_SIZE(prefix); > > + if (PREF_SIZE(prefix) < 1) { > > AFAIU PREF_SIZE(prefix) == 0 is a perfectly valid item data size. Fair enough. How about the below instead then? diff --git a/drivers/input/tablet/gtco.c b/drivers/input/tablet/gtco.c index b796e891e2ee..7d8e9fb831c4 100644 --- a/drivers/input/tablet/gtco.c +++ b/drivers/input/tablet/gtco.c @@ -230,13 +230,17 @@ static void parse_hid_report_descriptor(struct gtco *device, char * report, /* Walk this report and pull out the info we need */ while (i < length) { - prefix = report[i]; - - /* Skip over prefix */ - i++; + prefix = report[i++]; /* Determine data size and save the data in the proper variable */ - size = PREF_SIZE(prefix); + size = (1U << PREF_SIZE(prefix)) >> 1; + if (size && i + size >= length) { + dev_err(ddev, + "Not enough data (need %d, have %d)\n", + i + size, length); + break; + } + switch (size) { case 1: data = report[i]; @@ -244,8 +248,7 @@ static void parse_hid_report_descriptor(struct gtco *device, char * report, case 2: data16 = get_unaligned_le16(&report[i]); break; - case 3: - size = 4; + case 4: data32 = get_unaligned_le32(&report[i]); break; }