From 48d89973c64b7cde5c35f1fe2428766d1080ae41 Mon Sep 17 00:00:00 2001 From: NIIBE Yutaka Date: Wed, 28 Dec 2011 12:27:16 +0900 Subject: [PATCH] fixed long standing bug of ZLP --- ChangeLog | 7 +++++++ NEWS | 8 ++++++++ src/usb-icc.c | 50 ++++++++++++++++++++++++++++++-------------------- src/usb_prop.c | 2 ++ 4 files changed, 47 insertions(+), 20 deletions(-) diff --git a/ChangeLog b/ChangeLog index f5cf0b6..2a5d7f3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2011-12-28 Niibe Yutaka + + * src/usb_prop.c (msc_lun_info) [PINPAD_DND_SUPPORT]: ifdef-out. + + * src/usb-icc.c (EP2_OUT_Callback): Fix apdu size == 49 bug, + we don't assume host sends ZLP (But accepts ZLP, just in case). + 2011-12-22 Niibe Yutaka * src/openpgp-do.c (extended_capabilities) [CERTDO_SUPPORT]: diff --git a/NEWS b/NEWS index 8928329..dd38543 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,14 @@ Gnuk NEWS - User visible changes Released 2012-01-XX, by NIIBE Yutaka +** USB CCID/ICCD low level bug is fixed +When the size of command APDU data is just 49, the lower level packet +size is 64. This is maximum size of BULK-OUT transfer packet, and +caused trouble in the past implementations. Example is setting url +(0x5f50) as: http://www.gniibe.org/adpu-string-size-is-just-49 +This is because the past implementations expect ZLP (zero size +packet). Now, it has been fixed. You can use any size. + ** CERT.3 Data Object (0x7f21) is now optional As there's no valid use case for this data object and it does not work as current version of GnuPG, this is now optional feature. diff --git a/src/usb-icc.c b/src/usb-icc.c index 0258930..a027bab 100644 --- a/src/usb-icc.c +++ b/src/usb-icc.c @@ -169,12 +169,31 @@ void EP2_OUT_Callback (void) { int len; + struct icc_header *icc_header; + int data_len_so_far; + int data_len; len = USB_SIL_Read (EP2_OUT, icc_next_p); + if (len == 0) + { /* Just ignore Zero Length Packet (ZLP), if any */ + SetEPRxValid (ENDP2); + return; + } - if (len == USB_LL_BUF_SIZE) /* The sequence of transactions continues */ + icc_next_p += len; + + if (icc_chain_p) + icc_header = (struct icc_header *)icc_chain_p; + else + icc_header = (struct icc_header *)icc_buffer; + + data_len = icc_header->data_len; /* NOTE: We're little endian */ + data_len_so_far = (icc_next_p - (uint8_t *)icc_header) - ICC_MSG_HEADER_SIZE; + + if (len == USB_LL_BUF_SIZE + && data_len != data_len_so_far) + /* The sequence of transactions continues */ { - icc_next_p += USB_LL_BUF_SIZE; SetEPRxValid (ENDP2); if ((icc_next_p - icc_buffer) >= USB_BUF_SIZE) /* No room to receive any more */ @@ -186,27 +205,18 @@ EP2_OUT_Callback (void) * (and discard the whole block) */ } + + /* + * NOTE: It is possible a transaction may stall when the size of + * BULK_OUT transaction it's bigger than USB_BUF_SIZE and stops + * with just USB_LL_BUF_SIZE packet. Device will remain waiting + * another packet. + */ } else /* Finished */ { - struct icc_header *icc_header; - int data_len; - - icc_next_p += len; - if (icc_chain_p) - { - icc_header = (struct icc_header *)icc_chain_p; - icc_data_size = (icc_next_p - icc_chain_p) - ICC_MSG_HEADER_SIZE; - } - else - { - icc_header = (struct icc_header *)icc_buffer; - icc_data_size = (icc_next_p - icc_buffer) - ICC_MSG_HEADER_SIZE; - } - - /* NOTE: We're little endian, nothing to convert */ - data_len = icc_header->data_len; - icc_seq = icc_header->seq; + icc_data_size = data_len_so_far; + icc_seq = icc_header->seq; /* NOTE: We're little endian */ if (icc_data_size != data_len) { diff --git a/src/usb_prop.c b/src/usb_prop.c index c063a28..454ee0b 100644 --- a/src/usb_prop.c +++ b/src/usb_prop.c @@ -288,6 +288,7 @@ gnuk_data_rates (uint16_t len) return (uint8_t *)data_rate_table; } +#if defined(PINPAD_DND_SUPPORT) static const uint8_t lun_table[] = { 0, 0, 0, 0, }; static uint8_t * msc_lun_info (uint16_t len) @@ -300,6 +301,7 @@ msc_lun_info (uint16_t len) return (uint8_t *)lun_table; } +#endif static RESULT gnuk_setup_with_data (uint8_t RequestNo)