Re: [CRYPTO-LUKS v2 03/19] cryptodisk: Incorrect calculation of sector in grub_cryptodisk_read/write.

[Glenn Washburn]

Ignore this patch, I neglected to run my test until after I had sent
the patch.  My tests immediately found an error. Please see version v3.

On Mon, 24 Aug 2020 18:42:30 -0500
Glenn Washburn <development@efficientek.com> wrote:

> Here dev is a grub_cryptodisk_t and dev->offset is offset in sectors
> of size native to the cryptodisk device. The sector is correctly
> transformed into native grub sector size, but then added to
> dev->offset which is not transformed. Create function
> grub_disk_from_native_sector to do the native disk sector to grub
> sector size conversion.
> 
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/disk/cryptodisk.c | 12 +++++-------
>  include/grub/disk.h         |  7 +++++++
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index b87925ad3..b3610a1b6 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -761,10 +761,10 @@ grub_cryptodisk_read (grub_disk_t disk,
> grub_disk_addr_t sector, PRIxGRUB_UINT64_T " with offset of %"
> PRIuGRUB_UINT64_T "\n", size, sector, dev->offset);
>  
> +  sector = sector + dev->offset;
>    err = grub_disk_read (dev->source_disk,
> -                     (sector << (disk->log_sector_size
> -                                - GRUB_DISK_SECTOR_BITS)) +
> dev->offset, 0,
> -                     size << disk->log_sector_size, buf);
> +                     grub_disk_from_native_sector (disk, sector),
> +                     0, size << disk->log_sector_size, buf);
>    if (err)
>      {
>        grub_dprintf ("cryptodisk", "grub_disk_read failed with error
> %d\n", err); @@ -821,12 +821,10 @@ grub_cryptodisk_write (grub_disk_t
> disk, grub_disk_addr_t sector, }
>  
>    /* Since ->write was called so disk.mod is loaded but be paranoid
> */
> -  
> +  sector = sector + dev->offset;
>    if (grub_disk_write_weak)
>      err = grub_disk_write_weak (dev->source_disk,
> -                             (sector << (disk->log_sector_size
> -                                         - GRUB_DISK_SECTOR_BITS))
> -                             + dev->offset,
> +                             grub_disk_from_native_sector (disk,
> sector), 0, size << disk->log_sector_size, tmp);
>    else
>      err = grub_error (GRUB_ERR_BUG, "disk.mod not loaded");
> diff --git a/include/grub/disk.h b/include/grub/disk.h
> index 316659fee..af9f886d3 100644
> --- a/include/grub/disk.h
> +++ b/include/grub/disk.h
> @@ -174,6 +174,13 @@ typedef struct grub_disk_memberlist
> *grub_disk_memberlist_t; /* Return value of grub_disk_get_size() in
> case disk size is unknown. */ #define GRUB_DISK_SIZE_UNKNOWN
> 0xffffffffffffffffULL 
> +/* Convert to grub native disk sized sector from disk sized sector */
> +static inline grub_disk_addr_t
> +grub_disk_from_native_sector (grub_disk_t disk, grub_disk_addr_t
> sector) +{
> +  return sector << (disk->log_sector_size - GRUB_DISK_SECTOR_BITS);
> +}
> +
>  /* This is called from the memory manager.  */
>  void grub_disk_cache_invalidate_all (void);
>