In ide_set_sector() if we're using CHS addressing the way we update
the drive state involves dividing by (s->heads * s->sectors).  These
values can be set by the guest using the INITIALIZE DRIVE PARAMETERS
command, which means that s->sectors can be 0.  (s->heads can't be 0
because the command passes a heads-1 value.) INITIALIZE DRIVE
PARAMETERS is specified to not check the input values for validity;
instead no error is posted until some other command makes an illegal
access. So we have to cope with a 0 divisor here.

Special-case s->sectors being zero.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2399
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
We don't seem to ever actually check that the guest's read/write ops
are within the CHS limits it sets with INITIALIZE DRIVE PARAMETERS --
ide_sect_range_ok() only checks against the underlying block device
geometry.  Maybe we should?  Then a zero-sectors misconfig would
result in our reporting the error on the read/write as the spec
suggests we should.  This patch fixes the division-by-zero, at least,
though.
---
 hw/ide/core.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index f78b00220b..b030531534 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -646,13 +646,27 @@ void ide_set_sector(IDEState *s, int64_t sector_num)
         }
     } else {
         /* CHS */
-        cyl = sector_num / (s->heads * s->sectors);
-        r = sector_num % (s->heads * s->sectors);
-        s->hcyl = cyl >> 8;
-        s->lcyl = cyl;
-        s->select = (s->select & ~(ATA_DEV_HS)) |
-            ((r / s->sectors) & (ATA_DEV_HS));
-        s->sector = (r % s->sectors) + 1;
+        if (s->sectors == 0) {
+            /*
+             * s->sectors is under guest control via INITIALIZE DRIVE
+             * PARAMETERS and can be 0; this is an error but can't
+             * be reported at INITIALIZE DRIVE PARAMETERS time so we
+             * have to cope with the bad value here.
+             * (s->heads is always at least 1.)
+             */
+            s->hcyl = 0;
+            s->lcyl = 0;
+            s->select = s->select & ~(ATA_DEV_HS);
+            s->sector = 1;
+        } else {
+            cyl = sector_num / (s->heads * s->sectors);
+            r = sector_num % (s->heads * s->sectors);
+            s->hcyl = cyl >> 8;
+            s->lcyl = cyl;
+            s->select = (s->select & ~(ATA_DEV_HS)) |
+                ((r / s->sectors) & (ATA_DEV_HS));
+            s->sector = (r % s->sectors) + 1;
+        }
     }
 }
 
-- 
2.43.0

Ping -- would anybody on the qemu-block side like to review this?

thanks
-- PMM

On Tue, 30 Jun 2026 at 11:42, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In ide_set_sector() if we're using CHS addressing the way we update
> the drive state involves dividing by (s->heads * s->sectors).  These
> values can be set by the guest using the INITIALIZE DRIVE PARAMETERS
> command, which means that s->sectors can be 0.  (s->heads can't be 0
> because the command passes a heads-1 value.) INITIALIZE DRIVE
> PARAMETERS is specified to not check the input values for validity;
> instead no error is posted until some other command makes an illegal
> access. So we have to cope with a 0 divisor here.
>
> Special-case s->sectors being zero.
>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2399
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> We don't seem to ever actually check that the guest's read/write ops
> are within the CHS limits it sets with INITIALIZE DRIVE PARAMETERS --
> ide_sect_range_ok() only checks against the underlying block device
> geometry.  Maybe we should?  Then a zero-sectors misconfig would
> result in our reporting the error on the read/write as the spec
> suggests we should.  This patch fixes the division-by-zero, at least,
> though.
> ---
>  hw/ide/core.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index f78b00220b..b030531534 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -646,13 +646,27 @@ void ide_set_sector(IDEState *s, int64_t sector_num)
>          }
>      } else {
>          /* CHS */
> -        cyl = sector_num / (s->heads * s->sectors);
> -        r = sector_num % (s->heads * s->sectors);
> -        s->hcyl = cyl >> 8;
> -        s->lcyl = cyl;
> -        s->select = (s->select & ~(ATA_DEV_HS)) |
> -            ((r / s->sectors) & (ATA_DEV_HS));
> -        s->sector = (r % s->sectors) + 1;
> +        if (s->sectors == 0) {
> +            /*
> +             * s->sectors is under guest control via INITIALIZE DRIVE
> +             * PARAMETERS and can be 0; this is an error but can't
> +             * be reported at INITIALIZE DRIVE PARAMETERS time so we
> +             * have to cope with the bad value here.
> +             * (s->heads is always at least 1.)
> +             */
> +            s->hcyl = 0;
> +            s->lcyl = 0;
> +            s->select = s->select & ~(ATA_DEV_HS);
> +            s->sector = 1;
> +        } else {
> +            cyl = sector_num / (s->heads * s->sectors);
> +            r = sector_num % (s->heads * s->sectors);
> +            s->hcyl = cyl >> 8;
> +            s->lcyl = cyl;
> +            s->select = (s->select & ~(ATA_DEV_HS)) |
> +                ((r / s->sectors) & (ATA_DEV_HS));
> +            s->sector = (r % s->sectors) + 1;
> +        }
>      }
>  }