Re: [PATCH v2 07/10] cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt.

[Daniel Kiper]

Hi Glenn,

I will be looking at the patches on the grub-devel this week. I will
send you my comments then.

Daniel

On Sun, Oct 18, 2020 at 01:23:25PM -0500, Glenn Washburn wrote:
> Daniel, Patrick,
>
> Have either of you had a chance to review this patch?  Daniel, this is
> my interpretation of what you suggested earlier to remove constant
> literals in this message:
> https://lists.gnu.org/archive/html/grub-devel/2020-09/msg00069.html
>
> I'm going to make a v3 patchset with Patrick's suggested changes.
> Please reply to that patch.
>
> Glenn
>
> On Sat,  3 Oct 2020 17:55:31 -0500
> Glenn Washburn <development@efficientek.com> wrote:
>
> > This should improve readability of code by providing clues as to what
> > the value represents.
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  grub-core/disk/cryptodisk.c | 12 +++++++-----
> >  include/grub/types.h        |  3 +++
> >  2 files changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> > index 623f0f396..1a91c2d55 100644
> > --- a/grub-core/disk/cryptodisk.c
> > +++ b/grub-core/disk/cryptodisk.c
> > @@ -297,19 +297,21 @@ grub_cryptodisk_endecrypt (struct
> > grub_cryptodisk *dev, }
> >       break;
> >     case GRUB_CRYPTODISK_MODE_IV_BYTECOUNT64:
> > -     iv[1] = grub_cpu_to_le32 (sector >> (32 -
> > log_sector_size));
> > +     /* The IV is the 64 bit byte offset of the sector. */
> > +     iv[1] = grub_cpu_to_le32 (sector >> (GRUB_TYPE_BIT(iv[0])
> > +                                          - log_sector_size));
> >       iv[0] = grub_cpu_to_le32 ((sector << log_sector_size)
> > -                               & 0xFFFFFFFF);
> > +                               & GRUB_TYPE_MAX(iv[0]));
> >       break;
> >     case GRUB_CRYPTODISK_MODE_IV_BENBI:
> >       {
> >         grub_uint64_t num = (sector << dev->benbi_log) + 1;
> > -       iv[sz - 2] = grub_cpu_to_be32 (num >> 32);
> > -       iv[sz - 1] = grub_cpu_to_be32 (num & 0xFFFFFFFF);
> > +       iv[sz - 2] = grub_cpu_to_be32 (num >>
> > GRUB_TYPE_BIT(iv[0]));
> > +       iv[sz - 1] = grub_cpu_to_be32 (num &
> > GRUB_TYPE_MAX(iv[0])); }
> >       break;
> >     case GRUB_CRYPTODISK_MODE_IV_ESSIV:
> > -     iv[0] = grub_cpu_to_le32 (sector & 0xFFFFFFFF);
> > +     iv[0] = grub_cpu_to_le32 (sector & GRUB_TYPE_MAX(iv[0]));
> >       err = grub_crypto_ecb_encrypt (dev->essiv_cipher, iv, iv,
> >                                      dev->cipher->cipher->blocksize);
> >       if (err)
> > diff --git a/include/grub/types.h b/include/grub/types.h
> > index 035a4b528..8b4267ebd 100644
> > --- a/include/grub/types.h
> > +++ b/include/grub/types.h
> > @@ -319,4 +319,7 @@ static inline void grub_set_unaligned64 (void
> > *ptr, grub_uint64_t val)
> >  #define GRUB_CHAR_BIT 8
> >
> > +#define GRUB_TYPE_BIT(type) (sizeof(type) * GRUB_CHAR_BIT)
> > +#define GRUB_TYPE_MAX(type) ((2 * ((1ULL << (GRUB_TYPE_BIT(type) -
> > 1)) - 1)) + 1) +
> >  #endif /* ! GRUB_TYPES_HEADER */