Brad Bishop | 64c979e | 2019-11-04 13:55:29 -0500 | [diff] [blame^] | 1 | From de594e47659029316bbf9391efb79da0a1a08e08 Mon Sep 17 00:00:00 2001 |
| 2 | From: Paolo Bonzini <pbonzini@redhat.com> |
| 3 | Date: Wed, 14 Aug 2019 17:35:21 +0530 |
| 4 | Subject: [PATCH] scsi: lsi: exit infinite loop while executing script |
| 5 | (CVE-2019-12068) |
| 6 | |
| 7 | When executing script in lsi_execute_script(), the LSI scsi adapter |
| 8 | emulator advances 's->dsp' index to read next opcode. This can lead |
| 9 | to an infinite loop if the next opcode is empty. Move the existing |
| 10 | loop exit after 10k iterations so that it covers no-op opcodes as |
| 11 | well. |
| 12 | |
| 13 | Upstream-Status: Backport [https://git.qemu.org/?p=qemu.git;a=commit;h=de594e47659029316bbf9391efb79da0a1a08e08] |
| 14 | CVE: CVE-2019-12068 |
| 15 | |
| 16 | Reported-by: Bugs SysSec <bugs-syssec@rub.de> |
| 17 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
| 18 | Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> |
| 19 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
| 20 | |
| 21 | Signed-off-by: Changqing Li <changqing.li@windriver.com> |
| 22 | --- |
| 23 | hw/scsi/lsi53c895a.c | 41 +++++++++++++++++++++++++++-------------- |
| 24 | 1 file changed, 27 insertions(+), 14 deletions(-) |
| 25 | |
| 26 | diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c |
| 27 | index 222a286..ec53b14 100644 |
| 28 | --- a/hw/scsi/lsi53c895a.c |
| 29 | +++ b/hw/scsi/lsi53c895a.c |
| 30 | @@ -186,6 +186,9 @@ static const char *names[] = { |
| 31 | /* Flag set if this is a tagged command. */ |
| 32 | #define LSI_TAG_VALID (1 << 16) |
| 33 | |
| 34 | +/* Maximum instructions to process. */ |
| 35 | +#define LSI_MAX_INSN 10000 |
| 36 | + |
| 37 | typedef struct lsi_request { |
| 38 | SCSIRequest *req; |
| 39 | uint32_t tag; |
| 40 | @@ -1133,7 +1136,21 @@ static void lsi_execute_script(LSIState *s) |
| 41 | |
| 42 | s->istat1 |= LSI_ISTAT1_SRUN; |
| 43 | again: |
| 44 | - insn_processed++; |
| 45 | + if (++insn_processed > LSI_MAX_INSN) { |
| 46 | + /* Some windows drivers make the device spin waiting for a memory |
| 47 | + location to change. If we have been executed a lot of code then |
| 48 | + assume this is the case and force an unexpected device disconnect. |
| 49 | + This is apparently sufficient to beat the drivers into submission. |
| 50 | + */ |
| 51 | + if (!(s->sien0 & LSI_SIST0_UDC)) { |
| 52 | + qemu_log_mask(LOG_GUEST_ERROR, |
| 53 | + "lsi_scsi: inf. loop with UDC masked"); |
| 54 | + } |
| 55 | + lsi_script_scsi_interrupt(s, LSI_SIST0_UDC, 0); |
| 56 | + lsi_disconnect(s); |
| 57 | + trace_lsi_execute_script_stop(); |
| 58 | + return; |
| 59 | + } |
| 60 | insn = read_dword(s, s->dsp); |
| 61 | if (!insn) { |
| 62 | /* If we receive an empty opcode increment the DSP by 4 bytes |
| 63 | @@ -1570,19 +1587,7 @@ again: |
| 64 | } |
| 65 | } |
| 66 | } |
| 67 | - if (insn_processed > 10000 && s->waiting == LSI_NOWAIT) { |
| 68 | - /* Some windows drivers make the device spin waiting for a memory |
| 69 | - location to change. If we have been executed a lot of code then |
| 70 | - assume this is the case and force an unexpected device disconnect. |
| 71 | - This is apparently sufficient to beat the drivers into submission. |
| 72 | - */ |
| 73 | - if (!(s->sien0 & LSI_SIST0_UDC)) { |
| 74 | - qemu_log_mask(LOG_GUEST_ERROR, |
| 75 | - "lsi_scsi: inf. loop with UDC masked"); |
| 76 | - } |
| 77 | - lsi_script_scsi_interrupt(s, LSI_SIST0_UDC, 0); |
| 78 | - lsi_disconnect(s); |
| 79 | - } else if (s->istat1 & LSI_ISTAT1_SRUN && s->waiting == LSI_NOWAIT) { |
| 80 | + if (s->istat1 & LSI_ISTAT1_SRUN && s->waiting == LSI_NOWAIT) { |
| 81 | if (s->dcntl & LSI_DCNTL_SSM) { |
| 82 | lsi_script_dma_interrupt(s, LSI_DSTAT_SSI); |
| 83 | } else { |
| 84 | @@ -1970,6 +1975,10 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val) |
| 85 | case 0x2f: /* DSP[24:31] */ |
| 86 | s->dsp &= 0x00ffffff; |
| 87 | s->dsp |= val << 24; |
| 88 | + /* |
| 89 | + * FIXME: if s->waiting != LSI_NOWAIT, this will only execute one |
| 90 | + * instruction. Is this correct? |
| 91 | + */ |
| 92 | if ((s->dmode & LSI_DMODE_MAN) == 0 |
| 93 | && (s->istat1 & LSI_ISTAT1_SRUN) == 0) |
| 94 | lsi_execute_script(s); |
| 95 | @@ -1988,6 +1997,10 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val) |
| 96 | break; |
| 97 | case 0x3b: /* DCNTL */ |
| 98 | s->dcntl = val & ~(LSI_DCNTL_PFF | LSI_DCNTL_STD); |
| 99 | + /* |
| 100 | + * FIXME: if s->waiting != LSI_NOWAIT, this will only execute one |
| 101 | + * instruction. Is this correct? |
| 102 | + */ |
| 103 | if ((val & LSI_DCNTL_STD) && (s->istat1 & LSI_ISTAT1_SRUN) == 0) |
| 104 | lsi_execute_script(s); |
| 105 | break; |
| 106 | -- |
| 107 | 2.7.4 |
| 108 | |