On Wed, Sep 30, 2026 at 8:32 PM Philippe Mathieu-Daudé <[email protected]> wrote: > > On 2026-07-29 9:57, [email protected] wrote: > > From: Marc-André Lureau <[email protected]> > > > > ccid_bulk_in_get() decrements bulk_in_pending_num when a slot becomes > > active, but the slot is only truly free after ccid_bulk_in_release(). A > > guest can fill all 8 ring slots, partially read one (leaving pos > > non-zero), then send another command whose response reuses the active > > slot. The stale pos > new len makes the uint32_t subtraction in > > ccid_bulk_in_copy_to_guest() wrap, reading past data[]. > > > > Move the decrement to ccid_bulk_in_release() so the slot stays busy > > until then. Add an assert(pos <= len) before the substraction. > > > > In practice, the device advertises bMaxCCIDBusySlots=1, so a conforming > > host never has more than one command in flight and the ring never fills. > > > > Fixes: CVE-2026-18204 > > Fixes: 367071447ec5 ("usb-ccid: add CCID bus") > > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4086 > > Reported-by: Warisjeet Singh <[email protected]> > > Signed-off-by: Marc-André Lureau <[email protected]> > > --- > > hw/usb/dev-smartcard-reader.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c > > index 964c142d1066..5f8b95be46b2 100644 > > --- a/hw/usb/dev-smartcard-reader.c > > +++ b/hw/usb/dev-smartcard-reader.c > > @@ -580,6 +580,7 @@ static void ccid_bulk_in_release(USBCCIDState *s) > > assert(s->current_bulk_in != NULL); > > s->current_bulk_in->pos = 0; > > s->current_bulk_in = NULL; > > + s->bulk_in_pending_num--; > > } > > > > static void ccid_bulk_in_get(USBCCIDState *s) > > @@ -587,8 +588,6 @@ static void ccid_bulk_in_get(USBCCIDState *s) > > if (s->current_bulk_in != NULL || s->bulk_in_pending_num == 0) { > > return; > > } > > - assert(s->bulk_in_pending_num > 0); > > - s->bulk_in_pending_num--; > > s->current_bulk_in = > > &s->bulk_in_pending[(s->bulk_in_pending_start++) % > > BULK_IN_PENDING_NUM]; > > } > > @@ -1078,6 +1077,7 @@ static void ccid_bulk_in_copy_to_guest(USBCCIDState > > *s, USBPacket *p, > > > > ccid_bulk_in_get(s); > > if (s->current_bulk_in != NULL) { > > + assert(s->current_bulk_in->pos <= s->current_bulk_in->len); > > len = MIN(s->current_bulk_in->len - s->current_bulk_in->pos, > > p->iov.size); > > if (len) { > > This is the runtime path, but we need also to clear the reset (and card > removal) one: > > static void ccid_bulk_in_clear(USBCCIDState *s) > { > + if (s->current_bulk_in != NULL) { > + s->current_bulk_in->pos = 0; > + s->current_bulk_in = NULL; > + } > s->bulk_in_pending_start = 0; > s->bulk_in_pending_end = 0; > s->bulk_in_pending_num = 0; > } > > ''' > Resetting or removing the card must also discard the active slot so > that its stale position and pointer cannot be reused. > ''' > > With that amended: > Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
corrected, thanks
