ide: set an upper limit to nb_sectors

Commit Message

Prasad Pandit Jan. 19, 2021, 1:42 p.m. UTC

From: Prasad J Pandit <[e-mail]>

Set an upper limit to number of sectors on an IDE disk media.
This is to ensure that logical block addresses (LBA) and
nb_sector arguments remain within INT_MAX range.

Suggested-by: Paolo Bonzini <[e-mail]>
Signed-off-by: Prasad J Pandit <[e-mail]>
---
 hw/ide/core.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Update: limit s->nb_sectors count
  -> https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg04270.html
  -> https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg04173.html

--
2.29.2

Comments

Philippe Mathieu-Daudé Jan. 19, 2021, 2:13 p.m. UTC | #1

Hi Prasad,

On 1/19/21 2:42 PM, P J P wrote:
> From: Prasad J Pandit <[e-mail]>
> 
> Set an upper limit to number of sectors on an IDE disk media.
> This is to ensure that logical block addresses (LBA) and
> nb_sector arguments remain within INT_MAX range.
> 
> Suggested-by: Paolo Bonzini <[e-mail]>
> Signed-off-by: Prasad J Pandit <[e-mail]>
> ---
>  hw/ide/core.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> Update: limit s->nb_sectors count
>   -> https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg04270.html
>   -> https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg04173.html
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index b49e4cfbc6..064998804a 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -1161,15 +1161,21 @@ static void ide_cfata_metadata_write(IDEState *s)
>                                      s->nsector << 9), 0x200 - 2));
>  }
> 
> +static void ide_set_nb_sectors(IDEState *s)
> +{
> +    uint64_t nb_sectors;
> +
> +    blk_get_geometry(s->blk, &nb_sectors);

       /* An explanation here would be useful */

or better, add a self-explaining definition for this magic value.

> +    s->nb_sectors = MIN(nb_sectors, (uint64_t)INT_MAX << 2);
> +}

Patch

diff --git a/hw/ide/core.c b/hw/ide/core.c
index b49e4cfbc6..064998804a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1161,15 +1161,21 @@  static void ide_cfata_metadata_write(IDEState *s)
                                     s->nsector << 9), 0x200 - 2));
 }

+static void ide_set_nb_sectors(IDEState *s)
+{
+    uint64_t nb_sectors;
+
+    blk_get_geometry(s->blk, &nb_sectors);
+    s->nb_sectors = MIN(nb_sectors, (uint64_t)INT_MAX << 2);
+}
+
 /* called when the inserted state of the media has changed */
 static void ide_cd_change_cb(void *opaque, bool load, Error **errp)
 {
     IDEState *s = opaque;
-    uint64_t nb_sectors;

     s->tray_open = !load;
-    blk_get_geometry(s->blk, &nb_sectors);
-    s->nb_sectors = nb_sectors;
+    ide_set_nb_sectors(s);

     /*
      * First indicate to the guest that a CD has been removed.  That's
@@ -2475,14 +2481,12 @@  static bool ide_cd_is_medium_locked(void *opaque)
 static void ide_resize_cb(void *opaque)
 {
     IDEState *s = opaque;
-    uint64_t nb_sectors;

     if (!s->identify_set) {
         return;
     }

-    blk_get_geometry(s->blk, &nb_sectors);
-    s->nb_sectors = nb_sectors;
+    ide_set_nb_sectors(s);

     /* Update the identify data buffer. */
     if (s->drive_kind == IDE_CFATA) {
@@ -2511,17 +2515,14 @@  int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,
                    uint32_t cylinders, uint32_t heads, uint32_t secs,
                    int chs_trans, Error **errp)
 {
-    uint64_t nb_sectors;
-
     s->blk = blk;
     s->drive_kind = kind;
-
-    blk_get_geometry(blk, &nb_sectors);
     s->cylinders = cylinders;
     s->heads = heads;
     s->sectors = secs;
     s->chs_trans = chs_trans;
-    s->nb_sectors = nb_sectors;
+    ide_set_nb_sectors(s);
+
     s->wwn = wwn;
     /* The SMART values should be preserved across power cycles
        but they aren't.  */