Message ID | 20221109084450.2181848-1-gregkh@linuxfoundation.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp224428wru; Wed, 9 Nov 2022 00:56:25 -0800 (PST) X-Google-Smtp-Source: AMsMyM6FTyZlaJeYKHu4ZZOAEJdura6iCeHcHe3udYm7fvUsJO9Jwyv6mNsnv/6wVE7i+Br9yDF9 X-Received: by 2002:a17:90b:3c8a:b0:213:e48a:3ac5 with SMTP id pv10-20020a17090b3c8a00b00213e48a3ac5mr52955508pjb.48.1667984185553; Wed, 09 Nov 2022 00:56:25 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1667984185; cv=none; d=google.com; s=arc-20160816; b=qjY8oc5cpPg6KZ0++2gaKTM0ztSYtG4J4DssKBei+UCL35LfAy1CQUdnqS0ZeieIJV 22stjoS9iEl/AXOgiSsSfa9W2a7fFlNachc68wNgg+pqGIgiAcRqOGrsVfKh37MDVdIG HedbVyqVCweyGjd5tnJqZ+jpNqJskViN/B+NOLmP2gVrfGyzKlq57o9whCyEU9Xk4RlL CZyf3xERe3Fs+USo27dvtA8V0aeDiXtDSGLwr1vg00iweeibSNc73NRBcUesA3OMV8dJ A6kdv+ObTpVn6CUKvZ34u9J4BjqYmTCO0JyQi3UlY+vJAHQfX5JdjmD4CkHK5HQVetpE 1XMA== 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 :message-id:date:subject:cc:to:from:dkim-signature; bh=pFQevyd90+W84I1HvSk23HQcl6eWYp2BLq2s7POXu6E=; b=vKFIB5gbEHiVebYVm1x9/mOnpyAoqVsdcBz/l3Q+SoaJlhvIzJTGw2sGNAhv947qjo srL548Byn4NoBG1Y2IXs4piSg4ueKWfQvfhYN74dgxwiChMHfAODhffyRk/X1TI7+CVj iW7R78b78nGEKKweG/8UXB/qY7nxIOGet6MnAXqux9UiiuQ0SGdaGqVg4kCFgnKhj1FS JTcxpFrxTMKrwcc7c4yz6oeeaVdfK4ib4n9/cNH8+yC30k7VJswDpCQujLy4OJpVHW69 MWGMHhVuVSKkde0FbmDkHr7ZKviLVV96Lf8KO31dQszRIL1G8fE4iSBEdrea78+CTenj QM/Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=pBT6BNA5; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id i9-20020a63e449000000b004594e9f9b10si17359087pgk.188.2022.11.09.00.56.12; Wed, 09 Nov 2022 00:56:25 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=pBT6BNA5; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229809AbiKIIpG (ORCPT <rfc822;dexuan.linux@gmail.com> + 99 others); Wed, 9 Nov 2022 03:45:06 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55370 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229591AbiKIIpE (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 9 Nov 2022 03:45:04 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 14572231; Wed, 9 Nov 2022 00:44:58 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 650C3B81D46; Wed, 9 Nov 2022 08:44:57 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8FC5DC433C1; Wed, 9 Nov 2022 08:44:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1667983496; bh=gubP5wUhKwsJa2PWsBZNowl7hrwhl0rnCsCJXTmXSnY=; h=From:To:Cc:Subject:Date:From; b=pBT6BNA5qf/wh+ppx/PLc1yeErH5HcRlf2fmSiKdNg+3QN4nB1uGZQEjSZw/aQs3o ZGnMI6DtDXlqry99ie+yx/ba0zE6wv7pd1EqFq8A4rfkiQULuLfEjvsMVfeZxbdeiR m4spPUsmqc4dkSfd4OiKDbyaIoQjMtU8IfJZwc4k= From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> To: linux-usb@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Felipe Balbi <balbi@kernel.org>, Jakob Koschel <jakobkoschel@gmail.com>, Randy Dunlap <rdunlap@infradead.org>, Ira Weiny <ira.weiny@intel.com>, Alan Stern <stern@rowland.harvard.edu>, Linus Torvalds <torvalds@linux-foundation.org> Subject: [PATCH v2] USB: gadget: dummy_hcd: switch char * to u8 * Date: Wed, 9 Nov 2022 09:44:50 +0100 Message-Id: <20221109084450.2181848-1-gregkh@linuxfoundation.org> X-Mailer: git-send-email 2.38.1 MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2518; i=gregkh@linuxfoundation.org; h=from:subject; bh=gubP5wUhKwsJa2PWsBZNowl7hrwhl0rnCsCJXTmXSnY=; b=owGbwMvMwCRo6H6F97bub03G02pJDMnZGQ2T+6cf/i25Rm1n+DX7Tw1cHwu23cg6IX06d8ln9+0/ 30pO74hlYRBkYpAVU2T5so3n6P6KQ4pehranYeawMoEMYeDiFICJFGkxzBW4+Z1X74b99RXchX5pXY oT1jktms+wYGNFXQzT4ll67TtDRHt5P+/749ohDwA= X-Developer-Key: i=gregkh@linuxfoundation.org; a=openpgp; fpr=F4B60CC5BF78C2214A313DCB3147D40DDB2DFB29 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1749008185442574031?= X-GMAIL-MSGID: =?utf-8?q?1749008185442574031?= |
Series |
[v2] USB: gadget: dummy_hcd: switch char * to u8 *
|
|
Commit Message
Greg KH
Nov. 9, 2022, 8:44 a.m. UTC
The function handle_control_request() casts the urb buffer to a char *,
and then treats it like a unsigned char buffer when assigning data to
it. On some architectures, "char" is really signed, so let's just
properly set this pointer to a u8 to take away any potential problems as
that's what is really wanted here.
Use put_unaligned_le16() to copy the full 16bits into the buffer,
it's not a problem just yet as only 7 bits are being used here, but this
protects us when/if the USB spec changes in the future to define more of
these bits.
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Jakob Koschel <jakobkoschel@gmail.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2: - use put_unaligned_le16() on Linus's recommendation as a simpler and
more obvious way to describe the data being copied here.
- update device: comment based on Alan's review
drivers/usb/gadget/udc/dummy_hcd.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
Comments
On Wed, Nov 09, 2022 at 09:44:50AM +0100, Greg Kroah-Hartman wrote: > The function handle_control_request() casts the urb buffer to a char *, > and then treats it like a unsigned char buffer when assigning data to > it. On some architectures, "char" is really signed, so let's just > properly set this pointer to a u8 to take away any potential problems as > that's what is really wanted here. > > Use put_unaligned_le16() to copy the full 16bits into the buffer, > it's not a problem just yet as only 7 bits are being used here, but this > protects us when/if the USB spec changes in the future to define more of > these bits. > > Cc: Felipe Balbi <balbi@kernel.org> > Cc: Jakob Koschel <jakobkoschel@gmail.com> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Ira Weiny <ira.weiny@intel.com> > Cc: Alan Stern <stern@rowland.harvard.edu> > Reported-by: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > v2: - use put_unaligned_le16() on Linus's recommendation as a simpler and > more obvious way to describe the data being copied here. > - update device: comment based on Alan's review > > drivers/usb/gadget/udc/dummy_hcd.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c > index 899ac9f9c279..7c59c20c8623 100644 > --- a/drivers/usb/gadget/udc/dummy_hcd.c > +++ b/drivers/usb/gadget/udc/dummy_hcd.c > @@ -1740,13 +1740,13 @@ static int handle_control_request(struct dummy_hcd *dum_hcd, struct urb *urb, > if (setup->bRequestType == Dev_InRequest > || setup->bRequestType == Intf_InRequest > || setup->bRequestType == Ep_InRequest) { > - char *buf; > + u8 *buf; > /* > - * device: remote wakeup, selfpowered > + * device: remote wakeup, selfpowered, LTM, U1, or U2 > * interface: nothing > * endpoint: halt > */ > - buf = (char *)urb->transfer_buffer; > + buf = urb->transfer_buffer; > if (urb->transfer_buffer_length > 0) { > if (setup->bRequestType == Ep_InRequest) { > ep2 = find_endpoint(dum, w_index); > @@ -1755,10 +1755,9 @@ static int handle_control_request(struct dummy_hcd *dum_hcd, struct urb *urb, > break; > } > buf[0] = ep2->halted; > - } else if (setup->bRequestType == > - Dev_InRequest) { > - buf[0] = (u8)dum->devstatus; > - } else > + } else if (setup->bRequestType == Dev_InRequest) > + put_unaligned_le16(dum->devstatus, buf); This is tempting but wrong. At this point we haven't checked how big the transfer_buffer is; if it is only one byte long then this call will overflow the buffer. Notice the "if" statement three lines below. Alan Stern > + else > buf[0] = 0; > } > if (urb->transfer_buffer_length > 1) > -- > 2.38.1 >
On Wed, Nov 09, 2022 at 10:03:52AM -0500, Alan Stern wrote: > On Wed, Nov 09, 2022 at 09:44:50AM +0100, Greg Kroah-Hartman wrote: > > The function handle_control_request() casts the urb buffer to a char *, > > and then treats it like a unsigned char buffer when assigning data to > > it. On some architectures, "char" is really signed, so let's just > > properly set this pointer to a u8 to take away any potential problems as > > that's what is really wanted here. > > > > Use put_unaligned_le16() to copy the full 16bits into the buffer, > > it's not a problem just yet as only 7 bits are being used here, but this > > protects us when/if the USB spec changes in the future to define more of > > these bits. > > > > Cc: Felipe Balbi <balbi@kernel.org> > > Cc: Jakob Koschel <jakobkoschel@gmail.com> > > Cc: Randy Dunlap <rdunlap@infradead.org> > > Cc: Ira Weiny <ira.weiny@intel.com> > > Cc: Alan Stern <stern@rowland.harvard.edu> > > Reported-by: Linus Torvalds <torvalds@linux-foundation.org> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > v2: - use put_unaligned_le16() on Linus's recommendation as a simpler and > > more obvious way to describe the data being copied here. > > - update device: comment based on Alan's review > > > > drivers/usb/gadget/udc/dummy_hcd.c | 13 ++++++------- > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c > > index 899ac9f9c279..7c59c20c8623 100644 > > --- a/drivers/usb/gadget/udc/dummy_hcd.c > > +++ b/drivers/usb/gadget/udc/dummy_hcd.c > > @@ -1740,13 +1740,13 @@ static int handle_control_request(struct dummy_hcd *dum_hcd, struct urb *urb, > > if (setup->bRequestType == Dev_InRequest > > || setup->bRequestType == Intf_InRequest > > || setup->bRequestType == Ep_InRequest) { > > - char *buf; > > + u8 *buf; > > /* > > - * device: remote wakeup, selfpowered > > + * device: remote wakeup, selfpowered, LTM, U1, or U2 > > * interface: nothing > > * endpoint: halt > > */ > > - buf = (char *)urb->transfer_buffer; > > + buf = urb->transfer_buffer; > > if (urb->transfer_buffer_length > 0) { > > if (setup->bRequestType == Ep_InRequest) { > > ep2 = find_endpoint(dum, w_index); > > @@ -1755,10 +1755,9 @@ static int handle_control_request(struct dummy_hcd *dum_hcd, struct urb *urb, > > break; > > } > > buf[0] = ep2->halted; > > - } else if (setup->bRequestType == > > - Dev_InRequest) { > > - buf[0] = (u8)dum->devstatus; > > - } else > > + } else if (setup->bRequestType == Dev_InRequest) > > + put_unaligned_le16(dum->devstatus, buf); > > This is tempting but wrong. At this point we haven't checked how big > the transfer_buffer is; if it is only one byte long then this call will > overflow the buffer. Notice the "if" statement three lines below. Ick, good catch. I'll redo this again... thanks, greg k-h
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c index 899ac9f9c279..7c59c20c8623 100644 --- a/drivers/usb/gadget/udc/dummy_hcd.c +++ b/drivers/usb/gadget/udc/dummy_hcd.c @@ -1740,13 +1740,13 @@ static int handle_control_request(struct dummy_hcd *dum_hcd, struct urb *urb, if (setup->bRequestType == Dev_InRequest || setup->bRequestType == Intf_InRequest || setup->bRequestType == Ep_InRequest) { - char *buf; + u8 *buf; /* - * device: remote wakeup, selfpowered + * device: remote wakeup, selfpowered, LTM, U1, or U2 * interface: nothing * endpoint: halt */ - buf = (char *)urb->transfer_buffer; + buf = urb->transfer_buffer; if (urb->transfer_buffer_length > 0) { if (setup->bRequestType == Ep_InRequest) { ep2 = find_endpoint(dum, w_index); @@ -1755,10 +1755,9 @@ static int handle_control_request(struct dummy_hcd *dum_hcd, struct urb *urb, break; } buf[0] = ep2->halted; - } else if (setup->bRequestType == - Dev_InRequest) { - buf[0] = (u8)dum->devstatus; - } else + } else if (setup->bRequestType == Dev_InRequest) + put_unaligned_le16(dum->devstatus, buf); + else buf[0] = 0; } if (urb->transfer_buffer_length > 1)