From: Alan Stern <stern@rowland.harvard.edu>
To: "Bjørn Mork" <bjorn@mork.no>
Cc: Jiri Slaby <jslaby@suse.cz>, Vittorio Zecca <zeccav@gmail.com>,
<stable@vger.kernel.org>, USB list <linux-usb@vger.kernel.org>,
Linux kernel mailing list <linux-kernel@vger.kernel.org>
Subject: Re: UBSAN: Undefined behaviour in linux-4.7.2/drivers/usb/core/devio.c:1713:25
Date: Mon, 22 Aug 2016 16:40:32 -0400 (EDT) [thread overview]
Message-ID: <Pine.LNX.4.44L0.1608221628320.5195-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <87twecy2qa.fsf@miraculix.mork.no>
On Mon, 22 Aug 2016, Bjørn Mork wrote:
> Alan Stern <stern@rowland.harvard.edu> writes:
>
> > On Sun, 21 Aug 2016, Jiri Slaby wrote:
> >
> >> Cc: proper lists.
> >>
> >> ep->desc.bInterval seems to be 0 here.
> > As far as I can see, this isn't possible. The usb_parse_endpoint()
> > routine in drivers/usb/core/config.c is supposed to guarantee that
> > ep->desc.bInterval is never 0.
>
> That is if it is an ISO endpoint, right?
I can't tell; the bug report doesn't say. However, ep->desc.bInterval
is ignored for bulk and control endpoints, so it must be either
isochronous or interrupt.
> Maybe I misunderstand something fundamental, but the "||" strikes me as
> odd here:
>
> as->urb->stream_id = stream_id;
> if (uurb->type == USBDEVFS_URB_TYPE_ISO ||
> ps->dev->speed == USB_SPEED_HIGH)
> as->urb->interval = 1 << min(15, ep->desc.bInterval - 1);
> else
> as->urb->interval = ep->desc.bInterval;
> as->urb->context = as;
No, that's right (mostly -- we really should check for ps->dev->speed
>= USB_SPEED_SUPER as well as == USB_SPEED_HIGH).
> Typo?
USB uses two different encodings for endpoint intervals. The second
encoding above just gives the interval in frames; this is used for low-
and full-speed interrupt endpoints. The first encoding above is
exponential (it gives n where the actual interval is 2^(n-1) frames or
microframes); this is used for all isochronous endpoints and for
high-speed (or SuperSpeed etc.) interrupt endpoints.
See for example the definition of usb_fill_int_urb() in
include/linux/usb.h.
Alan Stern
next prev parent reply other threads:[~2016-08-22 20:40 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAFQ=mRQeV_UHYuU20ppHG6L=mNcr_cKxBMMM731XfWeoEO-L=g@mail.gmail.com>
2016-08-21 10:46 ` UBSAN: Undefined behaviour in linux-4.7.2/drivers/usb/core/devio.c:1713:25 Jiri Slaby
2016-08-22 17:43 ` Alan Stern
2016-08-22 20:21 ` Bjørn Mork
2016-08-22 20:40 ` Alan Stern [this message]
2016-08-22 20:57 ` Bjørn Mork
2016-08-22 21:06 ` Alan Stern
2016-08-22 21:45 ` Vittorio Zecca
2016-08-23 14:55 ` Alan Stern
2016-08-23 15:47 ` Vittorio Zecca
2016-08-23 18:58 ` 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=Pine.LNX.4.44L0.1608221628320.5195-100000@iolanthe.rowland.org \
--to=stern@rowland.harvard.edu \
--cc=bjorn@mork.no \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=zeccav@gmail.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).