diff mbox

scsi: fix sign extension problems

Message ID 1315579646-6874-1-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Sept. 9, 2011, 2:47 p.m. UTC 
When assigning a 32-bit value to cmd->xfer (which is 64-bits)
it can be erroneously sign extended because the intermediate
32-bit computation is signed.  Fix this by standardizing on
the ld*_be_p functions.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-bus.c |   22 +++++++---------------
 1 files changed, 7 insertions(+), 15 deletions(-)

Comments

Kevin Wolf Sept. 19, 2011, 3 p.m. UTC | #1 
Am 09.09.2011 16:47, schrieb Paolo Bonzini:
> When assigning a 32-bit value to cmd->xfer (which is 64-bits)
> it can be erroneously sign extended because the intermediate
> 32-bit computation is signed.  Fix this by standardizing on
> the ld*_be_p functions.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks, applied to the block branch.

Kevin
diff mbox

Patch

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 96d6305..731d3b9 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -546,15 +546,15 @@  static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
         break;
     case 1:
     case 2:
-        cmd->xfer = buf[8] | (buf[7] << 8);
+        cmd->xfer = lduw_be_p(&buf[7]);
         cmd->len = 10;
         break;
     case 4:
-        cmd->xfer = buf[13] | (buf[12] << 8) | (buf[11] << 16) | (buf[10] << 24);
+        cmd->xfer = ldl_be_p(&buf[10]);
         cmd->len = 16;
         break;
     case 5:
-        cmd->xfer = buf[9] | (buf[8] << 8) | (buf[7] << 16) | (buf[6] << 24);
+        cmd->xfer = ldl_be_p(&buf[6]);
         cmd->len = 12;
         break;
     default:
@@ -714,23 +714,15 @@  static uint64_t scsi_cmd_lba(SCSICommand *cmd)
 
     switch (buf[0] >> 5) {
     case 0:
-        lba = (uint64_t) buf[3] | ((uint64_t) buf[2] << 8) |
-              (((uint64_t) buf[1] & 0x1f) << 16);
+        lba = ldl_be_p(&buf[0]) & 0x1fffff;
         break;
     case 1:
     case 2:
-        lba = (uint64_t) buf[5] | ((uint64_t) buf[4] << 8) |
-              ((uint64_t) buf[3] << 16) | ((uint64_t) buf[2] << 24);
+    case 5:
+        lba = ldl_be_p(&buf[2]);
         break;
     case 4:
-        lba = (uint64_t) buf[9] | ((uint64_t) buf[8] << 8) |
-              ((uint64_t) buf[7] << 16) | ((uint64_t) buf[6] << 24) |
-              ((uint64_t) buf[5] << 32) | ((uint64_t) buf[4] << 40) |
-              ((uint64_t) buf[3] << 48) | ((uint64_t) buf[2] << 56);
-        break;
-    case 5:
-        lba = (uint64_t) buf[5] | ((uint64_t) buf[4] << 8) |
-              ((uint64_t) buf[3] << 16) | ((uint64_t) buf[2] << 24);
+        lba = ldq_be_p(&buf[2]);
         break;
     default:
         lba = -1;