Re: [PATCH] Fix integer overflow at left shift expression

[Glenn Washburn]

On Sun, 04 Dec 2022 13:06:37 +0000
Maxim Fomin <maxim@fomin.one> wrote:

> From db82faafba5e7eccd9fd6c0b7314f7322c1aecbd Mon Sep 17 00:00:00 2001
> From: Maxim Fomin <maxim@fomin.one>
> Date: Sun, 4 Dec 2022 12:05:34 +0000
> Subject: [PATCH] Fix integer overflow at left shift expression.
> 
> In case of large partitions (>1TiB) left shift
> with signed int GRUB_DISK_SECTOR_BITS macro may
> cause integer overflow which results in wrong
> partition size.
> ---
>  grub-core/kern/fs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/kern/fs.c b/grub-core/kern/fs.c
> index b9508296d..c196f2bf1 100644
> --- a/grub-core/kern/fs.c
> +++ b/grub-core/kern/fs.c
> @@ -130,7 +130,7 @@ grub_fs_probe (grub_device_t device)
>  struct grub_fs_block
>  {
>    grub_disk_addr_t offset;
> -  unsigned long length;
> +  grub_disk_addr_t length;
>  };
>  
>  static grub_err_t
> @@ -195,7 +195,7 @@ grub_fs_blocklist_open (grub_file_t file, const
> char *name) goto fail;
>       }
>  
> -      file->size += (blocks[i].length << GRUB_DISK_SECTOR_BITS);
> +      file->size += (blocks[i].length << (grub_disk_addr_t)
> GRUB_DISK_SECTOR_BITS);
>       p++;

Is this change actually necessary? You're making sure that
GRUB_DISK_SECTOR_BITS is treated as a 64-bit integer, but it would be
crazy for it to even be more than an 8 bit integer. Is there some other
desirable effect of this?

>      }
>  

Glenn