public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Michal Pecio <michal.pecio@gmail.com>
To: "Heitor Alves de Siqueira" <halves@igalia.com>
Cc: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	<linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<kernel-dev@igalia.com>,
	<syzbot+abbfd103085885cf16a2@syzkaller.appspotmail.com>,
	<stable@kernel.org>
Subject: Re: [PATCH v2] usb: usbtmc: reject invalid interrupt endpoints
Date: Thu, 30 Apr 2026 22:48:49 +0200	[thread overview]
Message-ID: <20260430224849.3322afb0.michal.pecio@gmail.com> (raw)
In-Reply-To: <DI6PD3MOZAXW.RNUBKGQG1O6M@igalia.com>

On Thu, 30 Apr 2026 15:31:55 -0300, Heitor Alves de Siqueira wrote:
> > I think a minimal fix which mostly preserves existing behavior would
> > be adding "urb->actual_length == 2" as a requirement for all USB488
> > notifications. Then any truncated message will be ignored and logged.  
> 
> Yes, that's my understanding as well! Although I don't think bNotify2
> would ever be zero in practice, this sounds like a good approach. I'll
> submit a v3 with this change plus the endpoint check from v2, hopefully
> that'll improve things for these edge cases.

With actual_length check, wMaxPacketSize check isn't critical anymore
because actual_length won't exceed URB buffer size.

> > wMaxPacketSize is a separate issue indeed and it seems that a USB488
> > device could legally set it to 1, though it would be crazy. Your v1
> > patch would probably make such devices work, if anyone cares.  
> 
> Honestly, I'm also more inclined to just reject endpoints with this
> configuration. This seems like a very niche edge-case, I'd be surprised
> if real hardware operated like this (famous last words? heh). I'm not
> sure if this would even be valid/legal, given your previous point on
> bNotify2 being one byte.

USBTMC spec refers to USB 2.0 section 5.7.3, which states that an
interrupt transfer may take multiple packets until either the IRP (URB)
is filled or a packet shorter than wMaxPacketSize (possibly 0) is sent.

So slow, inefficient and unlikely to exist - yes.
But illegal - not really. Such endpoint can deliver 2 byte messages.

Also, a non-USB488 device may be sending different, very simple 1 byte
messages, perhaps vendor specific ones. None of them are recognized by
the driver, but other functionality of such device could still work, so
rejecting it is overkill.

> Considering these devices do not work at all currently, checking if
> wMaxPacketSize and urb->actual_length are big enough seems like a
> saner approach and won't require bigger changes to the driver.

The only change to support USB488 devices with wMaxPacketSize == 1
should be increasing URB size to at least 2 bytes. But I wouldn't
bother when no such HW is known to exist, and surely not as part of
a barely related bugfix patch.

Regards,
Michal

  reply	other threads:[~2026-04-30 20:48 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-23 18:04 [PATCH v2] usb: usbtmc: reject invalid interrupt endpoints Heitor Alves de Siqueira
2026-04-23 22:28 ` Michal Pecio
2026-04-28 19:55   ` Heitor Alves de Siqueira
2026-04-28 22:16     ` Michal Pecio
2026-04-30 18:31       ` Heitor Alves de Siqueira
2026-04-30 20:48         ` Michal Pecio [this message]
2026-04-30 21:04           ` Michal Pecio

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=20260430224849.3322afb0.michal.pecio@gmail.com \
    --to=michal.pecio@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=halves@igalia.com \
    --cc=kernel-dev@igalia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stable@kernel.org \
    --cc=syzbot+abbfd103085885cf16a2@syzkaller.appspotmail.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