![]() 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 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 |