diff mbox

scsi: avoid an off-by-one error in megasas_mmio_write

Message ID 20170424120634.12268-1-ppandit@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Prasad Pandit April 24, 2017, 12:06 p.m. UTC 
From: Prasad J Pandit <pjp@fedoraproject.org>

While reading magic sequence(MFI_SEQ) in megasas_mmio_write,
an off-by-one error could occur as 's->adp_reset' index is not
reset after reading the last sequence.

Reported-by: YY Z <bigbird475958471@gmail.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 hw/scsi/megasas.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Prasad Pandit April 28, 2017, 8:35 a.m. UTC | #1 
+-- On Mon, 24 Apr 2017, P J P wrote --+
| While reading magic sequence(MFI_SEQ) in megasas_mmio_write,
| an off-by-one error could occur as 's->adp_reset' index is not
| reset after reading the last sequence.
| 
| --- a/hw/scsi/megasas.c
| +++ b/hw/scsi/megasas.c
| @@ -2138,15 +2138,15 @@ static void megasas_mmio_write(void *opaque, hwaddr addr,
|      case MFI_SEQ:
|          trace_megasas_mmio_writel("MFI_SEQ", val);
|          /* Magic sequence to start ADP reset */
| -        if (adp_reset_seq[s->adp_reset] == val) {
| -            s->adp_reset++;
| +        if (adp_reset_seq[s->adp_reset++] == val) {
| +            if (s->adp_reset == 6) {
| +                s->adp_reset = 0;
| +                s->diag = MFI_DIAG_WRITE_ENABLE;
| +            }
|          } else {
|              s->adp_reset = 0;
|              s->diag = 0;
|          }
| -        if (s->adp_reset == 6) {
| -            s->diag = MFI_DIAG_WRITE_ENABLE;
| -        }
|          break;

Ping...!
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Paolo Bonzini April 28, 2017, 9:54 a.m. UTC | #2 
On 28/04/2017 10:35, P J P wrote:
> +-- On Mon, 24 Apr 2017, P J P wrote --+
> | While reading magic sequence(MFI_SEQ) in megasas_mmio_write,
> | an off-by-one error could occur as 's->adp_reset' index is not
> | reset after reading the last sequence.
> | 
> | --- a/hw/scsi/megasas.c
> | +++ b/hw/scsi/megasas.c
> | @@ -2138,15 +2138,15 @@ static void megasas_mmio_write(void *opaque, hwaddr addr,
> |      case MFI_SEQ:
> |          trace_megasas_mmio_writel("MFI_SEQ", val);
> |          /* Magic sequence to start ADP reset */
> | -        if (adp_reset_seq[s->adp_reset] == val) {
> | -            s->adp_reset++;
> | +        if (adp_reset_seq[s->adp_reset++] == val) {
> | +            if (s->adp_reset == 6) {
> | +                s->adp_reset = 0;
> | +                s->diag = MFI_DIAG_WRITE_ENABLE;
> | +            }
> |          } else {
> |              s->adp_reset = 0;
> |              s->diag = 0;
> |          }
> | -        if (s->adp_reset == 6) {
> | -            s->diag = MFI_DIAG_WRITE_ENABLE;
> | -        }
> |          break;
> 
> Ping...!
> --
> Prasad J Pandit / Red Hat Product Security Team
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
> 

I've already queued this patch.

Paolo
Prasad Pandit April 28, 2017, 11:03 a.m. UTC | #3 
+-- On Fri, 28 Apr 2017, Paolo Bonzini wrote --+
| > |          /* Magic sequence to start ADP reset */
| > | -        if (adp_reset_seq[s->adp_reset] == val) {
| > | -            s->adp_reset++;
| > | +        if (adp_reset_seq[s->adp_reset++] == val) {
| > | +            if (s->adp_reset == 6) {
| > | +                s->adp_reset = 0;
| > | +                s->diag = MFI_DIAG_WRITE_ENABLE;
| > | +            }
| > |          } else {
| > |              s->adp_reset = 0;
| > |              s->diag = 0;
| > |          }
| > | -        if (s->adp_reset == 6) {
| > | -            s->diag = MFI_DIAG_WRITE_ENABLE;
| > | -        }
| > |          break;
| 
| I've already queued this patch.

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

Patch

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 84b8caf..804122a 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2138,15 +2138,15 @@  static void megasas_mmio_write(void *opaque, hwaddr addr,
     case MFI_SEQ:
         trace_megasas_mmio_writel("MFI_SEQ", val);
         /* Magic sequence to start ADP reset */
-        if (adp_reset_seq[s->adp_reset] == val) {
-            s->adp_reset++;
+        if (adp_reset_seq[s->adp_reset++] == val) {
+            if (s->adp_reset == 6) {
+                s->adp_reset = 0;
+                s->diag = MFI_DIAG_WRITE_ENABLE;
+            }
         } else {
             s->adp_reset = 0;
             s->diag = 0;
         }
-        if (s->adp_reset == 6) {
-            s->diag = MFI_DIAG_WRITE_ENABLE;
-        }
         break;
     case MFI_DIAG:
         trace_megasas_mmio_writel("MFI_DIAG", val);