[PATCH] disk: fail LBA I/O with zero sector count

[Paolo Bonzini]

Unlike basic_access, extended_access does not check for a zero
sector count.  However, this is a problem because for example
it would be interpreted as 256 when processing an ATA request.

Signed-off-by: Paolo Bonzini 
---
 src/disk.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/disk.c b/src/disk.c
index f2c6621..a124a0f 100644
--- a/src/disk.c
+++ b/src/disk.c
@@ -127,20 +127,24 @@ extended_access(struct bregs *regs, struct drive_s *drive_g, u16 command)
     dop.lba = GET_INT13EXT(regs, lba);
     dop.command = command;
     dop.drive_g = drive_g;
-    if (dop.lba >= GET_GLOBAL(drive_g->sectors)) {
-        warn_invalid(regs);
-        disk_ret(regs, DISK_RET_EPARAM);
-        return;
-    }
+    if (dop.lba >= GET_GLOBAL(drive_g->sectors))
+        goto fail;

dop.buf_fl = SEGOFF_TO_FLATPTR(GET_INT13EXT(regs, data));
     dop.count = GET_INT13EXT(regs, count);
+    if (dop.count == 0)
+        goto fail;

int status = send_disk_op(&dop);

SET_INT13EXT(regs, count, dop.count);

disk_ret(regs, status);
+    return;
+
+fail:
+    warn_invalid(regs);
+    disk_ret(regs, DISK_RET_EPARAM);
 }

[Kevin O'Connor]

On Fri, Feb 10, 2012 at 02:46:30PM +0100, Paolo Bonzini wrote:
> Unlike basic_access, extended_access does not check for a zero
> sector count.  However, this is a problem because for example
> it would be interpreted as 256 when processing an ATA request.

Thanks.  The EDD spec seems to imply that sending a zero isn't an
error.  How about the below instead?

-Kevin

--- a/src/disk.c
+++ b/src/disk.c
@@ -135,6 +135,11 @@ extended_access(struct bregs *regs, struct drive_s *drive_g, u16 command)

dop.buf_fl = SEGOFF_TO_FLATPTR(GET_INT13EXT(regs, data));
     dop.count = GET_INT13EXT(regs, count);
+    if (! dop.count) {
+        // Nothing to do.
+        disk_ret(regs, DISK_RET_SUCCESS);
+        return;
+    }

int status = send_disk_op(&dop);

[Paolo Bonzini]

On 02/11/2012 03:38 PM, Kevin O'Connor wrote:
> > > Unlike basic_access, extended_access does not check for a zero
> > > sector count.  However, this is a problem because for example
> > > it would be interpreted as 256 when processing an ATA request.
>
> Thanks.  The EDD spec seems to imply that sending a zero isn't an
> error.  How about the below instead?

That's okay too, of course.

Paolo