From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EB1EFC4332F for ; Sun, 20 Nov 2022 17:54:35 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 6FBF185484; Sun, 20 Nov 2022 18:54:33 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="PFG1rhsV"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 7B5838541A; Sun, 20 Nov 2022 16:29:48 +0100 (CET) Received: from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com [IPv6:2a00:1450:4864:20::12c]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 3ED9085436 for ; Sun, 20 Nov 2022 16:29:46 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=szymon.heidrich@gmail.com Received: by mail-lf1-x12c.google.com with SMTP id bp15so15433511lfb.13 for ; Sun, 20 Nov 2022 07:29:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=aZeqEC1/4yIpEWuxKYqJD5F8JjfWMLJ96Ual5ZH+4Wg=; b=PFG1rhsVKa1K7FlqaBnky73CHS2UHe4ezRb2E2SLSkK+An5yNnUdGAqkJDDzIfVE11 VPmbO+GGEEc3cUXICo8oqUdOj6Y+oIpkCOyXDSj5ecYJGPIk0NvQJqUm+1HDswnltRUq dsNG+S5E/TTvLeQSTAC0I0wblWsSDYBaqLvDFHtIluwOUkz3KaMjmUX1SlEtXYrr3Z3S GMzKIc54x99EOvOFr5L7LXqGG2UfYix1B6Afsc51EjoDO4cWUle8FcOyU+li24eT9P6W aC6Fd6swlQXd4BrBWB+TeMyDj/gnPYm8v8SqrzUwAvqSJ/eipCOvAOokoV6hqAvOKIVa fjYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=aZeqEC1/4yIpEWuxKYqJD5F8JjfWMLJ96Ual5ZH+4Wg=; b=4uu1gvQl3eyPelRP9GrH6QJx+GyMmKeekG1/EZnRwo1lDKIb+fJ6s/epWUiTqwdCbP tIKqyGMnGmnsgRU8wh6np4ALCKyLmngGOn+ZRrcjJEmbvtTpc3NzqKs2lK3YTLvX431H FTbNlHS0eGGGp8G0kFnnyJ7kpCvSuc/iiBGLucJy5SwtbKKKSRq0cwkfAVMPlWX1yBas uuOQJFM2JN6r+yRhzpcKIQymjBDFbiz7xyMAfkMRjlYsiPd5xBClbxSyNp44qdJX4inN KP9sFcYnRP518eR2ZNt6w92luPe9dj0d5yH+VuoQB+l503GHHqkXnWIvELycg6I8xlLg u+uQ== X-Gm-Message-State: ANoB5pnbqP4GVI64kbwVRFvx4Zk3Sbxx9Zu1fv2Mswkwvd6tYK6WsS/W tgHvXJdglcmV5YGa1sTQh9U= X-Google-Smtp-Source: AA0mqf5rjBFFBl3qxt2OjG89Ru+AZpHX7Ds1i5wRWt5VVo1vp8edRBGfhxgJblpgTqzrLkvweXFX8A== X-Received: by 2002:a19:4f56:0:b0:4af:cd2:f8df with SMTP id a22-20020a194f56000000b004af0cd2f8dfmr4614522lfk.586.1668958185358; Sun, 20 Nov 2022 07:29:45 -0800 (PST) Received: from [192.168.50.20] (077222237105.warszawa.vectranet.pl. [77.222.237.105]) by smtp.gmail.com with ESMTPSA id d3-20020a05651233c300b004a0589786ddsm1593329lfg.69.2022.11.20.07.29.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 20 Nov 2022 07:29:44 -0800 (PST) Message-ID: <18de8ae7-fda2-ce0c-b83b-98c3af85aa9d@gmail.com> Date: Sun, 20 Nov 2022 16:29:43 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: Re: [PATCH] Prevent buffer overflow on USB control endpoint To: Marek Vasut , Fabio Estevam , Lukasz Majewski Cc: u-boot@lists.denx.de References: <20221117094847.60409-1-szymon.heidrich@gmail.com> <030344eb-e9d8-2bf9-a2c3-f124a32f323b@denx.de> Content-Language: en-US From: Szymon Heidrich In-Reply-To: <030344eb-e9d8-2bf9-a2c3-f124a32f323b@denx.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Mailman-Approved-At: Sun, 20 Nov 2022 18:54:32 +0100 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On 20/11/2022 15:43, Marek Vasut wrote: > On 11/17/22 12:50, Fabio Estevam wrote: >> [Adding Lukasz and Marek] >> >> On Thu, Nov 17, 2022 at 6:50 AM Szymon Heidrich >> wrote: >>> >>> Assure that the control endpoint buffer of size USB_BUFSIZ (4096) >>> can not be overflown during handling of USB control transfer >>> requests with wLength greater than USB_BUFSIZ. >>> >>> Signed-off-by: Szymon Heidrich >>> --- >>>   drivers/usb/gadget/composite.c | 11 +++++++++++ >>>   1 file changed, 11 insertions(+) >>> >>> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c >>> index 2a309e624e..cb89f6dca9 100644 >>> --- a/drivers/usb/gadget/composite.c >>> +++ b/drivers/usb/gadget/composite.c >>> @@ -1019,6 +1019,17 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) >>>          u8                              endp; >>>          struct usb_configuration        *c; >>> >>> +       if (w_length > USB_BUFSIZ) { >>> +               if (ctrl->bRequestType & USB_DIR_IN) { >>> +                       /* Cast away the const, we are going to overwrite on purpose. */ >>> +                       __le16 *temp = (__le16 *)&ctrl->wLength; >>> +                       *temp = cpu_to_le16(USB_BUFSIZ); >>> +                       w_length = USB_BUFSIZ; > > Won't this end up sending corrupted packets in case they are longer than USB_BUFSIZ ? > > Where do such long packets come from ? > > What is the test-case ? The USB host will not attempt to retrieve more than wLenght bytes during transfer phase. If the device would erroneously attempt to provide more data it would result in an unexpected state. In case of most implementations the buffer for endpoint 0 along with max control transfer is limited to 4096 bytes (USB_BUFSIZ for U-Boot and Linux kernel). Still according to the USB specification wLength is two bytes an the device may receive requests with wLength larger than 4096 bytes e.g. in case of a custom/malicious USB host. For example one may build libusb with MAX_CTRL_BUFFER_LENGTH altered to 0xffff and this will allow the host to send requests with wLength up to 0xffff. In this case the original implementation may result in buffer overflows as in multiple locations a value directly derived from wLength is set as the transfer phase length. With the change applied IN requests with wLength larger than USB_BUFSIZ will be trimmed to USB_BUFSIZ, otherwise the host would read wLength-USB_BUFSIZ past cdev->req->buf. I am not aware of any cases where more than USB_BUFSIZ would be provided from a buffer other than cdev->req->buf. In case I missed such case please let me know.