From nobody Sun Feb 8 19:48:36 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1580173712418872.9059884590886; Mon, 27 Jan 2020 17:08:32 -0800 (PST) Received: from localhost ([::1]:52875 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iwFMl-0003bh-BK for importer@patchew.org; Mon, 27 Jan 2020 20:08:31 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:51724) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iwFLc-0002CY-0w for qemu-devel@nongnu.org; Mon, 27 Jan 2020 20:07:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iwFLa-0003TV-NM for qemu-devel@nongnu.org; Mon, 27 Jan 2020 20:07:19 -0500 Received: from us-smtp-delivery-1.mimecast.com ([207.211.31.120]:20228 helo=us-smtp-1.mimecast.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iwFLa-0003TA-Jw for qemu-devel@nongnu.org; Mon, 27 Jan 2020 20:07:18 -0500 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-339-bSvrcc-lOoqm9vXI7wTj7g-1; Mon, 27 Jan 2020 20:07:13 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 66D1B1800D41; Tue, 28 Jan 2020 01:07:12 +0000 (UTC) Received: from probe.bos.redhat.com (dhcp-17-116.bos.redhat.com [10.18.17.116]) by smtp.corp.redhat.com (Postfix) with ESMTP id 92D035D9CA; Tue, 28 Jan 2020 01:07:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1580173638; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=O9xC74yWJ9mbCiU1xQq2wAud7U8Mw1JfFxaKeDu30EI=; b=BImSBpsVJMXI8MfJEko28nJYMZNfyVvs6/Ib1GOQatOkqm+Yl91j2aw6lztqGwlXQBERKT ii/+1OTI4MUEoXBCbwy454EUrPWIUgN+3SIMG+wCVolBqQxkL3+MaETVKMn4/qoV3em4CV YIUT6gyka+BAmmMUMJeSFfnXWGj2b0w= From: John Snow To: Peter Maydell , qemu-devel@nongnu.org Subject: [PULL 1/2] ide: Fix incorrect handling of some PRDTs in ide_dma_cb() Date: Mon, 27 Jan 2020 20:07:06 -0500 Message-Id: <20200128010707.19572-2-jsnow@redhat.com> In-Reply-To: <20200128010707.19572-1-jsnow@redhat.com> References: <20200128010707.19572-1-jsnow@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-MC-Unique: bSvrcc-lOoqm9vXI7wTj7g-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 207.211.31.120 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Alexander Popov , John Snow , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Type: text/plain; charset="utf-8" From: Alexander Popov The commit a718978ed58a from July 2015 introduced the assertion which implies that the size of successful DMA transfers handled in ide_dma_cb() should be multiple of 512 (the size of a sector). But guest systems can initiate DMA transfers that don't fit this requirement. For fixing that let's check the number of bytes prepared for the transfer by the prepare_buf() handler. The code in ide_dma_cb() must behave according to the Programming Interface for Bus Master IDE Controller (Revision 1.0 5/16/94): 1. If PRDs specified a smaller size than the IDE transfer size, then the Interrupt and Active bits in the Controller status register are not set (Error Condition). 2. If the size of the physical memory regions was equal to the IDE device transfer size, the Interrupt bit in the Controller status register is set to 1, Active bit is set to 0. 3. If PRDs specified a larger size than the IDE transfer size, the Interrupt and Active bits in the Controller status register are both set to 1. Signed-off-by: Alexander Popov Reviewed-by: Kevin Wolf Message-id: 20191223175117.508990-2-alex.popov@linux.com Signed-off-by: John Snow --- hw/ide/core.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 754ff4dc34..80000eb766 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -849,6 +849,7 @@ static void ide_dma_cb(void *opaque, int ret) int64_t sector_num; uint64_t offset; bool stay_active =3D false; + int32_t prep_size =3D 0; =20 if (ret =3D=3D -EINVAL) { ide_dma_error(s); @@ -863,13 +864,15 @@ static void ide_dma_cb(void *opaque, int ret) } } =20 - n =3D s->io_buffer_size >> 9; - if (n > s->nsector) { - /* The PRDs were longer than needed for this request. Shorten them= so - * we don't get a negative remainder. The Active bit must remain s= et - * after the request completes. */ + if (s->io_buffer_size > s->nsector * 512) { + /* + * The PRDs were longer than needed for this request. + * The Active bit must remain set after the request completes. + */ n =3D s->nsector; stay_active =3D true; + } else { + n =3D s->io_buffer_size >> 9; } =20 sector_num =3D ide_get_sector(s); @@ -892,9 +895,20 @@ static void ide_dma_cb(void *opaque, int ret) n =3D s->nsector; s->io_buffer_index =3D 0; s->io_buffer_size =3D n * 512; - if (s->bus->dma->ops->prepare_buf(s->bus->dma, s->io_buffer_size) < 51= 2) { - /* The PRDs were too short. Reset the Active bit, but don't raise = an - * interrupt. */ + prep_size =3D s->bus->dma->ops->prepare_buf(s->bus->dma, s->io_buffer_= size); + /* prepare_buf() must succeed and respect the limit */ + assert(prep_size >=3D 0 && prep_size <=3D n * 512); + + /* + * Now prep_size stores the number of bytes in the sglist, and + * s->io_buffer_size stores the number of bytes described by the PRDs. + */ + + if (prep_size < n * 512) { + /* + * The PRDs are too short for this request. Error condition! + * Reset the Active bit and don't raise the interrupt. + */ s->status =3D READY_STAT | SEEK_STAT; dma_buf_commit(s, 0); goto eot; --=20 2.21.0 From nobody Sun Feb 8 19:48:36 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1580173707463452.8836297138778; Mon, 27 Jan 2020 17:08:27 -0800 (PST) Received: from localhost ([::1]:52872 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iwFMg-0003T3-An for importer@patchew.org; Mon, 27 Jan 2020 20:08:26 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:51744) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iwFLc-0002Cr-RX for qemu-devel@nongnu.org; Mon, 27 Jan 2020 20:07:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iwFLb-0003UE-Ds for qemu-devel@nongnu.org; Mon, 27 Jan 2020 20:07:20 -0500 Received: from us-smtp-delivery-1.mimecast.com ([207.211.31.120]:44375 helo=us-smtp-1.mimecast.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iwFLb-0003Tt-9q for qemu-devel@nongnu.org; Mon, 27 Jan 2020 20:07:19 -0500 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-361-na_2oIgKO9iEKx9qbxPvFg-1; Mon, 27 Jan 2020 20:07:14 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 51331800D48; Tue, 28 Jan 2020 01:07:13 +0000 (UTC) Received: from probe.bos.redhat.com (dhcp-17-116.bos.redhat.com [10.18.17.116]) by smtp.corp.redhat.com (Postfix) with ESMTP id 88A475D9CA; Tue, 28 Jan 2020 01:07:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1580173639; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZyRxWrOGbv9to/iNkAWzvepPIhJ+AKRYBpvFEqPlzw8=; b=R0//kZWEIEmniTpJLtHS8UL6YJi/FPYUCY40WXVPQEreH+/g7EWc6WBeemfDReqy95MSTJ KthK9ELsuDN+bZgo5HY5cXjhpi2FanCjoJJxf0PhqlQg32KJutzUSv68v/KbeI7kaOqdx6 wY/MNaX6fIOUwkgx/oJD0YQpheMF65g= From: John Snow To: Peter Maydell , qemu-devel@nongnu.org Subject: [PULL 2/2] tests/ide-test: Create a single unit-test covering more PRDT cases Date: Mon, 27 Jan 2020 20:07:07 -0500 Message-Id: <20200128010707.19572-3-jsnow@redhat.com> In-Reply-To: <20200128010707.19572-1-jsnow@redhat.com> References: <20200128010707.19572-1-jsnow@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-MC-Unique: na_2oIgKO9iEKx9qbxPvFg-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 207.211.31.120 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Alexander Popov , John Snow , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Type: text/plain; charset="utf-8" From: Alexander Popov Fuzzing the Linux kernel with syzkaller allowed to find how to crash qemu using a special SCSI_IOCTL_SEND_COMMAND. It hits the assertion in ide_dma_cb() introduced in the commit a718978ed58a in July 2015. Currently this bug is not reproduced by the unit tests. Let's improve the ide-test to cover more PRDT cases including one that causes this particular qemu crash. The test is developed according to the Programming Interface for Bus Master IDE Controller (Revision 1.0 5/16/94). Signed-off-by: Alexander Popov Message-id: 20191223175117.508990-3-alex.popov@linux.com Signed-off-by: John Snow --- tests/qtest/ide-test.c | 176 ++++++++++++++++++----------------------- 1 file changed, 75 insertions(+), 101 deletions(-) diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c index 0277e7d5a9..5cfd97f915 100644 --- a/tests/qtest/ide-test.c +++ b/tests/qtest/ide-test.c @@ -445,104 +445,81 @@ static void test_bmdma_trim(void) test_bmdma_teardown(qts); } =20 -static void test_bmdma_short_prdt(void) +/* + * This test is developed according to the Programming Interface for + * Bus Master IDE Controller (Revision 1.0 5/16/94) + */ +static void test_bmdma_various_prdts(void) { - QTestState *qts; - QPCIDevice *dev; - QPCIBar bmdma_bar, ide_bar; - uint8_t status; - - PrdtEntry prdt[] =3D { - { - .addr =3D 0, - .size =3D cpu_to_le32(0x10 | PRDT_EOT), - }, - }; - - qts =3D test_bmdma_setup(); - - dev =3D get_pci_device(qts, &bmdma_bar, &ide_bar); - - /* Normal request */ - status =3D send_dma_request(qts, CMD_READ_DMA, 0, 1, - prdt, ARRAY_SIZE(prdt), NULL); - g_assert_cmphex(status, =3D=3D, 0); - assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR); - - /* Abort the request before it completes */ - status =3D send_dma_request(qts, CMD_READ_DMA | CMDF_ABORT, 0, 1, - prdt, ARRAY_SIZE(prdt), NULL); - g_assert_cmphex(status, =3D=3D, 0); - assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR); - free_pci_device(dev); - test_bmdma_teardown(qts); -} - -static void test_bmdma_one_sector_short_prdt(void) -{ - QTestState *qts; - QPCIDevice *dev; - QPCIBar bmdma_bar, ide_bar; - uint8_t status; - - /* Read 2 sectors but only give 1 sector in PRDT */ - PrdtEntry prdt[] =3D { - { - .addr =3D 0, - .size =3D cpu_to_le32(0x200 | PRDT_EOT), - }, - }; - - qts =3D test_bmdma_setup(); - - dev =3D get_pci_device(qts, &bmdma_bar, &ide_bar); - - /* Normal request */ - status =3D send_dma_request(qts, CMD_READ_DMA, 0, 2, - prdt, ARRAY_SIZE(prdt), NULL); - g_assert_cmphex(status, =3D=3D, 0); - assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR); - - /* Abort the request before it completes */ - status =3D send_dma_request(qts, CMD_READ_DMA | CMDF_ABORT, 0, 2, - prdt, ARRAY_SIZE(prdt), NULL); - g_assert_cmphex(status, =3D=3D, 0); - assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR); - free_pci_device(dev); - test_bmdma_teardown(qts); -} - -static void test_bmdma_long_prdt(void) -{ - QTestState *qts; - QPCIDevice *dev; - QPCIBar bmdma_bar, ide_bar; - uint8_t status; - - PrdtEntry prdt[] =3D { - { - .addr =3D 0, - .size =3D cpu_to_le32(0x1000 | PRDT_EOT), - }, - }; - - qts =3D test_bmdma_setup(); - - dev =3D get_pci_device(qts, &bmdma_bar, &ide_bar); - - /* Normal request */ - status =3D send_dma_request(qts, CMD_READ_DMA, 0, 1, - prdt, ARRAY_SIZE(prdt), NULL); - g_assert_cmphex(status, =3D=3D, BM_STS_ACTIVE | BM_STS_INTR); - assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR); - - /* Abort the request before it completes */ - status =3D send_dma_request(qts, CMD_READ_DMA | CMDF_ABORT, 0, 1, - prdt, ARRAY_SIZE(prdt), NULL); - g_assert_cmphex(status, =3D=3D, BM_STS_INTR); - assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR); - free_pci_device(dev); - test_bmdma_teardown(qts); + int sectors =3D 0; + uint32_t size =3D 0; + + for (sectors =3D 1; sectors <=3D 256; sectors *=3D 2) { + QTestState *qts =3D NULL; + QPCIDevice *dev =3D NULL; + QPCIBar bmdma_bar, ide_bar; + + qts =3D test_bmdma_setup(); + dev =3D get_pci_device(qts, &bmdma_bar, &ide_bar); + + for (size =3D 0; size < 65536; size +=3D 256) { + uint32_t req_size =3D sectors * 512; + uint32_t prd_size =3D size & 0xfffe; /* bit 0 is always set to= 0 */ + uint8_t ret =3D 0; + uint8_t req_status =3D 0; + uint8_t abort_req_status =3D 0; + PrdtEntry prdt[] =3D { + { + .addr =3D 0, + .size =3D cpu_to_le32(size | PRDT_EOT), + }, + }; + + /* A value of zero in PRD size indicates 64K */ + if (prd_size =3D=3D 0) { + prd_size =3D 65536; + } + + /* + * 1. If PRDs specified a smaller size than the IDE transfer + * size, then the Interrupt and Active bits in the Controller + * status register are not set (Error Condition). + * + * 2. If the size of the physical memory regions was equal to + * the IDE device transfer size, the Interrupt bit in the + * Controller status register is set to 1, Active bit is set t= o 0. + * + * 3. If PRDs specified a larger size than the IDE transfer si= ze, + * the Interrupt and Active bits in the Controller status regi= ster + * are both set to 1. + */ + if (prd_size < req_size) { + req_status =3D 0; + abort_req_status =3D 0; + } else if (prd_size =3D=3D req_size) { + req_status =3D BM_STS_INTR; + abort_req_status =3D BM_STS_INTR; + } else { + req_status =3D BM_STS_ACTIVE | BM_STS_INTR; + abort_req_status =3D BM_STS_INTR; + } + + /* Test the request */ + ret =3D send_dma_request(qts, CMD_READ_DMA, 0, sectors, + prdt, ARRAY_SIZE(prdt), NULL); + g_assert_cmphex(ret, =3D=3D, req_status); + assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF |= ERR); + + /* Now test aborting the same request */ + ret =3D send_dma_request(qts, CMD_READ_DMA | CMDF_ABORT, 0, + sectors, prdt, ARRAY_SIZE(prdt), NULL); + g_assert_cmphex(ret, =3D=3D, abort_req_status); + assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF |= ERR); + } + + free_pci_device(dev); + test_bmdma_teardown(qts); + } } =20 static void test_bmdma_no_busmaster(void) @@ -1066,10 +1043,7 @@ int main(int argc, char **argv) =20 qtest_add_func("/ide/bmdma/simple_rw", test_bmdma_simple_rw); qtest_add_func("/ide/bmdma/trim", test_bmdma_trim); - qtest_add_func("/ide/bmdma/short_prdt", test_bmdma_short_prdt); - qtest_add_func("/ide/bmdma/one_sector_short_prdt", - test_bmdma_one_sector_short_prdt); - qtest_add_func("/ide/bmdma/long_prdt", test_bmdma_long_prdt); + qtest_add_func("/ide/bmdma/various_prdts", test_bmdma_various_prdts); qtest_add_func("/ide/bmdma/no_busmaster", test_bmdma_no_busmaster); =20 qtest_add_func("/ide/flush", test_flush); --=20 2.21.0