[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/2] scsi: lsi: exit infinite loop while exec
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/2] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068) |
Date: |
Wed, 14 Aug 2019 12:38:39 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
On 14/08/19 12:25, P J P wrote:
> +-- On Tue, 13 Aug 2019, Paolo Bonzini wrote --+
> | After the first instruction is processed, "again" is only reached if
> | s->waiting == LSI_NOWAIT. Therefore, we could move the Windows hack to the
> | beginning and remove the s->waiting condition. The only change would be
> | that it would also be triggered by all zero instructions, like this:
> |
> | diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> | index 10468c1..9d714af 100644
> | --- a/hw/scsi/lsi53c895a.c
> | +++ b/hw/scsi/lsi53c895a.c
> | @@ -185,6 +185,9 @@ static const char *names[] = {
> | /* Flag set if this is a tagged command. */
> | #define LSI_TAG_VALID (1 << 16)
> |
> | +/* Maximum instructions to process. */
> | +#define LSI_MAX_INSN 10000
> | +
> | typedef struct lsi_request {
> | SCSIRequest *req;
> | uint32_t tag;
> | @@ -1132,7 +1135,19 @@ static void lsi_execute_script(LSIState *s)
> |
> | s->istat1 |= LSI_ISTAT1_SRUN;
> | again:
> | - insn_processed++;
> | + if (++insn_processed > LSI_MAX_INSN) {
> | + /* Some windows drivers make the device spin waiting for a memory
> | + location to change. If we have been executed a lot of code then
> | + assume this is the case and force an unexpected device
> disconnect.
> | + This is apparently sufficient to beat the drivers into
> submission.
> | + */
> | + if (!(s->sien0 & LSI_SIST0_UDC)) {
> | + qemu_log_mask(LOG_GUEST_ERROR,
> | + "lsi_scsi: inf. loop with UDC masked");
> | + }
> | + lsi_script_scsi_interrupt(s, LSI_SIST0_UDC, 0);
> | + lsi_disconnect(s);
> ...
> |
> | Does it make sense?
>
> Yes, this'd also work, but need to return after lsi_disconnect(s), otherwise
> loop would continue.
>
> Should I send a revised patch? (with above change)
Yes, please.
Paolo
[Qemu-devel] [PATCH v3 2/2] scsi: lsi: use macro LSI_MAX_INSN instead of a magic number, P J P, 2019/08/09