Andrew Geissler | c926e17 | 2021-05-07 16:11:35 -0500 | [diff] [blame^] | 1 | From c9a71afe182be5b62bd2ccdaf861695e0ec0731a Mon Sep 17 00:00:00 2001 |
| 2 | From: Prasad J Pandit <pjp@fedoraproject.org> |
| 3 | Date: Mon, 18 Jan 2021 17:21:30 +0530 |
| 4 | Subject: [PATCH] ide: atapi: check logical block address and read size |
| 5 | (CVE-2020-29443) |
| 6 | |
| 7 | While processing ATAPI cmd_read/cmd_read_cd commands, |
| 8 | Logical Block Address (LBA) maybe invalid OR closer to the last block, |
| 9 | leading to an OOB access issues. Add range check to avoid it. |
| 10 | |
| 11 | Fixes: CVE-2020-29443 |
| 12 | Reported-by: Wenxiang Qian <leonwxqian@gmail.com> |
| 13 | Suggested-by: Paolo Bonzini <pbonzini@redhat.com> |
| 14 | Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> |
| 15 | Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> |
| 16 | Message-Id: <20210118115130.457044-1-ppandit@redhat.com> |
| 17 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
| 18 | |
| 19 | Upstream-Status: Backport [b8d7f1bc59276fec85e4d09f1567613a3e14d31e] |
| 20 | CVE: CVE-2020-29443 |
| 21 | |
| 22 | Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com> |
| 23 | --- |
| 24 | hw/ide/atapi.c | 30 ++++++++++++++++++++++++------ |
| 25 | 1 file changed, 24 insertions(+), 6 deletions(-) |
| 26 | |
| 27 | diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c |
| 28 | index e79157863..b626199e3 100644 |
| 29 | --- a/hw/ide/atapi.c |
| 30 | +++ b/hw/ide/atapi.c |
| 31 | @@ -322,6 +322,8 @@ static void ide_atapi_cmd_reply(IDEState *s, int size, int max_size) |
| 32 | static void ide_atapi_cmd_read_pio(IDEState *s, int lba, int nb_sectors, |
| 33 | int sector_size) |
| 34 | { |
| 35 | + assert(0 <= lba && lba < (s->nb_sectors >> 2)); |
| 36 | + |
| 37 | s->lba = lba; |
| 38 | s->packet_transfer_size = nb_sectors * sector_size; |
| 39 | s->elementary_transfer_size = 0; |
| 40 | @@ -420,6 +422,8 @@ eot: |
| 41 | static void ide_atapi_cmd_read_dma(IDEState *s, int lba, int nb_sectors, |
| 42 | int sector_size) |
| 43 | { |
| 44 | + assert(0 <= lba && lba < (s->nb_sectors >> 2)); |
| 45 | + |
| 46 | s->lba = lba; |
| 47 | s->packet_transfer_size = nb_sectors * sector_size; |
| 48 | s->io_buffer_size = 0; |
| 49 | @@ -973,35 +977,49 @@ static void cmd_prevent_allow_medium_removal(IDEState *s, uint8_t* buf) |
| 50 | |
| 51 | static void cmd_read(IDEState *s, uint8_t* buf) |
| 52 | { |
| 53 | - int nb_sectors, lba; |
| 54 | + unsigned int nb_sectors, lba; |
| 55 | + |
| 56 | + /* Total logical sectors of ATAPI_SECTOR_SIZE(=2048) bytes */ |
| 57 | + uint64_t total_sectors = s->nb_sectors >> 2; |
| 58 | |
| 59 | if (buf[0] == GPCMD_READ_10) { |
| 60 | nb_sectors = lduw_be_p(buf + 7); |
| 61 | } else { |
| 62 | nb_sectors = ldl_be_p(buf + 6); |
| 63 | } |
| 64 | - |
| 65 | - lba = ldl_be_p(buf + 2); |
| 66 | if (nb_sectors == 0) { |
| 67 | ide_atapi_cmd_ok(s); |
| 68 | return; |
| 69 | } |
| 70 | |
| 71 | + lba = ldl_be_p(buf + 2); |
| 72 | + if (lba >= total_sectors || lba + nb_sectors - 1 >= total_sectors) { |
| 73 | + ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_LOGICAL_BLOCK_OOR); |
| 74 | + return; |
| 75 | + } |
| 76 | + |
| 77 | ide_atapi_cmd_read(s, lba, nb_sectors, 2048); |
| 78 | } |
| 79 | |
| 80 | static void cmd_read_cd(IDEState *s, uint8_t* buf) |
| 81 | { |
| 82 | - int nb_sectors, lba, transfer_request; |
| 83 | + unsigned int nb_sectors, lba, transfer_request; |
| 84 | |
| 85 | - nb_sectors = (buf[6] << 16) | (buf[7] << 8) | buf[8]; |
| 86 | - lba = ldl_be_p(buf + 2); |
| 87 | + /* Total logical sectors of ATAPI_SECTOR_SIZE(=2048) bytes */ |
| 88 | + uint64_t total_sectors = s->nb_sectors >> 2; |
| 89 | |
| 90 | + nb_sectors = (buf[6] << 16) | (buf[7] << 8) | buf[8]; |
| 91 | if (nb_sectors == 0) { |
| 92 | ide_atapi_cmd_ok(s); |
| 93 | return; |
| 94 | } |
| 95 | |
| 96 | + lba = ldl_be_p(buf + 2); |
| 97 | + if (lba >= total_sectors || lba + nb_sectors - 1 >= total_sectors) { |
| 98 | + ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_LOGICAL_BLOCK_OOR); |
| 99 | + return; |
| 100 | + } |
| 101 | + |
| 102 | transfer_request = buf[9] & 0xf8; |
| 103 | if (transfer_request == 0x00) { |
| 104 | /* nothing */ |
| 105 | -- |
| 106 | 2.29.2 |
| 107 | |