| From ed78352a59ea7acf7520d4d47a96b9911bae7fc3 Mon Sep 17 00:00:00 2001 |
| From: Alexander Popov <alex.popov@linux.com> |
| Date: Mon, 23 Dec 2019 20:51:16 +0300 |
| Subject: [PATCH] ide: Fix incorrect handling of some PRDTs in ide_dma_cb() |
| |
| 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 <alex.popov@linux.com> |
| Reviewed-by: Kevin Wolf <kwolf@redhat.com> |
| Message-id: 20191223175117.508990-2-alex.popov@linux.com |
| Signed-off-by: John Snow <jsnow@redhat.com> |
| |
| Upstream-Status: Backport |
| CVE: CVE-2019-20175 |
| Signed-off-by: Steve Sakoman <steve@sakoman.com> |
| |
| --- |
| 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 754ff4dc343..80000eb7661 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 = false; |
| + int32_t prep_size = 0; |
| |
| if (ret == -EINVAL) { |
| ide_dma_error(s); |
| @@ -863,13 +864,15 @@ static void ide_dma_cb(void *opaque, int ret) |
| } |
| } |
| |
| - n = 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 set |
| - * 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 = s->nsector; |
| stay_active = true; |
| + } else { |
| + n = s->io_buffer_size >> 9; |
| } |
| |
| sector_num = ide_get_sector(s); |
| @@ -892,9 +895,20 @@ static void ide_dma_cb(void *opaque, int ret) |
| n = s->nsector; |
| s->io_buffer_index = 0; |
| s->io_buffer_size = n * 512; |
| - if (s->bus->dma->ops->prepare_buf(s->bus->dma, s->io_buffer_size) < 512) { |
| - /* The PRDs were too short. Reset the Active bit, but don't raise an |
| - * interrupt. */ |
| + prep_size = s->bus->dma->ops->prepare_buf(s->bus->dma, s->io_buffer_size); |
| + /* prepare_buf() must succeed and respect the limit */ |
| + assert(prep_size >= 0 && prep_size <= 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 = READY_STAT | SEEK_STAT; |
| dma_buf_commit(s, 0); |
| goto eot; |