From patchwork Mon Oct 12 13:26:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg KH X-Patchwork-Id: 317648 Delivered-To: patch@linaro.org Received: by 2002:a92:d603:0:0:0:0:0 with SMTP id w3csp4652347ilm; Mon, 12 Oct 2020 07:05:02 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzmEPCRBHOPwFATB+hAdgrXuuS4nAY+poz5pGbC2sBpWfLGerM4q11AWvfZKXd7c8Ts81sH X-Received: by 2002:aa7:d892:: with SMTP id u18mr14796461edq.305.1602511502392; Mon, 12 Oct 2020 07:05:02 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1602511502; cv=none; d=google.com; s=arc-20160816; b=0vM9dMFgfLlVZzg88oVnyds2KGOVGkp8AqqScqlaKagsmlPOgkcH02ieZenVMj4iVN iJIT/oH4pbo1/SmX5H5yosLMTcnxSf2RPFXoGrJc1YIOo0Gy+83pfCjmh2wt8BiOdoA5 HZrg+PNa0TMA/eICZ4qIIu5UtdeZ3KBYxOOvjB2/MFLMgTumGQpwpbdDkKCfBdYnD+LJ Gu02uBTybZmqvP/DWUXpldeip8NBscJe+6xKyYQsHvboW0Z0NlGgViKs6JigKgvJ35KF nu+d2DKmGsNRpMD8JqtfoT3iLuMf6/rvzng2aBJKDnc4ZjPqK9W5m29PVDPIkBJuDfye ADmg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=Fu1wPgvdgdtx91FullMfonUEdqL5LObXxbYUZI7IRrs=; b=eIfxxMHD9MLezhiAzB1LwRdB2L3/ozlLC6NMHVGCL5JIt2MMV1l21M8jLveW38parS SceSOoymHMP6LIOW1FagwXht2ZpcHWcCWpOZBZlfidGAnmAdEo4wCpD/Qy50mlEUiab0 DGXnnqrmhMh1jDSiPZ6l3ffMRMtQ2KhuqdsUlShxTJovUsh8sZyzsvm90Pk3aQNqXnLL x3FgjQnobd16RwZbCRsa4o6R86gHvfmkMUwdJ4Ems1kgkNf0XdQZFcrmoacaZtJlFFxO lwUd8CMrt1NVgQCwm7x6rXt10VnLk4l1oWtx11mVnIfFqI1ybCe8r2mxBGeJs2rsNpDj Qa5Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=YJlbH0Be; spf=pass (google.com: domain of stable-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id g14si12819056ejr.221.2020.10.12.07.05.02; Mon, 12 Oct 2020 07:05:02 -0700 (PDT) Received-SPF: pass (google.com: domain of stable-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=YJlbH0Be; spf=pass (google.com: domain of stable-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388948AbgJLOE5 (ORCPT + 15 others); Mon, 12 Oct 2020 10:04:57 -0400 Received: from mail.kernel.org ([198.145.29.99]:38608 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730396AbgJLNgB (ORCPT ); Mon, 12 Oct 2020 09:36:01 -0400 Received: from localhost (83-86-74-64.cable.dynamic.v4.ziggo.nl [83.86.74.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 4750320678; Mon, 12 Oct 2020 13:35:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1602509759; bh=So+kyJqE4R5w+GLcDUp3WYF+xV+I884hMRFXdDTon20=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=YJlbH0Be/m76fopi/3Dx4GnFpTvGbQJnmQbI5Ee4mtXf3sf2ACQaWGf1ChZ59SVTW A1/MfzqG80s9lBV4SnuFf26Yv7k4t2G5I3FEorRRjR/6JLBWnzQ4C0t3qNgjgwtf7j 0fIp3FLIZZtV1vP/yPifnOiRlVlt3QDiGjZ9e8k0= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Ilja Van Sprundel , Brooke Basile , stable , Bryan ODonoghue Subject: [PATCH 4.14 05/70] USB: gadget: f_ncm: Fix NDP16 datagram validation Date: Mon, 12 Oct 2020 15:26:21 +0200 Message-Id: <20201012132630.470022726@linuxfoundation.org> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20201012132630.201442517@linuxfoundation.org> References: <20201012132630.201442517@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Bryan O'Donoghue commit 2b405533c2560d7878199c57d95a39151351df72 upstream. commit 2b74b0a04d3e ("USB: gadget: f_ncm: add bounds checks to ncm_unwrap_ntb()") adds important bounds checking however it unfortunately also introduces a bug with respect to section 3.3.1 of the NCM specification. wDatagramIndex[1] : "Byte index, in little endian, of the second datagram described by this NDP16. If zero, then this marks the end of the sequence of datagrams in this NDP16." wDatagramLength[1]: "Byte length, in little endian, of the second datagram described by this NDP16. If zero, then this marks the end of the sequence of datagrams in this NDP16." wDatagramIndex[1] and wDatagramLength[1] respectively then may be zero but that does not mean we should throw away the data referenced by wDatagramIndex[0] and wDatagramLength[0] as is currently the case. Breaking the loop on (index2 == 0 || dg_len2 == 0) should come at the end as was previously the case and checks for index2 and dg_len2 should be removed since zero is valid. I'm not sure how much testing the above patch received but for me right now after enumeration ping doesn't work. Reverting the commit restores ping, scp, etc. The extra validation associated with wDatagramIndex[0] and wDatagramLength[0] appears to be valid so, this change removes the incorrect restriction on wDatagramIndex[1] and wDatagramLength[1] restoring data processing between host and device. Fixes: 2b74b0a04d3e ("USB: gadget: f_ncm: add bounds checks to ncm_unwrap_ntb()") Cc: Ilja Van Sprundel Cc: Brooke Basile Cc: stable Signed-off-by: Bryan O'Donoghue Link: https://lore.kernel.org/r/20200920170158.1217068-1-bryan.odonoghue@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/usb/gadget/function/f_ncm.c | 30 ++---------------------------- 1 file changed, 2 insertions(+), 28 deletions(-) --- a/drivers/usb/gadget/function/f_ncm.c +++ b/drivers/usb/gadget/function/f_ncm.c @@ -1210,7 +1210,6 @@ static int ncm_unwrap_ntb(struct gether const struct ndp_parser_opts *opts = ncm->parser_opts; unsigned crc_len = ncm->is_crc ? sizeof(uint32_t) : 0; int dgram_counter; - bool ndp_after_header; /* dwSignature */ if (get_unaligned_le32(tmp) != opts->nth_sign) { @@ -1237,7 +1236,6 @@ static int ncm_unwrap_ntb(struct gether } ndp_index = get_ncm(&tmp, opts->ndp_index); - ndp_after_header = false; /* Run through all the NDP's in the NTB */ do { @@ -1253,8 +1251,6 @@ static int ncm_unwrap_ntb(struct gether ndp_index); goto err; } - if (ndp_index == opts->nth_size) - ndp_after_header = true; /* * walk through NDP @@ -1333,37 +1329,13 @@ static int ncm_unwrap_ntb(struct gether index2 = get_ncm(&tmp, opts->dgram_item_len); dg_len2 = get_ncm(&tmp, opts->dgram_item_len); - if (index2 == 0 || dg_len2 == 0) - break; - /* wDatagramIndex[1] */ - if (ndp_after_header) { - if (index2 < opts->nth_size + opts->ndp_size) { - INFO(port->func.config->cdev, - "Bad index: %#X\n", index2); - goto err; - } - } else { - if (index2 < opts->nth_size + opts->dpe_size) { - INFO(port->func.config->cdev, - "Bad index: %#X\n", index2); - goto err; - } - } if (index2 > block_len - opts->dpe_size) { INFO(port->func.config->cdev, "Bad index: %#X\n", index2); goto err; } - /* wDatagramLength[1] */ - if ((dg_len2 < 14 + crc_len) || - (dg_len2 > frame_max)) { - INFO(port->func.config->cdev, - "Bad dgram length: %#X\n", dg_len); - goto err; - } - /* * Copy the data into a new skb. * This ensures the truesize is correct @@ -1380,6 +1352,8 @@ static int ncm_unwrap_ntb(struct gether ndp_len -= 2 * (opts->dgram_item_len * 2); dgram_counter++; + if (index2 == 0 || dg_len2 == 0) + break; } while (ndp_len > 2 * (opts->dgram_item_len * 2)); } while (ndp_index);