[Qemu-devel] [PATCH] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068)

[Qemu-devel] [PATCH] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068)

[P J P]

From: Prasad J Pandit <pjp@fedoraproject.org>

When executing script in lsi_execute_script(), the LSI scsi
adapter emulator advances 's->dsp' index to read next opcode.
This can lead to an infinite loop if the next opcode is empty.
Exit such loop after reading 10k empty opcodes.

Reported-by: Bugs SysSec <bugs-syssec@rub.de>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 hw/scsi/lsi53c895a.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 10468c1ec1..c23a40525e 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -1132,7 +1132,10 @@ static void lsi_execute_script(LSIState *s)
 
     s->istat1 |= LSI_ISTAT1_SRUN;
 again:
-    insn_processed++;
+    if (++insn_processed > 10000) {
+        s->waiting = LSI_NOWAIT;
+        goto exitloop;
+    }
     insn = read_dword(s, s->dsp);
     if (!insn) {
         /* If we receive an empty opcode increment the DSP by 4 bytes
@@ -1569,6 +1572,7 @@ again:
             }
         }
     }
+exitloop:
     if (insn_processed > 10000 && s->waiting == LSI_NOWAIT) {
         /* Some windows drivers make the device spin waiting for a memory
            location to change.  If we have been executed a lot of code then
-- 
2.21.0

Re: [Qemu-devel] [PATCH] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068)

[Stefano Garzarella]

On Thu, Aug 08, 2019 at 12:03:40PM +0530, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
> 
> When executing script in lsi_execute_script(), the LSI scsi
> adapter emulator advances 's->dsp' index to read next opcode.
> This can lead to an infinite loop if the next opcode is empty.
> Exit such loop after reading 10k empty opcodes.
> 
> Reported-by: Bugs SysSec <bugs-syssec@rub.de>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/scsi/lsi53c895a.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index 10468c1ec1..c23a40525e 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -1132,7 +1132,10 @@ static void lsi_execute_script(LSIState *s)
>  
>      s->istat1 |= LSI_ISTAT1_SRUN;
>  again:
> -    insn_processed++;
> +    if (++insn_processed > 10000) {
                              ^
Since we are using this "magic" number in several lines,
should we define a macro?

Thanks,
Stefano

> +        s->waiting = LSI_NOWAIT;
> +        goto exitloop;
> +    }
>      insn = read_dword(s, s->dsp);
>      if (!insn) {
>          /* If we receive an empty opcode increment the DSP by 4 bytes
> @@ -1569,6 +1572,7 @@ again:
>              }
>          }
>      }
> +exitloop:
>      if (insn_processed > 10000 && s->waiting == LSI_NOWAIT) {
>          /* Some windows drivers make the device spin waiting for a memory
>             location to change.  If we have been executed a lot of code then
> -- 
> 2.21.0
> 
> 

-- 

Re: [Qemu-devel] [PATCH] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068)

[Paolo Bonzini]

On 08/08/19 08:33, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
> 
> When executing script in lsi_execute_script(), the LSI scsi
> adapter emulator advances 's->dsp' index to read next opcode.
> This can lead to an infinite loop if the next opcode is empty.
> Exit such loop after reading 10k empty opcodes.
> 
> Reported-by: Bugs SysSec <bugs-syssec@rub.de>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/scsi/lsi53c895a.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index 10468c1ec1..c23a40525e 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -1132,7 +1132,10 @@ static void lsi_execute_script(LSIState *s)
>  
>      s->istat1 |= LSI_ISTAT1_SRUN;
>  again:
> -    insn_processed++;
> +    if (++insn_processed > 10000) {
> +        s->waiting = LSI_NOWAIT;
> +        goto exitloop;
> +    }
>      insn = read_dword(s, s->dsp);
>      if (!insn) {
>          /* If we receive an empty opcode increment the DSP by 4 bytes
> @@ -1569,6 +1572,7 @@ again:
>              }
>          }
>      }
> +exitloop:
>      if (insn_processed > 10000 && s->waiting == LSI_NOWAIT) {
>          /* Some windows drivers make the device spin waiting for a memory
>             location to change.  If we have been executed a lot of code then
> 

I am not sure this is worth a CVE.  The kernel can cause QEMU to break,
but is there a practical case in which an unprivileged user can do that?

Paolo

Re: [Qemu-devel] [PATCH] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068)

[P J P]

+-- On Thu, 8 Aug 2019, Stefano Garzarella wrote --+
| > +    if (++insn_processed > 10000) {
|                               ^
| Since we are using this "magic" number in several lines,
| should we define a macro?

Sent patch v2. Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068)

[P J P]

+-- On Thu, 8 Aug 2019, Paolo Bonzini wrote --+
| I am not sure this is worth a CVE. 

True, it is a low one, as QEMU consumes cycles on the host.

| The kernel can cause QEMU to break, but is there a practical case in which 
| an unprivileged user can do that?

QEMU does not break, it keeps running in interruptible sleep 'S' state. 
They've a reproducer wherein guest does mmio calls to trigger the issue.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068)

[Philippe Mathieu-Daudé]

On 8/8/19 11:48 AM, P J P wrote:
> +-- On Thu, 8 Aug 2019, Paolo Bonzini wrote --+
> | I am not sure this is worth a CVE. 
> 
> True, it is a low one, as QEMU consumes cycles on the host.
> 
> | The kernel can cause QEMU to break, but is there a practical case in which 
> | an unprivileged user can do that?
> 
> QEMU does not break, it keeps running in interruptible sleep 'S' state. 
> They've a reproducer wherein guest does mmio calls to trigger the issue.

From user-mode? As unprivileged user?

Re: [Qemu-devel] [PATCH] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068)

[Paolo Bonzini]

On 08/08/19 11:48, P J P wrote:
> +-- On Thu, 8 Aug 2019, Paolo Bonzini wrote --+
> | I am not sure this is worth a CVE. 
> 
> True, it is a low one, as QEMU consumes cycles on the host.

A guest that runs an infinite loop would be an easier way to do that.  I
suppose this one also blocks the monitor, but then "kill -9" is always
your friend. :)

Paolo

> | The kernel can cause QEMU to break, but is there a practical case in which 
> | an unprivileged user can do that?
> 
> QEMU does not break, it keeps running in interruptible sleep 'S' state. 
> They've a reproducer wherein guest does mmio calls to trigger the issue.

Re: [Qemu-devel] [PATCH] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068)

[P J P]

+-- On Thu, 8 Aug 2019, Philippe Mathieu-Daudé wrote --+
| >From user-mode? As unprivileged user?

No, needs privileges inside guest.

--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068)

[P J P]

+-- On Thu, 8 Aug 2019, Paolo Bonzini wrote --+
| I suppose this one also blocks the monitor, but then "kill -9" is always 
| your friend. :)

True. :)
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F