hw/scsi/lsi53c895a.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
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
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
>
>
--
+-- 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
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
+-- 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
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?
+-- 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
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.
+-- 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
© 2016 - 2025 Red Hat, Inc.