Re: [PATCH v2] disk: Move hardcoded max disk size literal to a GRUB_DISK_MAX_SECTORS in disk.h

[Daniel Kiper]

On Mon, Nov 30, 2020 at 11:16:19PM -0600, Glenn Washburn wrote:
> There is a hardcoded maximum disk size that can be read or written from,
> currently set at 1EiB in grub_disk_adjust_range. Move the literal into a
> macro in disk.h, so our assumptions are more visible. This hard coded limit
> does not prevent using larger disks, just grub won't read/write past the
> limit. The comment accompanying this restriction didn't quite make sense to
> me, so its been modified.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Except one nit...

> ---
>  grub-core/kern/disk_common.c | 15 ++++++++-------
>  include/grub/disk.h          |  6 ++++++
>  2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/grub-core/kern/disk_common.c b/grub-core/kern/disk_common.c
> index 2ca12b5f8..d43b927a4 100644
> --- a/grub-core/kern/disk_common.c
> +++ b/grub-core/kern/disk_common.c
> @@ -32,13 +32,14 @@ grub_disk_adjust_range (grub_disk_t disk, 
> grub_disk_addr_t *sector,
>    /* Transform total_sectors to number of 512B blocks.  */
>    total_sectors = disk->total_sectors << (disk->log_sector_size - 
> GRUB_DISK_SECTOR_BITS);
>
> -  /* Some drivers have problems with disks above reasonable.
> -     Treat unknown as 1EiB disk. While on it, clamp the size to 1EiB.
> -     Just one condition is enough since GRUB_DISK_UNKNOWN_SIZE << ls is 
> always
> -     above 9EiB.
> -  */
> -  if (total_sectors > (1ULL << 51))
> -    total_sectors = (1ULL << 51);
> +  /*
> +   * Some drivers have problems with disks above reasonable sizes.
> +   * Clamp the size to GRUB_DISK_MAX_SECTORS. Just one condition is enough
> +   * since GRUB_DISK_UNKNOWN_SIZE is always above GRUB_DISK_MAX_SECTORS,

s/GRUB_DISK_UNKNOWN_SIZE/GRUB_DISK_SIZE_UNKNOWN/

I can fix it before committing the patch.

Daniel