Re: [PATCH 0/3] usbio: fix two out-of-bounds accesses and a hang

skyexpoc <[email protected]> Sun, 26 Jul 2026 20:32:25 +0900
Newsgroups	org.kernel.vger.linux-i2c,org.kernel.vger.linux-kernel,org.kernel.vger.linux-usb
Message-ID	<e6ce9251-fcaa-4833-ae6b-12a5d46415ba@Canary>
I’m sorry. I should have included an attribution.
That was my oversight.

I did use an LLM this time to assist with the audit and translation. It performed variant analysis on the code around 8c6314489550, identified these three issues, and wrote the patches and changelog. I ran the build, reviewed the results, decided what was worth submitting, and sent everything out. The Signed-off-by line is mine, and I take responsibility for this work.

I have now read ⁠coding-assistants.rst and ⁠generated-content.rst. The following has been added to all three patches in v2:

Assisted-by: Claude:claude-opus-5 asan

The cover letter also explains what the tool did and what I did. There are no code changes compared with v1.

Thank you,

HE WEI

> On Sunday, Jul 26, 2026 at 8:11 PM, Greg Kroah-Hartman <[email protected] (mailto:[email protected])> wrote:
> On Sun, Jul 26, 2026 at 04:59:10PM +0900, HE WEI (ギカク) wrote:
> > This is variant analysis around 8c6314489550 ("usb: misc: usbio: bound
> > bulk IN response length to the received transfer"), which fixed a slab
> > out-of-bounds read in usbio_bulk_msg(). Re-reading that function,
> > usbio_ctrl_msg() and the I2C client built on top of them turned up three
> > further issues.
> >
> > Patch 1 fixes the length checks in usbio_ctrl_msg() and
> > usbio_bulk_msg(). They are computed as "<buf>_len - sizeof(*pkt)", u16
> > minus size_t, which wraps for any endpoint smaller than the protocol
> > header. usb_parse_endpoint() only clamps wMaxPacketSize downwards, so
> > a device can advertise 1 and turn the check off entirely; the first I2C
> > transfer then overflows a one byte slab object. The overflow completes
> > before usb_bulk_msg() is reached, so it does not depend on the host
> > controller accepting the transfer. Fixed by validating the three
> > lengths once in usbio_probe(), which is the only place that assigns
> > them.
> >
> > Patch 2 is the equivalent arithmetic in i2c-usbio.c, where the overhead
> > is 10 rather than 5. The interesting value there is exactly 10: the
> > chunk size is then 0 and the split loop in usbio_i2c_read() never
> > advances, so a device that keeps answering keeps the loop alive
> > uninterruptibly while holding usbio->bulk_mutex. That is a hang rather
> > than memory corruption, and patch 1 does not address it. This is also
> > the one patch in the series with a behaviour change -- a bridge whose
> > bulk wMaxPacketSize is below 11 no longer gets an I2C adapter -- and the
> > commit message spells out why such an adapter could never have
> > completed a transfer in the first place.
> >
> > Patch 3 is a different bug from the two above, and the one that affects
> > existing hardware. usbio_ctrl_msg() and usbio_bulk_msg() hex dump the
> > reply with "%*phN" using a length the device supplied and that has not
> > been validated yet. hex_string() caps the field width at 64, not at
> > the buffer size, so the dump reads up to byte 67 of ctrlbuf and byte 68
> > of rxbuf: out of bounds for every low, full and high speed ep0 packet
> > size and for the bulk sizes these bridges actually use. It needs no
> > malformed descriptor at all, only the dev_dbg() calls enabled.
> >
> > The new probe checks do not reject any supported bridge: the low, full
> > and high speed devices this driver binds to have an ep0 packet size of
> > at least 8, and they use bulk endpoints of 64, or 63 via
> > USBIO_QUIRK_BULK_MAXP_63.
> >
> > What was verified, and what was not:
> >
> > - All three build cleanly with x86_64 gcc 14.2.0 at W=1, with
> > CONFIG_DYNAMIC_DEBUG both disabled and enabled.
> >
> > - The out-of-bounds accesses in patches 1 and 3, and the
> > non-terminating loop in patch 2, reproduce against a userspace model
> > that copies the struct definitions from usbio.h and the checks and
> > stores from usbio.c and i2c-usbio.c verbatim, with only
> > devm_kzalloc(), usb_bulk_msg() and usb_control_msg() replaced; the
> > memory errors are reported by AddressSanitizer. The same source
> > built with the patches applied is clean, and both a normal device
> > (ep0 64, bulk 64/64) and the USBIO_QUIRK_BULK_MAXP_63 path still
> > work.
> >
> > - The decision space was swept exhaustively, endpoint sizes 0..2048
> > against caller lengths 0..4101: the mainline bulk check can be
> > bypassed for exactly 0..4 and the control check for exactly 0..3.
> > After patch 1 no accepted endpoint size admits an overflow, and no
> > size that could carry a payload byte is rejected.
> >
> > - I have not run any of this on hardware or on dummy_hcd. The
> > reachability arguments are read from usbcore and from
> > lib/vsprintf.c, not observed at runtime. I am happy to build a
> > raw-gadget reproducer if that would help review.
> >
> > Patches 1 and 3 touch drivers/usb/misc/usbio.c and patch 2 touches
> > drivers/i2c/busses/i2c-usbio.c; MAINTAINERS lists all three files under
> > the same INTEL USBIO entry, so they are sent as one series. They apply
> > to mainline 3dab139d4795, to usb-next and to usb-testing.
> >
> > HE WEI (ギカク) (3):
> > usb: misc: usbio: reject endpoints smaller than the packet header
> > i2c: usbio: reject bridges with undersized transfer buffers
> > usb: misc: usbio: bound the debug hex dumps by the received length
> >
> > drivers/i2c/busses/i2c-usbio.c | 14 ++++++++++++
> > drivers/usb/misc/usbio.c | 50 ++++++++++++++++++++++++++++++++++++++----
> > 2 files changed, 60 insertions(+), 4 deletions(-)
>
> As you obviously used a LLM for these, why did it not properly tag the
> changes with an "Assisted-by:" tag? That is required.
>
> thanks,
>
> greg k-h
signature.asc (application/pgp-signature, 885 B)