From dc4ff14029538f4f2787271b98d6e8e403cbfcc5 Mon Sep 17 00:00:00 2001 From: Jon Maloy Date: Fri, 26 Feb 2021 01:06:42 -0500 Subject: [PATCH] ide: atapi: check logical block address and read size (CVE-2020-29443) RH-Author: Jon Maloy Message-id: <20210226010642.3200257-2-jmaloy@redhat.com> Patchwork-id: 101270 O-Subject: [RHEL-7.9.z qemu-kvm PATCH v2 1/1] ide: atapi: check logical block address and read size (CVE-2020-29443) Bugzilla: 1917449 RH-Acked-by: Kevin Wolf RH-Acked-by: Danilo de Paula RH-Acked-by: Paolo Bonzini From: Prasad J Pandit While processing ATAPI cmd_read/cmd_read_cd commands, Logical Block Address (LBA) maybe invalid OR closer to the last block, leading to an OOB access issues. Add range check to avoid it. Fixes: CVE-2020-29443 Reported-by: Wenxiang Qian Suggested-by: Paolo Bonzini Reviewed-by: Paolo Bonzini Signed-off-by: Prasad J Pandit Message-Id: <20210118115130.457044-1-ppandit@redhat.com> Signed-off-by: Paolo Bonzini (cherry picked from commit b8d7f1bc59276fec85e4d09f1567613a3e14d31e) Conflict: There is a conflict in cmd_read_cd(), because commit e7bd708ec85e ("atapi: classify read_cd as conditionally returning data") is missing in this code version. That seems to be an unrelated fix that drags in further changes, so instead of applying it we choose to adapt the commit directly to the current code version. Signed-off-by: Jon Maloy Signed-off-by: Jon Maloy --- hw/ide/atapi.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index 490070a1b4..bef74914d1 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -267,6 +267,8 @@ static void ide_atapi_cmd_reply(IDEState *s, int size, int max_size) static void ide_atapi_cmd_read_pio(IDEState *s, int lba, int nb_sectors, int sector_size) { + assert(0 <= lba && lba < (s->nb_sectors >> 2)); + s->lba = lba; s->packet_transfer_size = nb_sectors * sector_size; s->elementary_transfer_size = 0; @@ -365,6 +367,8 @@ eot: static void ide_atapi_cmd_read_dma(IDEState *s, int lba, int nb_sectors, int sector_size) { + assert(0 <= lba && lba < (s->nb_sectors >> 2)); + s->lba = lba; s->packet_transfer_size = nb_sectors * sector_size; s->io_buffer_index = 0; @@ -823,7 +827,10 @@ static void cmd_prevent_allow_medium_removal(IDEState *s, uint8_t* buf) static void cmd_read(IDEState *s, uint8_t* buf) { - int nb_sectors, lba; + unsigned int nb_sectors, lba; + + /* Total logical sectors of ATAPI_SECTOR_SIZE(=2048) bytes */ + uint64_t total_sectors = s->nb_sectors >> 2; if (buf[0] == GPCMD_READ_10) { nb_sectors = ube16_to_cpu(buf + 7); @@ -831,27 +838,39 @@ static void cmd_read(IDEState *s, uint8_t* buf) nb_sectors = ube32_to_cpu(buf + 6); } - lba = ube32_to_cpu(buf + 2); if (nb_sectors == 0) { ide_atapi_cmd_ok(s); return; } + lba = ldl_be_p(buf + 2); + if (lba >= total_sectors || lba + nb_sectors - 1 >= total_sectors) { + ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_LOGICAL_BLOCK_OOR); + return; + } + ide_atapi_cmd_read(s, lba, nb_sectors, 2048); } static void cmd_read_cd(IDEState *s, uint8_t* buf) { - int nb_sectors, lba, transfer_request; + unsigned int nb_sectors, lba, transfer_request; - nb_sectors = (buf[6] << 16) | (buf[7] << 8) | buf[8]; - lba = ube32_to_cpu(buf + 2); + /* Total logical sectors of ATAPI_SECTOR_SIZE(=2048) bytes */ + uint64_t total_sectors = s->nb_sectors >> 2; + nb_sectors = (buf[6] << 16) | (buf[7] << 8) | buf[8]; if (nb_sectors == 0) { ide_atapi_cmd_ok(s); return; } + lba = ldl_be_p(buf + 2); + if (lba >= total_sectors || lba + nb_sectors - 1 >= total_sectors) { + ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_LOGICAL_BLOCK_OOR); + return; + } + transfer_request = buf[9]; switch(transfer_request & 0xf8) { case 0x00: -- 2.18.2