[RFC PATCH] hw/block: validate the fdc sector position fits within bounds

Previously the guest controlled the value of last_sec and wasn't
particularly careful to check it didn't exceed the media size. We
can't re-use drv->last_sec as that changes as we do operations so we
set media_last_sect when the geometry is probed.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3800
Signed-off-by: Alex Bennée <[email protected]>
---
 hw/block/fdc-internal.h |  1 +
 hw/block/fdc.c          | 28 ++++++++++++++++++++++------
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/hw/block/fdc-internal.h b/hw/block/fdc-internal.h
index e219623dc7a..81775fe55aa 100644
--- a/hw/block/fdc-internal.h
+++ b/hw/block/fdc-internal.h
@@ -83,6 +83,7 @@ typedef struct FDrive {
     uint8_t max_track;        /* Nb of tracks           */
     uint16_t bps;             /* Bytes per sector       */
     uint8_t ro;               /* Is read-only           */
+    uint8_t media_last_sect;  /* last sector of current track according to media */
     uint8_t media_changed;    /* Is media changed       */
     uint8_t media_rate;       /* Data rate of medium    */
 
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 1178b959a64..18a61f5ce87 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -192,6 +192,7 @@ static void fd_init(FDrive *drv)
     drv->max_track = 0;
     drv->ro = true;
     drv->media_changed = 1;
+    drv->media_last_sect = 0;
 }
 
 #define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1)
@@ -373,6 +374,7 @@ static int pick_geometry(FDrive *drv)
     }
     drv->max_track = parse->max_track;
     drv->last_sect = parse->last_sect;
+    drv->media_last_sect = parse->last_sect;
     drv->disk = parse->drive;
     drv->media_rate = parse->rate;
     return 0;
@@ -1905,6 +1907,17 @@ static void fdctrl_handle_partid(FDCtrl *fdctrl, int direction)
     fdctrl_to_result_phase(fdctrl, 1);
 }
 
+static bool fd_validate_last_sect(FDrive *drv, uint8_t new_last_sect_val)
+{
+    if (drv->media_validated && new_last_sect_val > drv->media_last_sect) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "FDC: Guest attempted to set last_sect to %u, exceeding valid media max of %u\n",
+                      new_last_sect_val, drv->media_last_sect);
+        return false;
+    }
+    return true;
+}
+
 static void fdctrl_handle_restore(FDCtrl *fdctrl, int direction)
 {
     FDrive *cur_drv = get_cur_drv(fdctrl);
@@ -1919,6 +1932,10 @@ static void fdctrl_handle_restore(FDCtrl *fdctrl, int direction)
     /* timers */
     fdctrl->timer0 = fdctrl->fifo[7];
     fdctrl->timer1 = fdctrl->fifo[8];
+    if (!fd_validate_last_sect(cur_drv, fdctrl->fifo[9])) {
+        fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_EC, 0x00);
+        return;
+    }
     cur_drv->last_sect = fdctrl->fifo[9];
     fdctrl->lock = fdctrl->fifo[10] >> 7;
     cur_drv->perpendicular = (fdctrl->fifo[10] >> 2) & 0xF;
@@ -1983,13 +2000,12 @@ static void fdctrl_handle_format_track(FDCtrl *fdctrl, int direction)
         fdctrl->data_state &= ~FD_STATE_MULTI;
     cur_drv->bps =
         fdctrl->fifo[2] > 7 ? 16384 : 128 << fdctrl->fifo[2];
-#if 0
-    cur_drv->last_sect =
-        cur_drv->flags & FDISK_DBL_SIDES ? fdctrl->fifo[3] :
-        fdctrl->fifo[3] / 2;
-#else
+
+    if (!fd_validate_last_sect(cur_drv, fdctrl->fifo[3])) {
+        fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_EC, 0x00);
+        return;
+    }
     cur_drv->last_sect = fdctrl->fifo[3];
-#endif
     /* TODO: implement format using DMA expected by the Bochs BIOS
      * and Linux fdformat (read 3 bytes per sector via DMA and fill
      * the sector with the specified fill byte
-- 
2.47.3