Avoid LIBUSB_ERROR_OVERFLOW errors#164
Open
bluetech wants to merge 1 commit into
Open
Conversation
bluetech
commented
Apr 10, 2025
| unsigned int length, unsigned char buffer[]) | ||
| { | ||
| unsigned char cmd[10+length+2]; /* CCID + Protocol Data Structure */ | ||
| unsigned char cmd[ROUND_UP_BUF(10+length+2)]; /* CCID + Protocol Data Structure */ |
Contributor
Author
There was a problem hiding this comment.
I am wary of using VLAs (see e.g. here). It might be better to use malloc. Though I checked and in this case all actual length arguments are small constant size, so it's not exploitable or such.
Owner
|
It would be easier to just modify ReadUSB() to use a temporary big buffer. Then no need to touch anything in commands.c Can you generate a pcscd trace with your patch applied? |
I am trying to get the CCID driver to work with a bit finicky industrial device. Occasionally the communication with the device fails with error LIBUSB_TRANSFER_OVERFLOW, and then things stop working. The reason for this error is described here: https://libusb.sourceforge.io/api-1.0/libusb_packetoverflow.html The errors look like this: pcscd 99999999 src/ccid_usb.c:1082:ReadUSB() read failed (1/14): LIBUSB_ERROR_OVERFLOW pcscd 00000038 src/ifdhandler.c:1244:IFDHPowerICC() PowerDown failed pcscd 63039068 src/ccid_usb.c:1082:ReadUSB() read failed (1/14): LIBUSB_ERROR_TIMEOUT pcscd 00000022 src/ifdhandler.c:1296:IFDHPowerICC() PowerUp failed pcscd 00000005 winscard.c:333:SCardConnect() Error powering up card: rv=SCARD_E_NOT_TRANSACTED pcscd 00000004 prothandler.c:86:PHSetProtocol() Protocol T=1 requested but unsupported by the card Reader: idVendor: 0x0483 iManufacturer: TITENG idProduct: 0x3258 iProduct: TIT-RFREADR-CL As described in the libusb doc, I was able to resolve the problem by tweaking the buffer sizes that CCID is passing to ReadPort (AKA ReadUSB) in the commands.c file. In principle, the needed tweak is to make the buffer sizes a multiple of the endpoint's wMaxPacketSize. This value can be queried from libusb. In all CCID readers I have, the value is always 64 bytes, however in the latest USB version (3.2, very unlikely to actually be used by CCID readers) it can go up to 1024 bytes if I understand correctly. So we can avoid dealing with wMaxPacketSize and just assume 1024. Fixes LudovicRousseau#161
Contributor
Author
|
I rebased to fix some conflicts, no other code changes.
I did try to avoid this extra copying in the transmit path. Adding the few
I'll try to do it. Unfortunately it's a bit of a hassle for me to get the devices set up and reproduce the issue with logging etc. so I need to find some time to do this. For reference the reader in question is the one embedded in card printers/dispensers from this company: https://www.pointman.co.kr/ENG/ |
AGuyMarc
added a commit
to AGuyMarc/CCID
that referenced
this pull request
May 19, 2026
… ReadUSB
Some CCID devices send USB packets that cross the caller-buffer boundary
during libusb_bulk_transfer(), triggering LIBUSB_ERROR_OVERFLOW. After this
error the endpoint stays in a stuck state and pcscd stops being able to talk
to the reader. Root cause is described in the libusb documentation:
https://libusb.sourceforge.io/api-1.0/libusb_packetoverflow.html
Bluetech (PR LudovicRousseau#164) proposed solving this by rounding up every caller buffer
in commands.c to a multiple of 1024 bytes via a ROUND_UP_BUF() macro. That
PR was closed as wontfix, but during the review discussion the maintainer
suggested:
"It would be easier to just modify ReadUSB() to use a temporary big
buffer. Then no need to touch anything in commands.c"
This patch implements that suggestion. The simple libusb_bulk_transfer()
branch of ReadUSB() now allocates a temporary buffer rounded up to a
multiple of 1024 bytes (the maximum wMaxPacketSize for any CCID endpoint
across USB 1.x/2.0/3.x). The transfer is performed into that buffer, and
the data is then memcpy()'d back into the caller's buffer after a sanity
check that the device did not send more data than the caller expected.
Tradeoffs vs PR LudovicRousseau#164:
- Pro: one function modified instead of seven call sites in commands.c.
- Pro: no public macro added; no risk of impedance mismatch between call
sites that adopt the macro and those that don't.
- Pro: caller-buffer overflows that previously only manifested as
LIBUSB_ERROR_OVERFLOW now also trip an explicit "actual_length
greater than caller buffer" guard, which is a strictly safer
behaviour for downstream callers.
- Con: one malloc/free per ReadUSB call. The cost is negligible compared
to the USB round-trip latency (microseconds vs milliseconds), and
is acceptable for the safety guarantee gained.
The multislot_extension branch of ReadUSB and the libusb_bulk_transfer
calls in Multi_PollingProc / Multi_InterruptRead are NOT touched by this
patch — they have a different code path (separate polling thread + queue
of frames) and are out of scope for this fix.
Tested on:
- Ubuntu 26.04, libccid built from current master + this patch
- Neowave WinkeoSIM (USB ID 1e0d:0013), Oberthur v7 PKI applet
(ChamberSign Eiducio certificate). Before the patch, the journalctl
output contained recurring "LIBUSB_ERROR_OVERFLOW" and
"LIBUSB_ERROR_TIMEOUT" lines on every PowerOn attempt; after the
patch those errors are entirely gone.
Note that on the WinkeoSIM device there is a separate, unrelated issue
("Card not transacted: 612" at the IFD layer with bNumDataRatesSupported
reported as 0 in the CCID class descriptor) which is not addressed by
this patch — it remains to be investigated as a distinct issue, possibly
in CmdPowerOn handling or in the middleware. The patch is still a strict
improvement: it removes one real bug from the USB layer, even if a
second hardware-quirk bug remains for that particular device.
Refs: LudovicRousseau#161
Refs: LudovicRousseau#164
Credit-where-due: the analysis of the LIBUSB_ERROR_OVERFLOW root cause
and the initial workaround idea came from Ran Benita (bluetech) in
PR LudovicRousseau#164. This patch only re-implements the maintainer's preferred
location for the fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I am trying to get the CCID driver to work with a bit finicky industrial device. Occasionally the communication with the device fails with error LIBUSB_TRANSFER_OVERFLOW, and then things stop working. The reason for this error is described here:
https://libusb.sourceforge.io/api-1.0/libusb_packetoverflow.html
The errors look like this:
Reader:
idVendor: 0x0483
iManufacturer: TITENG
idProduct: 0x3258
iProduct: TIT-RFREADR-CL
As described in the libusb doc, I was able to resolve the problem by tweaking the buffer sizes that CCID is passing to ReadPort (AKA ReadUSB) in the commands.c file. In principle, the needed tweak is to make the buffer sizes a multiple of the endpoint's wMaxPacketSize. This value can be queried from libusb. In all CCID readers I have, the value is always 64 bytes, however in the latest USB version (3.2, very unlikely to actually be used by CCID readers) it can go up to 1024 bytes if I understand correctly. So we can avoid dealing with wMaxPacketSize and just assume 1024.
Fixes #161