[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 07/11] esp: don't underflow cmdfifo in do_cmd()
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v3 07/11] esp: don't underflow cmdfifo in do_cmd() |
Date: |
Thu, 1 Apr 2021 10:19:09 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 |
On 4/1/21 9:49 AM, Mark Cave-Ayland wrote:
> If the guest tries to execute a CDB when cmdfifo is not empty before the start
> of the message out phase then clearing the message out phase data will cause
> cmdfifo to underflow due to cmdfifo_cdb_offset being larger than the amount of
> data within.
>
> Since this can only occur by issuing deliberately incorrect instruction
> sequences, ensure that the maximum length of esp_fifo_pop_buf() is limited to
> the size of the data within cmdfifo.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1909247
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/scsi/esp.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 4decbbfc29..7f49522e1d 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -319,13 +319,15 @@ static void do_busid_cmd(ESPState *s, uint8_t busid)
>
> static void do_cmd(ESPState *s)
> {
> - uint8_t busid = fifo8_pop(&s->cmdfifo);
> + uint8_t busid = esp_fifo_pop(&s->cmdfifo);
> + int len;
>
> s->cmdfifo_cdb_offset--;
>
> /* Ignore extended messages for now */
> if (s->cmdfifo_cdb_offset) {
> - esp_fifo_pop_buf(&s->cmdfifo, NULL, s->cmdfifo_cdb_offset);
> + len = MIN(s->cmdfifo_cdb_offset, fifo8_num_used(&s->cmdfifo));
Do we want to log(GUEST_ERRORS) this?
> + esp_fifo_pop_buf(&s->cmdfifo, NULL, len);
> s->cmdfifo_cdb_offset = 0;
> }
>
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
- Re: [PATCH v3 03/11] esp: consolidate esp_cmdfifo_push() into esp_fifo_push(), (continued)
- [PATCH v3 06/11] esp: ensure cmdfifo is not empty and current_dev is non-NULL, Mark Cave-Ayland, 2021/04/01
- [PATCH v3 07/11] esp: don't underflow cmdfifo in do_cmd(), Mark Cave-Ayland, 2021/04/01
- Re: [PATCH v3 07/11] esp: don't underflow cmdfifo in do_cmd(),
Philippe Mathieu-Daudé <=
- [PATCH v3 08/11] esp: don't overflow cmdfifo in get_cmd(), Mark Cave-Ayland, 2021/04/01
- [PATCH v3 09/11] esp: don't overflow cmdfifo if TC is larger than the cmdfifo size, Mark Cave-Ayland, 2021/04/01
- [PATCH v3 10/11] esp: don't reset async_len directly in esp_select() if cancelling request, Mark Cave-Ayland, 2021/04/01
- [PATCH v3 11/11] tests/qtest: add tests for am53c974 device, Mark Cave-Ayland, 2021/04/01
- Re: [PATCH v3 00/11] esp: fix asserts/segfaults discovered by fuzzer, Alexander Bulekov, 2021/04/01