From b71bd2f7f4a259183fa3d38e6e891cffb86683e2 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 14 Mar 2017 08:52:54 +0100 Subject: [PATCH 21/24] usb-ccid: better bulk_out error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RH-Author: Gerd Hoffmann Message-id: <1489481576-26911-3-git-send-email-kraxel@redhat.com> Patchwork-id: 74289 O-Subject: [RHEL-7.4 qemu-kvm PATCH 2/4] usb-ccid: better bulk_out error handling Bugzilla: 1419818 RH-Acked-by: Stefan Hajnoczi RH-Acked-by: Laurent Vivier RH-Acked-by: Miroslav Rezanina Add err goto label where we can jump to from all error conditions. STALL request on all errors. Reset position on all errors. Normal request processing is not in a else branch any more, so this code is reintended, there are no code changes in that part of the code though. Signed-off-by: Gerd Hoffmann Reviewed-by: Marc-André Lureau Message-id: 1487250819-23764-2-git-send-email-kraxel@redhat.com (cherry picked from commit 0aeebc73b7976bae5cb7e9768e3d9a0fd9d634e8) Signed-off-by: Miroslav Rezanina --- hw/usb/dev-smartcard-reader.c | 116 ++++++++++++++++++++++-------------------- 1 file changed, 61 insertions(+), 55 deletions(-) diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c index 0e0b363..672a7bf 100644 --- a/hw/usb/dev-smartcard-reader.c +++ b/hw/usb/dev-smartcard-reader.c @@ -999,8 +999,7 @@ static void ccid_handle_bulk_out(USBCCIDState *s, USBPacket *p) CCID_Header *ccid_header; if (p->iov.size + s->bulk_out_pos > BULK_OUT_DATA_SIZE) { - p->status = USB_RET_STALL; - return; + goto err; } ccid_header = (CCID_Header *)s->bulk_out_data; usb_packet_copy(p, s->bulk_out_data + s->bulk_out_pos, p->iov.size); @@ -1015,64 +1014,71 @@ static void ccid_handle_bulk_out(USBCCIDState *s, USBPacket *p) DPRINTF(s, 1, "%s: bad USB_TOKEN_OUT length, should be at least 10 bytes\n", __func__); - } else { - DPRINTF(s, D_MORE_INFO, "%s %x %s\n", __func__, - ccid_header->bMessageType, - ccid_message_type_to_str(ccid_header->bMessageType)); - switch (ccid_header->bMessageType) { - case CCID_MESSAGE_TYPE_PC_to_RDR_GetSlotStatus: - ccid_write_slot_status(s, ccid_header); - break; - case CCID_MESSAGE_TYPE_PC_to_RDR_IccPowerOn: - DPRINTF(s, 1, "%s: PowerOn: %d\n", __func__, + goto err; + } + + DPRINTF(s, D_MORE_INFO, "%s %x %s\n", __func__, + ccid_header->bMessageType, + ccid_message_type_to_str(ccid_header->bMessageType)); + switch (ccid_header->bMessageType) { + case CCID_MESSAGE_TYPE_PC_to_RDR_GetSlotStatus: + ccid_write_slot_status(s, ccid_header); + break; + case CCID_MESSAGE_TYPE_PC_to_RDR_IccPowerOn: + DPRINTF(s, 1, "%s: PowerOn: %d\n", __func__, ((CCID_IccPowerOn *)(ccid_header))->bPowerSelect); - s->powered = true; - if (!ccid_card_inserted(s)) { - ccid_report_error_failed(s, ERROR_ICC_MUTE); - } - /* atr is written regardless of error. */ - ccid_write_data_block_atr(s, ccid_header); - break; - case CCID_MESSAGE_TYPE_PC_to_RDR_IccPowerOff: - ccid_reset_error_status(s); - s->powered = false; - ccid_write_slot_status(s, ccid_header); - break; - case CCID_MESSAGE_TYPE_PC_to_RDR_XfrBlock: - ccid_on_apdu_from_guest(s, (CCID_XferBlock *)s->bulk_out_data); - break; - case CCID_MESSAGE_TYPE_PC_to_RDR_SetParameters: - ccid_reset_error_status(s); - ccid_set_parameters(s, ccid_header); - ccid_write_parameters(s, ccid_header); - break; - case CCID_MESSAGE_TYPE_PC_to_RDR_ResetParameters: - ccid_reset_error_status(s); - ccid_reset_parameters(s); - ccid_write_parameters(s, ccid_header); - break; - case CCID_MESSAGE_TYPE_PC_to_RDR_GetParameters: - ccid_reset_error_status(s); - ccid_write_parameters(s, ccid_header); - break; - case CCID_MESSAGE_TYPE_PC_to_RDR_Mechanical: - ccid_report_error_failed(s, 0); - ccid_write_slot_status(s, ccid_header); - break; - default: - DPRINTF(s, 1, + s->powered = true; + if (!ccid_card_inserted(s)) { + ccid_report_error_failed(s, ERROR_ICC_MUTE); + } + /* atr is written regardless of error. */ + ccid_write_data_block_atr(s, ccid_header); + break; + case CCID_MESSAGE_TYPE_PC_to_RDR_IccPowerOff: + ccid_reset_error_status(s); + s->powered = false; + ccid_write_slot_status(s, ccid_header); + break; + case CCID_MESSAGE_TYPE_PC_to_RDR_XfrBlock: + ccid_on_apdu_from_guest(s, (CCID_XferBlock *)s->bulk_out_data); + break; + case CCID_MESSAGE_TYPE_PC_to_RDR_SetParameters: + ccid_reset_error_status(s); + ccid_set_parameters(s, ccid_header); + ccid_write_parameters(s, ccid_header); + break; + case CCID_MESSAGE_TYPE_PC_to_RDR_ResetParameters: + ccid_reset_error_status(s); + ccid_reset_parameters(s); + ccid_write_parameters(s, ccid_header); + break; + case CCID_MESSAGE_TYPE_PC_to_RDR_GetParameters: + ccid_reset_error_status(s); + ccid_write_parameters(s, ccid_header); + break; + case CCID_MESSAGE_TYPE_PC_to_RDR_Mechanical: + ccid_report_error_failed(s, 0); + ccid_write_slot_status(s, ccid_header); + break; + default: + DPRINTF(s, 1, "handle_data: ERROR: unhandled message type %Xh\n", ccid_header->bMessageType); - /* - * The caller is expecting the device to respond, tell it we - * don't support the operation. - */ - ccid_report_error_failed(s, ERROR_CMD_NOT_SUPPORTED); - ccid_write_slot_status(s, ccid_header); - break; - } + /* + * The caller is expecting the device to respond, tell it we + * don't support the operation. + */ + ccid_report_error_failed(s, ERROR_CMD_NOT_SUPPORTED); + ccid_write_slot_status(s, ccid_header); + break; } s->bulk_out_pos = 0; + return; + +err: + p->status = USB_RET_STALL; + s->bulk_out_pos = 0; + return; } static void ccid_bulk_in_copy_to_guest(USBCCIDState *s, USBPacket *p) -- 1.8.3.1