From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Julio Faracco <jcfaracco@gmail.com>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
gregkh@linuxfoundation.org, axboe@kernel.dk, tglx@linutronix.de,
damien.lemoal@wdc.com, dkadashev@gmail.com,
paul.gortmaker@windriver.com, zhouyanjie@wanyeetech.com,
niklas.cassel@wdc.com, macro@orcam.me.uk, caihuoqing@baidu.com
Subject: Re: [PATCH] usb: fixing some clang warnings inside usb host drivers
Date: Sun, 19 Dec 2021 16:59:44 +0900 [thread overview]
Message-ID: <0804469c-664a-219d-bb6a-b4e5f133edd9@i-love.sakura.ne.jp> (raw)
In-Reply-To: <Yb6d7tflQeJ+1Et2@rowland.harvard.edu>
On 2021/12/19 11:50, Alan Stern wrote:
> You should read this code in usb_submit_urb():
>
> max = usb_endpoint_maxp(&ep->desc);
> if (max <= 0) {
> dev_dbg(&dev->dev,
> "bogus endpoint ep%d%s in %s (bad maxpacket %d)\n",
> usb_endpoint_num(&ep->desc), is_out ? "out" : "in",
> __func__, max);
> return -EMSGSIZE;
> }
>
> As far as I know, every code path leading to qtd_fill() has to pass this
> test.
Excuse me, but surely qtd_fill() is using the result from usb_maxpacket()
----------------------------------------
static struct list_head *
qh_urb_transaction (
struct ehci_hcd *ehci,
struct urb *urb,
struct list_head *head,
gfp_t flags
) {
(...snipped...)
maxpacket = usb_maxpacket(urb->dev, urb->pipe, !is_input);
/*
* buffer gets wrapped in one or more qtds;
* last one may be "short" (including zero len)
* and may serve as a control status ack
*/
for (;;) {
int this_qtd_len;
this_qtd_len = qtd_fill(ehci, qtd, buf, this_sg_len, token,
maxpacket);
this_sg_len -= this_qtd_len;
len -= this_qtd_len;
buf += this_qtd_len;
(...snipped...)
}
----------------------------------------
and usb_maxpacket() may return 0 ?
----------------------------------------
static inline __u16
usb_maxpacket(struct usb_device *udev, int pipe, int is_out)
{
struct usb_host_endpoint *ep;
unsigned epnum = usb_pipeendpoint(pipe);
if (is_out) {
WARN_ON(usb_pipein(pipe));
ep = udev->ep_out[epnum];
} else {
WARN_ON(usb_pipeout(pipe));
ep = udev->ep_in[epnum];
}
if (!ep)
return 0;
/* NOTE: only 0x07ff bits are for packet size... */
return usb_endpoint_maxp(&ep->desc);
}
----------------------------------------
If we don't need to care about the possibility of returning 0 (including
all possible race conditions taken into account), please explain it as a
comment block.
next prev parent reply other threads:[~2021-12-19 8:00 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-18 4:24 [PATCH] usb: fixing some clang warnings inside usb host drivers Julio Faracco
2021-12-18 9:13 ` Greg KH
2021-12-18 10:49 ` Joe Perches
2021-12-18 18:05 ` Alan Stern
2021-12-19 1:41 ` Tetsuo Handa
2021-12-19 2:50 ` Alan Stern
2021-12-19 7:59 ` Tetsuo Handa [this message]
2021-12-19 15:46 ` Alan Stern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0804469c-664a-219d-bb6a-b4e5f133edd9@i-love.sakura.ne.jp \
--to=penguin-kernel@i-love.sakura.ne.jp \
--cc=axboe@kernel.dk \
--cc=caihuoqing@baidu.com \
--cc=damien.lemoal@wdc.com \
--cc=dkadashev@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=jcfaracco@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=macro@orcam.me.uk \
--cc=niklas.cassel@wdc.com \
--cc=paul.gortmaker@windriver.com \
--cc=stern@rowland.harvard.edu \
--cc=tglx@linutronix.de \
--cc=zhouyanjie@wanyeetech.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).