From nobody Fri May 3 23:43:00 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1487251077728663.0168970855999; Thu, 16 Feb 2017 05:17:57 -0800 (PST) Received: from localhost ([::1]:46584 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ceLwZ-0004rd-LX for importer@patchew.org; Thu, 16 Feb 2017 08:17:55 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57673) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ceLsZ-0001kz-8x for qemu-devel@nongnu.org; Thu, 16 Feb 2017 08:13:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ceLsW-0000Xq-FT for qemu-devel@nongnu.org; Thu, 16 Feb 2017 08:13:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34818) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ceLsW-0000Wv-7B for qemu-devel@nongnu.org; Thu, 16 Feb 2017 08:13:44 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 742868553C for ; Thu, 16 Feb 2017 13:13:44 +0000 (UTC) Received: from nilsson.home.kraxel.org (ovpn-116-43.ams2.redhat.com [10.36.116.43]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1GDDhqF009857; Thu, 16 Feb 2017 08:13:43 -0500 Received: by nilsson.home.kraxel.org (Postfix, from userid 500) id 53A39806AC; Thu, 16 Feb 2017 14:13:41 +0100 (CET) From: Gerd Hoffmann To: qemu-devel@nongnu.org Date: Thu, 16 Feb 2017 14:13:37 +0100 Message-Id: <1487250819-23764-2-git-send-email-kraxel@redhat.com> In-Reply-To: <1487250819-23764-1-git-send-email-kraxel@redhat.com> References: <1487250819-23764-1-git-send-email-kraxel@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 16 Feb 2017 13:13:44 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 1/3] usb-ccid: better bulk_out error handling X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Gerd Hoffmann Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" 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=C3=A9 Lureau --- 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 1325ea1..badcfcb 100644 --- a/hw/usb/dev-smartcard-reader.c +++ b/hw/usb/dev-smartcard-reader.c @@ -1001,8 +1001,7 @@ static void ccid_handle_bulk_out(USBCCIDState *s, USB= Packet *p) CCID_Header *ccid_header; =20 if (p->iov.size + s->bulk_out_pos > BULK_OUT_DATA_SIZE) { - p->status =3D USB_RET_STALL; - return; + goto err; } ccid_header =3D (CCID_Header *)s->bulk_out_data; usb_packet_copy(p, s->bulk_out_data + s->bulk_out_pos, p->iov.size); @@ -1017,64 +1016,71 @@ static void ccid_handle_bulk_out(USBCCIDState *s, U= SBPacket *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 =3D 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 =3D 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 =3D 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 =3D 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 =3D 0; + return; + +err: + p->status =3D USB_RET_STALL; + s->bulk_out_pos =3D 0; + return; } =20 static void ccid_bulk_in_copy_to_guest(USBCCIDState *s, USBPacket *p) --=20 1.8.3.1 From nobody Fri May 3 23:43:00 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1487251079679223.45270943811295; Thu, 16 Feb 2017 05:17:59 -0800 (PST) Received: from localhost ([::1]:46583 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ceLwZ-0004ra-DB for importer@patchew.org; Thu, 16 Feb 2017 08:17:55 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57672) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ceLsZ-0001ky-8k for qemu-devel@nongnu.org; Thu, 16 Feb 2017 08:13:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ceLsW-0000Xe-86 for qemu-devel@nongnu.org; Thu, 16 Feb 2017 08:13:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37456) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ceLsW-0000Wi-2P for qemu-devel@nongnu.org; Thu, 16 Feb 2017 08:13:44 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4BD223E9 for ; Thu, 16 Feb 2017 13:13:44 +0000 (UTC) Received: from nilsson.home.kraxel.org (ovpn-116-43.ams2.redhat.com [10.36.116.43]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1GDDh01003974; Thu, 16 Feb 2017 08:13:43 -0500 Received: by nilsson.home.kraxel.org (Postfix, from userid 500) id 72AA180BAE; Thu, 16 Feb 2017 14:13:41 +0100 (CET) From: Gerd Hoffmann To: qemu-devel@nongnu.org Date: Thu, 16 Feb 2017 14:13:38 +0100 Message-Id: <1487250819-23764-3-git-send-email-kraxel@redhat.com> In-Reply-To: <1487250819-23764-1-git-send-email-kraxel@redhat.com> References: <1487250819-23764-1-git-send-email-kraxel@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Thu, 16 Feb 2017 13:13:44 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 2/3] usb-ccid: move header size check X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Gerd Hoffmann Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Move up header size check, so we can use header fields in sanity checks (in followup patches). Also reword the debug message. Signed-off-by: Gerd Hoffmann Reviewed-by: Marc-Andr=C3=A9 Lureau --- hw/usb/dev-smartcard-reader.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c index badcfcb..1acc1fb 100644 --- a/hw/usb/dev-smartcard-reader.c +++ b/hw/usb/dev-smartcard-reader.c @@ -1003,21 +1003,20 @@ static void ccid_handle_bulk_out(USBCCIDState *s, U= SBPacket *p) if (p->iov.size + s->bulk_out_pos > BULK_OUT_DATA_SIZE) { goto err; } - ccid_header =3D (CCID_Header *)s->bulk_out_data; usb_packet_copy(p, s->bulk_out_data + s->bulk_out_pos, p->iov.size); s->bulk_out_pos +=3D p->iov.size; + if (s->bulk_out_pos < 10) { + DPRINTF(s, 1, "%s: header incomplete\n", __func__); + goto err; + } + + ccid_header =3D (CCID_Header *)s->bulk_out_data; if (p->iov.size =3D=3D CCID_MAX_PACKET_SIZE) { DPRINTF(s, D_VERBOSE, "usb-ccid: bulk_in: expecting more packets (%zd/%d)\n", p->iov.size, ccid_header->dwLength); return; } - if (s->bulk_out_pos < 10) { - DPRINTF(s, 1, - "%s: bad USB_TOKEN_OUT length, should be at least 10 bytes= \n", - __func__); - goto err; - } =20 DPRINTF(s, D_MORE_INFO, "%s %x %s\n", __func__, ccid_header->bMessageType, --=20 1.8.3.1 From nobody Fri May 3 23:43:00 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1487250905354478.412045554472; Thu, 16 Feb 2017 05:15:05 -0800 (PST) Received: from localhost ([::1]:46570 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ceLtm-0002ML-VI for importer@patchew.org; Thu, 16 Feb 2017 08:15:02 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57674) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ceLsZ-0001l0-8m for qemu-devel@nongnu.org; Thu, 16 Feb 2017 08:13:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ceLsW-0000Xj-8q for qemu-devel@nongnu.org; Thu, 16 Feb 2017 08:13:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34176) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ceLsW-0000Wk-2X for qemu-devel@nongnu.org; Thu, 16 Feb 2017 08:13:44 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 53D38C04B321 for ; Thu, 16 Feb 2017 13:13:44 +0000 (UTC) Received: from nilsson.home.kraxel.org (ovpn-116-43.ams2.redhat.com [10.36.116.43]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1GDDh26003337; Thu, 16 Feb 2017 08:13:43 -0500 Received: by nilsson.home.kraxel.org (Postfix, from userid 500) id 8C71D80C50; Thu, 16 Feb 2017 14:13:41 +0100 (CET) From: Gerd Hoffmann To: qemu-devel@nongnu.org Date: Thu, 16 Feb 2017 14:13:39 +0100 Message-Id: <1487250819-23764-4-git-send-email-kraxel@redhat.com> In-Reply-To: <1487250819-23764-1-git-send-email-kraxel@redhat.com> References: <1487250819-23764-1-git-send-email-kraxel@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Thu, 16 Feb 2017 13:13:44 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 3/3] usb-ccid: add check message size checks X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Gerd Hoffmann Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Check message size too when figuring whenever we should expect more data. Fix debug message to show useful data, p->iov.size is fixed anyway if we land there, print how much we got meanwhile instead. Also check announced message size against actual message size. That is a more general fix for CVE-2017-5898 than commit "c7dfbf3 usb: ccid: check ccid apdu length". Signed-off-by: Gerd Hoffmann Reviewed-by: Marc-Andr=C3=A9 Lureau --- hw/usb/dev-smartcard-reader.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c index 1acc1fb..7cd4ed0 100644 --- a/hw/usb/dev-smartcard-reader.c +++ b/hw/usb/dev-smartcard-reader.c @@ -1011,12 +1011,19 @@ static void ccid_handle_bulk_out(USBCCIDState *s, U= SBPacket *p) } =20 ccid_header =3D (CCID_Header *)s->bulk_out_data; - if (p->iov.size =3D=3D CCID_MAX_PACKET_SIZE) { + if ((s->bulk_out_pos - 10 < ccid_header->dwLength) && + (p->iov.size =3D=3D CCID_MAX_PACKET_SIZE)) { DPRINTF(s, D_VERBOSE, - "usb-ccid: bulk_in: expecting more packets (%zd/%d)\n", - p->iov.size, ccid_header->dwLength); + "usb-ccid: bulk_in: expecting more packets (%d/%d)\n", + s->bulk_out_pos - 10, ccid_header->dwLength); return; } + if (s->bulk_out_pos - 10 !=3D ccid_header->dwLength) { + DPRINTF(s, 1, + "usb-ccid: bulk_in: message size mismatch (got %d, expecte= d %d)\n", + s->bulk_out_pos - 10, ccid_header->dwLength); + goto err; + } =20 DPRINTF(s, D_MORE_INFO, "%s %x %s\n", __func__, ccid_header->bMessageType, --=20 1.8.3.1