Interrupt boot and display menu with `GRUB_TIMEOUT=0`?

2020-12-06 Thread Paul Menzel

Dear GRUB folks,


Setting the GRUB timeout to 0

`GRUB_TIMEOUT=0`

is it possible to interrupt the boot somehow for example by holding a 
key done? There are some posts on the WWW claiming it was once possible 
using the shift key [1], but it didn’t work in my tests with Debian 
bullseye/testing.



Kind regards,

Paul


[1]: https://centosfaq.org/centos/display-boot-menu-with-grub_timeout0/

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 01/17] disk: Rename grub_disk_get_size to grub_disk_native_sectors

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:30AM -0600, Glenn Washburn wrote:
> The function grub_disk_get_size is confusingly named because it actually
> returns a sector count where the sectors are sized in the grub native sector
> size. Rename to something more appropriate.
> 
> Suggested-by: Daniel Kiper 
> 
> Signed-off-by: Glenn Washburn 

Reviewed-by: Patrick Steinhardt 

> ---
>  grub-core/disk/diskfilter.c| 12 ++--
>  grub-core/disk/dmraid_nvidia.c |  2 +-
>  grub-core/disk/efi/efidisk.c   |  2 +-
>  grub-core/disk/geli.c  |  6 +++---
>  grub-core/disk/ldm.c   |  4 ++--
>  grub-core/disk/luks.c  |  2 +-
>  grub-core/disk/mdraid1x_linux.c|  2 +-
>  grub-core/disk/mdraid_linux.c  |  2 +-
>  grub-core/fs/cbfs.c| 16 
>  grub-core/fs/nilfs2.c  |  2 +-
>  grub-core/fs/zfs/zfs.c |  4 ++--
>  grub-core/kern/disk.c  |  2 +-
>  grub-core/kern/mips/arc/init.c |  2 +-
>  grub-core/normal/misc.c|  6 +++---
>  grub-core/osdep/windows/platform.c |  2 +-
>  include/grub/disk.h|  4 ++--
>  util/grub-install.c|  2 +-
>  util/grub-probe.c  |  2 +-
>  18 files changed, 37 insertions(+), 37 deletions(-)
> 
> diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
> index 86557f923..032011566 100644
> --- a/grub-core/disk/diskfilter.c
> +++ b/grub-core/disk/diskfilter.c
> @@ -148,7 +148,7 @@ scan_disk_partition_iter (grub_disk_t disk, 
> grub_partition_t p, void *data)
>   if (m->disk && m->disk->id == disk->id
>   && m->disk->dev->id == disk->dev->id
>   && m->part_start == grub_partition_get_start (disk->partition)
> - && m->part_size == grub_disk_get_size (disk))
> + && m->part_size == grub_disk_native_sectors (disk))
> return 0;
>  }
>  
> @@ -1190,13 +1190,13 @@ insert_array (grub_disk_t disk, const struct 
> grub_diskfilter_pv_id *id,
>  
>grub_dprintf ("diskfilter", "Inserting %s (+%lld,%lld) into %s (%s)\n", 
> disk->name,
>   (unsigned long long) grub_partition_get_start (disk->partition),
> - (unsigned long long) grub_disk_get_size (disk),
> + (unsigned long long) grub_disk_native_sectors (disk),
>   array->name, diskfilter->name);
>  #ifdef GRUB_UTIL
>grub_util_info ("Inserting %s (+%" GRUB_HOST_PRIuLONG_LONG ",%"
> GRUB_HOST_PRIuLONG_LONG ") into %s (%s)\n", disk->name,
> (unsigned long long) grub_partition_get_start 
> (disk->partition),
> -   (unsigned long long) grub_disk_get_size (disk),
> +   (unsigned long long) grub_disk_native_sectors (disk),
> array->name, diskfilter->name);
>array->driver = diskfilter;
>  #endif
> @@ -1210,7 +1210,7 @@ insert_array (grub_disk_t disk, const struct 
> grub_diskfilter_pv_id *id,
>   struct grub_diskfilter_lv *lv;
>   /* FIXME: Check whether the update time of the superblocks are
>  the same.  */
> - if (pv->disk && grub_disk_get_size (disk) >= pv->part_size)
> + if (pv->disk && grub_disk_native_sectors (disk) >= pv->part_size)
> return GRUB_ERR_NONE;
>   pv->disk = grub_disk_open (disk->name);
>   if (!pv->disk)
> @@ -1219,7 +1219,7 @@ insert_array (grub_disk_t disk, const struct 
> grub_diskfilter_pv_id *id,
>  raid device, we shouldn't change it.  */
>   pv->start_sector -= pv->part_start;
>   pv->part_start = grub_partition_get_start (disk->partition);
> - pv->part_size = grub_disk_get_size (disk);
> + pv->part_size = grub_disk_native_sectors (disk);
>  
>  #ifdef GRUB_UTIL
>   {
> @@ -1311,7 +1311,7 @@ grub_diskfilter_get_pv_from_disk (grub_disk_t disk,
>   if (pv->disk && pv->disk->id == disk->id
>   && pv->disk->dev->id == disk->dev->id
>   && pv->part_start == grub_partition_get_start (disk->partition)
> - && pv->part_size == grub_disk_get_size (disk))
> + && pv->part_size == grub_disk_native_sectors (disk))
> {
>   if (vg_out)
> *vg_out = vg;
> diff --git a/grub-core/disk/dmraid_nvidia.c b/grub-core/disk/dmraid_nvidia.c
> index 060279124..4d2fb04d1 100644
> --- a/grub-core/disk/dmraid_nvidia.c
> +++ b/grub-core/disk/dmraid_nvidia.c
> @@ -107,7 +107,7 @@ grub_dmraid_nv_detect (grub_disk_t disk,
>  /* Skip partition.  */
>  return NULL;
>  
> -  sector = grub_disk_get_size (disk);
> +  sector = grub_disk_native_sectors (disk);
>if (sector == GRUB_DISK_SIZE_UNKNOWN)
>  /* Not raid.  */
>  return NULL;
> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> index 9e20af70e..f077b5f55 100644
> --- a/grub-core/disk/efi/efidisk.c
> +++ b/grub-core/disk/efi/efidisk.c
> @@ -867,7 +867,7 @@ grub_efidisk_get_device_name (grub_efi_handle_t *handle)
>if (ctx.hd->partition_start == 0
> && (ctx.hd->partition_size <

Re: [PATCH v7 02/17] misc: Add parentheses around ALIGN_UP and ALIGN_DOWN arguments

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:31AM -0600, Glenn Washburn wrote:
> This ensures that expected order of operations is preserved when arguments
> are expressions.
> 
> Signed-off-by: Glenn Washburn 

Reviewed-by: Patrick Steinhardt 

> ---
>  include/grub/misc.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/grub/misc.h b/include/grub/misc.h
> index b7ca6dd58..780a34e90 100644
> --- a/include/grub/misc.h
> +++ b/include/grub/misc.h
> @@ -28,10 +28,10 @@
>  #include 
>  
>  #define ALIGN_UP(addr, align) \
> - ((addr + (typeof (addr)) align - 1) & ~((typeof (addr)) align - 1))
> + (((addr) + (typeof (addr)) (align) - 1) & ~((typeof (addr)) (align) - 
> 1))
>  #define ALIGN_UP_OVERHEAD(addr, align) ((-(addr)) & ((typeof (addr)) (align) 
> - 1))
>  #define ALIGN_DOWN(addr, align) \
> - ((addr) & ~((typeof (addr)) align - 1))
> + ((addr) & ~((typeof (addr)) (align) - 1))
>  #define ARRAY_SIZE(array) (sizeof (array) / sizeof (array[0]))
>  #define COMPILE_TIME_ASSERT(cond) switch (0) { case 1: case !(cond): ; }
>  
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 03/17] luks2: Remove unused argument in grub_error

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:32AM -0600, Glenn Washburn wrote:
> Reviewed-by: Daniel Kiper 
> Signed-off-by: Glenn Washburn 

Reviewed-by: Patrick Steinhardt 

> ---
>  grub-core/disk/luks2.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index d96764a02..bdb90e4b6 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -200,7 +200,7 @@ luks2_parse_segment (grub_luks2_segment_t *out, const 
> grub_json_t *segment)
>grub_json_getstring (&out->size, segment, "size") ||
>grub_json_getstring (&out->encryption, segment, "encryption") ||
>grub_json_getint64 (&out->sector_size, segment, "sector_size"))
> -return grub_error (GRUB_ERR_BAD_ARGUMENT, "Missing segment parameters", 
> type);
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, "Missing segment parameters");
>  
>return GRUB_ERR_NONE;
>  }
> @@ -228,7 +228,7 @@ luks2_parse_digest (grub_luks2_digest_t *out, const 
> grub_json_t *digest)
>  
>if (grub_json_getsize (&size, &segments))
>  return grub_error (GRUB_ERR_BAD_ARGUMENT,
> -"Digest references no segments", type);
> +"Digest references no segments");
>  
>for (i = 0; i < size; i++)
>  {
> @@ -240,7 +240,7 @@ luks2_parse_digest (grub_luks2_digest_t *out, const 
> grub_json_t *digest)
>  
>if (grub_json_getsize (&size, &keyslots))
>  return grub_error (GRUB_ERR_BAD_ARGUMENT,
> -"Digest references no keyslots", type);
> +"Digest references no keyslots");
>  
>for (i = 0; i < size; i++)
>  {
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 04/17] luks2: Make sure all fields of output argument in luks2_parse_digest() are written to

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:33AM -0600, Glenn Washburn wrote:
> We should assume that the output argument "out" is uninitialized and could
> have random data. So, make sure to initialize the segments and keyslots bit
> fields because potentially not all bits of those fields are written to.
> Otherwise, the digest could say it belongs to keyslots and segments that it
> does not.
> 
> Signed-off-by: Glenn Washburn 

Makes sense.

Signed-off-by: Patrick Steinhardt 

> ---
>  grub-core/disk/luks2.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index bdb90e4b6..eadd529e9 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -230,6 +230,7 @@ luks2_parse_digest (grub_luks2_digest_t *out, const 
> grub_json_t *digest)
>  return grub_error (GRUB_ERR_BAD_ARGUMENT,
>  "Digest references no segments");
>  
> +  out->segments = 0;
>for (i = 0; i < size; i++)
>  {
>if (grub_json_getchild (&o, &segments, i) ||
> @@ -242,6 +243,7 @@ luks2_parse_digest (grub_luks2_digest_t *out, const 
> grub_json_t *digest)
>  return grub_error (GRUB_ERR_BAD_ARGUMENT,
>  "Digest references no keyslots");
>  
> +  out->keyslots = 0;
>for (i = 0; i < size; i++)
>  {
>if (grub_json_getchild (&o, &keyslots, i) ||
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 05/17] luks2: Add json_slot_key member to struct grub_luks2_keyslot/segment/digest

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:34AM -0600, Glenn Washburn wrote:
> This allows code using these structs to know the named key associated with
> these json data structures. In the future we can use these to provide better
> error messages to the user.
> 
> Get rid of idx variable in luks2_get_keyslot() which was overloaded to be
> used for both keyslot and segment slot keys.
> 
> Signed-off-by: Glenn Washburn 

Personally, I'd have named them `json_slot_idx`. But you've already done
so much work on improving the code that I don't want this to be the
reason to not give an SOB, especially considering that it's a strict
improvement anyway. So:

Signed-off-by: Patrick Steinhardt 

> ---
>  grub-core/disk/luks2.c | 14 +-
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index eadd529e9..437c1da07 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -65,6 +65,8 @@ typedef struct grub_luks2_header grub_luks2_header_t;
>  
>  struct grub_luks2_keyslot
>  {
> +  /* The integer key to the associative array of keyslots */
> +  grub_uint64_t json_slot_key;
>grub_int64_t key_size;
>grub_int64_t priority;
>struct
> @@ -103,6 +105,7 @@ typedef struct grub_luks2_keyslot grub_luks2_keyslot_t;
>  
>  struct grub_luks2_segment
>  {
> +  grub_uint64_t json_slot_key;
>grub_uint64_t offset;
>const char *size;
>const char *encryption;
> @@ -112,6 +115,7 @@ typedef struct grub_luks2_segment grub_luks2_segment_t;
>  
>  struct grub_luks2_digest
>  {
> +  grub_uint64_t json_slot_key;
>/* Both keyslots and segments are interpreted as bitfields here */
>grub_uint64_t  keyslots;
>grub_uint64_t  segments;
> @@ -261,12 +265,11 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>  {
>grub_json_t keyslots, keyslot, digests, digest, segments, segment;
>grub_size_t i, size;
> -  grub_uint64_t idx;
>  
>/* Get nth keyslot */
>if (grub_json_getvalue (&keyslots, root, "keyslots") ||
>grub_json_getchild (&keyslot, &keyslots, keyslot_idx) ||
> -  grub_json_getuint64 (&idx, &keyslot, NULL) ||
> +  grub_json_getuint64 (&k->json_slot_key, &keyslot, NULL) ||
>grub_json_getchild (&keyslot, &keyslot, 0) ||
>luks2_parse_keyslot (k, &keyslot))
>  return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse keyslot 
> %"PRIuGRUB_SIZE, keyslot_idx);
> @@ -278,11 +281,12 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>for (i = 0; i < size; i++)
>  {
>if (grub_json_getchild (&digest, &digests, i) ||
> +   grub_json_getuint64 (&d->json_slot_key, &digest, NULL) ||
>grub_json_getchild (&digest, &digest, 0) ||
>luks2_parse_digest (d, &digest))
>   return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest 
> %"PRIuGRUB_SIZE, i);
>  
> -  if ((d->keyslots & (1 << idx)))
> +  if ((d->keyslots & (1 << k->json_slot_key)))
>   break;
>  }
>if (i == size)
> @@ -295,12 +299,12 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>for (i = 0; i < size; i++)
>  {
>if (grub_json_getchild (&segment, &segments, i) ||
> -   grub_json_getuint64 (&idx, &segment, NULL) ||
> +   grub_json_getuint64 (&s->json_slot_key, &segment, NULL) ||
> grub_json_getchild (&segment, &segment, 0) ||
>luks2_parse_segment (s, &segment))
>   return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment 
> %"PRIuGRUB_SIZE, i);
>  
> -  if ((d->segments & (1 << idx)))
> +  if ((d->segments & (1 << s->json_slot_key)))
>   break;
>  }
>if (i == size)
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 08/17] cryptodisk: Add macro GRUB_TYPE_BITS() to replace some literals

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:37AM -0600, Glenn Washburn wrote:
> The new macro GRUB_TYPE_BITS(type) returns the number of bits allocated for
> type.
> 
> Signed-off-by: Glenn Washburn 

Reviewed-by: Patrick Steinhardt 

> ---
>  grub-core/disk/cryptodisk.c | 7 ---
>  include/grub/types.h| 2 ++
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 473c93976..0e955a020 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -281,20 +281,21 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
> }
> break;
>   case GRUB_CRYPTODISK_MODE_IV_PLAIN64:
> -   iv[1] = grub_cpu_to_le32 (sector >> 32);
> +   iv[1] = grub_cpu_to_le32 (sector >> GRUB_TYPE_BITS (iv[0]));
> /* FALLTHROUGH */
>   case GRUB_CRYPTODISK_MODE_IV_PLAIN:
> iv[0] = grub_cpu_to_le32 (sector & 0x);
> break;
>   case GRUB_CRYPTODISK_MODE_IV_BYTECOUNT64:
> -   iv[1] = grub_cpu_to_le32 (sector >> (32 - dev->log_sector_size));
> +   iv[1] = grub_cpu_to_le32 (sector >> (GRUB_TYPE_BITS (iv[1])
> +- dev->log_sector_size));
> iv[0] = grub_cpu_to_le32 ((sector << dev->log_sector_size)
>   & 0x);
> 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 - 2] = grub_cpu_to_be32 (num >> GRUB_TYPE_BITS (iv[0]));
>   iv[sz - 1] = grub_cpu_to_be32 (num & 0x);
> }
> break;
> diff --git a/include/grub/types.h b/include/grub/types.h
> index f22055f98..9989e3a16 100644
> --- a/include/grub/types.h
> +++ b/include/grub/types.h
> @@ -80,6 +80,8 @@
>  # define GRUB_CHAR_BIT   __CHAR_BIT__
>  #endif
>  
> +#define GRUB_TYPE_BITS(type) (sizeof(type) * GRUB_CHAR_BIT)
> +
>  /* Define various wide integers.  */
>  typedef signed char  grub_int8_t;
>  typedef shortgrub_int16_t;
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 06/17] luks2: Use more intuitive slot key instead of index in user messages

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:35AM -0600, Glenn Washburn wrote:
> Use the slot key name in the json array rather than the 0 based index in the
> json array for keyslots, segments, and digests. This is less confusing for
> the end user. For example, say you have a LUKS2 device with a key in slot 1
> and slot 4. When using the password for slot 4 to unlock the device, the
> messages using the index of the keyslot will mention keyslot 1 (its a
> zero-based index). Furthermore, with this change the keyslot number will
> align with the number used to reference the keyslot when using the
> --key-slot argument to cryptsetup.
> 
> Signed-off-by: Glenn Washburn 

Reviewed-by: Patrick Steinhardt 

Oops. I just realized I gave SOBs on the previous patches. Naturally,
these should've been Reviewed-by's :(

> ---
>  grub-core/disk/luks2.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index 437c1da07..ea1065bcf 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -284,13 +284,13 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
> grub_json_getuint64 (&d->json_slot_key, &digest, NULL) ||
>grub_json_getchild (&digest, &digest, 0) ||
>luks2_parse_digest (d, &digest))
> - return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest 
> %"PRIuGRUB_SIZE, i);
> + return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest index 
> %"PRIuGRUB_SIZE, i);
>  
>if ((d->keyslots & (1 << k->json_slot_key)))
>   break;
>  }
>if (i == size)
> -  return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot 
> %"PRIuGRUB_SIZE, keyslot_idx);
> +  return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot 
> \"%"PRIuGRUB_UINT64_T"\"", k->slot_key);
>  
>/* Get segment that matches the digest. */
>if (grub_json_getvalue (&segments, root, "segments") ||
> @@ -308,7 +308,7 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>   break;
>  }
>if (i == size)
> -return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest 
> %"PRIuGRUB_SIZE);
> +return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest 
> \"%"PRIuGRUB_UINT64_T"\"", d->slot_key);
>  
>return GRUB_ERR_NONE;
>  }
> @@ -604,11 +604,11 @@ luks2_recover_key (grub_disk_t source,
>  
>if (keyslot.priority == 0)
>   {
> -   grub_dprintf ("luks2", "Ignoring keyslot %"PRIuGRUB_SIZE" due to 
> priority\n", i);
> +   grub_dprintf ("luks2", "Ignoring keyslot \"%"PRIuGRUB_UINT64_T"\" due 
> to priority\n", keyslot.slot_key);
> continue;
>  }
>  
> -  grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_SIZE"\n", i);
> +  grub_dprintf ("luks2", "Trying keyslot \"%"PRIuGRUB_UINT64_T"\"\n", 
> keyslot.slot_key);
>  
>/* Set up disk according to keyslot's segment. */
>crypt->offset_sectors = grub_divmod64 (segment.offset, 
> segment.sector_size, NULL);
> @@ -623,16 +623,16 @@ luks2_recover_key (grub_disk_t source,
>  (const grub_uint8_t *) passphrase, grub_strlen 
> (passphrase));
>if (ret)
>   {
> -   grub_dprintf ("luks2", "Decryption with keyslot %"PRIuGRUB_SIZE" 
> failed: %s\n",
> - i, grub_errmsg);
> +   grub_dprintf ("luks2", "Decryption with keyslot 
> \"%"PRIuGRUB_UINT64_T"\" failed: %s\n",
> + keyslot.slot_key, grub_errmsg);
> continue;
>   }
>  
>ret = luks2_verify_key (&digest, candidate_key, keyslot.key_size);
>if (ret)
>   {
> -   grub_dprintf ("luks2", "Could not open keyslot %"PRIuGRUB_SIZE": 
> %s\n",
> - i, grub_errmsg);
> +   grub_dprintf ("luks2", "Could not open keyslot 
> \"%"PRIuGRUB_UINT64_T"\": %s\n",
> + keyslot.slot_key, grub_errmsg);
> continue;
>   }
>  
> @@ -640,7 +640,7 @@ luks2_recover_key (grub_disk_t source,
> * TRANSLATORS: It's a cryptographic key slot: one element of an array
> * where each element is either empty or holds a key.
> */
> -  grub_printf_ (N_("Slot %"PRIuGRUB_SIZE" opened\n"), i);
> +  grub_printf_ (N_("Slot \"%"PRIuGRUB_UINT64_T"\" opened\n"), 
> keyslot.slot_key);
>  
>candidate_key_len = keyslot.key_size;
>break;
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 09/17] cryptodisk: Add macros GRUB_TYPE_U_MAX/MIN(type) to replace literals

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:38AM -0600, Glenn Washburn wrote:
> Add GRUB_TYPE_U_MAX/MIN(type) macros to get the max/min values for an
> unsigned number with size of type.
> 
> Signed-off-by: Glenn Washburn 
> ---
>  grub-core/disk/cryptodisk.c | 8 
>  include/grub/types.h| 7 +++
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 0e955a020..5aa0c4720 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -284,23 +284,23 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
> iv[1] = grub_cpu_to_le32 (sector >> GRUB_TYPE_BITS (iv[0]));
> /* FALLTHROUGH */
>   case GRUB_CRYPTODISK_MODE_IV_PLAIN:
> -   iv[0] = grub_cpu_to_le32 (sector & 0x);
> +   iv[0] = grub_cpu_to_le32 (sector & GRUB_TYPE_U_MAX (iv[0]));
> break;
>   case GRUB_CRYPTODISK_MODE_IV_BYTECOUNT64:
> iv[1] = grub_cpu_to_le32 (sector >> (GRUB_TYPE_BITS (iv[1])
>  - dev->log_sector_size));
> iv[0] = grub_cpu_to_le32 ((sector << dev->log_sector_size)
> - & 0x);
> + & GRUB_TYPE_U_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 >> GRUB_TYPE_BITS (iv[0]));
> - iv[sz - 1] = grub_cpu_to_be32 (num & 0x);
> + iv[sz - 1] = grub_cpu_to_be32 (num & GRUB_TYPE_U_MAX (iv[0]));
> }
> break;
>   case GRUB_CRYPTODISK_MODE_IV_ESSIV:
> -   iv[0] = grub_cpu_to_le32 (sector & 0x);
> +   iv[0] = grub_cpu_to_le32 (sector & GRUB_TYPE_U_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 9989e3a16..0542011cc 100644
> --- a/include/grub/types.h
> +++ b/include/grub/types.h
> @@ -161,6 +161,13 @@ typedef grub_int32_t grub_ssize_t;
>  #endif
>  # define GRUB_LONG_MIN (-GRUB_LONG_MAX - 1)
>  
> +/*
> +  Cast to unsigned long long so that the "return value" is always a 
> consistent
> +  type and to ensure an unsigned value regardless of type parameter.
> + */
> +#define GRUB_TYPE_U_MAX(type) ((unsigned long long)((typeof (type))(~0)))
> +#define GRUB_TYPE_U_MIN(type) 0ULL
> +

Nit: multi-line comments do use the following format:

/*
 * Foo bar
 * bar foo
 /

Other than that:

Reviewed-by: Patrick Steinhardt 

>  typedef grub_uint64_t grub_properly_aligned_t;
>  
>  #define GRUB_PROPERLY_ALIGNED_ARRAY(name, size) grub_properly_aligned_t 
> name[((size) + sizeof (grub_properly_aligned_t) - 1) / sizeof 
> (grub_properly_aligned_t)]
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 10/17] luks2: grub_cryptodisk_t->total_sectors is the max number of device native sectors

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:39AM -0600, Glenn Washburn wrote:
> We need to convert the sectors from the size of the underlying device to the
> cryptodisk sector size; segment.size is in bytes which need to be converted
> to cryptodisk sectors as well.
> 
> Also, removed an empty statement.
> 
> Reviewed-by: Daniel Kiper 
> Signed-off-by: Glenn Washburn 

Reviewed-by: Patrick Steinhardt 

> ---
>  grub-core/disk/luks2.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index 4be40b22b..bdf40768b 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -429,7 +429,7 @@ luks2_decrypt_key (grub_uint8_t *out_key,
>grub_uint8_t salt[GRUB_CRYPTODISK_MAX_KEYLEN];
>grub_uint8_t *split_key = NULL;
>grub_size_t saltlen = sizeof (salt);
> -  char cipher[32], *p;;
> +  char cipher[32], *p;
>const gcry_md_spec_t *hash;
>gcry_err_code_t gcry_ret;
>grub_err_t ret;
> @@ -615,9 +615,10 @@ luks2_recover_key (grub_disk_t source,
>crypt->log_sector_size = sizeof (unsigned int) * 8
>   - __builtin_clz ((unsigned int) segment.sector_size) - 1;
>if (grub_strcmp (segment.size, "dynamic") == 0)
> - crypt->total_sectors = grub_disk_get_size (source) - 
> crypt->offset_sectors;
> + crypt->total_sectors = (grub_disk_get_size (source) >> 
> (crypt->log_sector_size - source->log_sector_size))
> +- crypt->offset_sectors;
>else
> - crypt->total_sectors = grub_strtoull (segment.size, NULL, 10);
> + crypt->total_sectors = grub_strtoull (segment.size, NULL, 10) >> 
> crypt->log_sector_size;
>  
>ret = luks2_decrypt_key (candidate_key, source, crypt, &keyslot,
>  (const grub_uint8_t *) passphrase, grub_strlen 
> (passphrase));
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 07/17] luks2: Add string "index" to user strings using a json index.

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:36AM -0600, Glenn Washburn wrote:
> This allows error messages to be more easily distinguishable between indexes
> and slot keys. The former include the string "index" in the error/debug
> string, and the later are surrounded in quotes.
> 
> Signed-off-by: Glenn Washburn 

Reviewed-by: Patrick Steinhardt 

> ---
>  grub-core/disk/luks2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index ea1065bcf..4be40b22b 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -272,7 +272,7 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>grub_json_getuint64 (&k->json_slot_key, &keyslot, NULL) ||
>grub_json_getchild (&keyslot, &keyslot, 0) ||
>luks2_parse_keyslot (k, &keyslot))
> -return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse keyslot 
> %"PRIuGRUB_SIZE, keyslot_idx);
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse keyslot index 
> %"PRIuGRUB_SIZE, keyslot_idx);
>  
>/* Get digest that matches the keyslot. */
>if (grub_json_getvalue (&digests, root, "digests") ||
> @@ -302,7 +302,7 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
> grub_json_getuint64 (&s->json_slot_key, &segment, NULL) ||
> grub_json_getchild (&segment, &segment, 0) ||
>luks2_parse_segment (s, &segment))
> - return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment 
> %"PRIuGRUB_SIZE, i);
> + return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment 
> index %"PRIuGRUB_SIZE, i);
>  
>if ((d->segments & (1 << s->json_slot_key)))
>   break;
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 14/17] whitespace: convert 8 spaces to tabs

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:43AM -0600, Glenn Washburn wrote:
> Reviewed-by: Daniel Kiper 
> Signed-off-by: Glenn Washburn 

Reviewed-by: Patrick Steinhardt 

> ---
>  grub-core/disk/luks2.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index 499c9330b..2335ded77 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -282,8 +282,8 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>  {
>if (grub_json_getchild (&digest, &digests, i) ||
> grub_json_getuint64 (&d->json_slot_key, &digest, NULL) ||
> -  grub_json_getchild (&digest, &digest, 0) ||
> -  luks2_parse_digest (d, &digest))
> +   grub_json_getchild (&digest, &digest, 0) ||
> +   luks2_parse_digest (d, &digest))
>   return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest index 
> %"PRIuGRUB_SIZE, i);
>  
>if ((d->keyslots & (1 << k->json_slot_key)))
> @@ -301,7 +301,7 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>if (grub_json_getchild (&segment, &segments, i) ||
> grub_json_getuint64 (&s->json_slot_key, &segment, NULL) ||
> grub_json_getchild (&segment, &segment, 0) ||
> -  luks2_parse_segment (s, &segment))
> +   luks2_parse_segment (s, &segment))
>   return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment 
> index %"PRIuGRUB_SIZE, i);
>  
>if ((d->segments & (1 << s->json_slot_key)))
> @@ -625,7 +625,7 @@ luks2_recover_key (grub_disk_t source,
>   {
> grub_dprintf ("luks2", "Ignoring keyslot \"%"PRIuGRUB_UINT64_T"\" due 
> to priority\n", keyslot.json_slot_key);
> continue;
> -}
> + }
>  
>grub_dprintf ("luks2", "Trying keyslot \"%"PRIuGRUB_UINT64_T"\"\n", 
> keyslot.json_slot_key);
>  
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 16/17] misc: Add grub_log2ull macro for calculating log base 2 of 64-bit integers

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:45AM -0600, Glenn Washburn wrote:
> Reviewed-by: Daniel Kiper 
> Signed-off-by: Glenn Washburn 

Reviewed-by: Patrick Steinhardt 

> ---
>  include/grub/misc.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/grub/misc.h b/include/grub/misc.h
> index 780a34e90..73a514eb1 100644
> --- a/include/grub/misc.h
> +++ b/include/grub/misc.h
> @@ -482,4 +482,7 @@ void EXPORT_FUNC(grub_real_boot_time) (const char *file,
>  #define grub_max(a, b) (((a) > (b)) ? (a) : (b))
>  #define grub_min(a, b) (((a) < (b)) ? (a) : (b))
>  
> +#define grub_log2ull(n) (GRUB_TYPE_BITS (grub_uint64_t) \
> + - __builtin_clzll (n) - 1)
> +
>  #endif /* ! GRUB_MISC_HEADER */
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 00/17] Cryptodisk fixes for v2.06 redux

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:29AM -0600, Glenn Washburn wrote:
> This patch series is an update to reflect changes suggested in v6. Of note,
> there are a few new patches: 01, 02, and 04.
> 
>   01: Daniel suggested to renae grub_disk_get_size to grub_disk_native_sectors
>   02: Make ALIGN_UP and ALIGN_DOWN safer
>   04: Make luks2_parse_digest() safer

The patches I reviewed look good to me. I gotta jump, so I'll hopefully
get to the remaining patches this evening.

Also note that I accidentally posted SOBs for the first few ones. These
should've been Reviewed-by's.

Thanks for all your work!

Patrick

> Glenn
> 
> Glenn Washburn (17):
>   disk: Rename grub_disk_get_size to grub_disk_native_sectors
>   misc: Add parentheses around ALIGN_UP and ALIGN_DOWN arguments
>   luks2: Remove unused argument in grub_error
>   luks2: Make sure all fields of output argument in luks2_parse_digest()
> are written to
>   luks2: Add json_slot_key member to struct
> grub_luks2_keyslot/segment/digest
>   luks2: Use more intuitive slot key instead of index in user messages
>   luks2: Add string "index" to user strings using a json index.
>   cryptodisk: Add macro GRUB_TYPE_BITS() to replace some literals
>   cryptodisk: Add macros GRUB_TYPE_U_MAX/MIN(type) to replace literals
>   luks2: grub_cryptodisk_t->total_sectors is the max number of device
> native sectors
>   cryptodisk: Properly handle non-512 byte sized sectors
>   luks2: Better error handling when setting up the cryptodisk
>   luks2: Error check segment.sector_size
>   whitespace: convert 8 spaces to tabs
>   mips: Enable __clzdi2()
>   misc: Add grub_log2ull macro for calculating log base 2 of 64-bit
> integers
>   luks2: Use grub_log2ull to calculate log_sector_size and improve
> readability
> 
>  grub-core/disk/cryptodisk.c|  64 +++-
>  grub-core/disk/diskfilter.c|  12 +--
>  grub-core/disk/dmraid_nvidia.c |   2 +-
>  grub-core/disk/efi/efidisk.c   |   2 +-
>  grub-core/disk/geli.c  |   6 +-
>  grub-core/disk/ldm.c   |   4 +-
>  grub-core/disk/luks.c  |   7 +-
>  grub-core/disk/luks2.c | 160 +++--
>  grub-core/disk/mdraid1x_linux.c|   2 +-
>  grub-core/disk/mdraid_linux.c  |   2 +-
>  grub-core/fs/cbfs.c|  16 +--
>  grub-core/fs/nilfs2.c  |   2 +-
>  grub-core/fs/zfs/zfs.c |   4 +-
>  grub-core/kern/compiler-rt.c   |   2 +-
>  grub-core/kern/disk.c  |   2 +-
>  grub-core/kern/mips/arc/init.c |   2 +-
>  grub-core/normal/misc.c|   6 +-
>  grub-core/osdep/windows/platform.c |   2 +-
>  include/grub/compiler-rt.h |   2 +-
>  include/grub/cryptodisk.h  |   8 +-
>  include/grub/disk.h|  21 +++-
>  include/grub/misc.h|   7 +-
>  include/grub/types.h   |   9 ++
>  util/grub-install.c|   2 +-
>  util/grub-probe.c  |   2 +-
>  25 files changed, 249 insertions(+), 99 deletions(-)
> 
> Range-diff against v6:
>  -:  - >  1:  7e79d6fb1 disk: Rename grub_disk_get_size to 
> grub_disk_native_sectors
>  -:  - >  2:  77f9671d5 misc: Add parentheses around ALIGN_UP and 
> ALIGN_DOWN arguments
>  3:  8527be145 !  3:  d1a36aa79 luks2: Remove unused argument in grub_error
> @@ Metadata
>   ## Commit message ##
>  luks2: Remove unused argument in grub_error
>  
> +Reviewed-by: Daniel Kiper 
> +
>   ## grub-core/disk/luks2.c ##
>  @@ grub-core/disk/luks2.c: luks2_parse_segment (grub_luks2_segment_t 
> *out, const grub_json_t *segment)
> grub_json_getstring (&out->size, segment, "size") ||
>  -:  - >  4:  dab43e033 luks2: Make sure all fields of output 
> argument in luks2_parse_digest() are written to
>  1:  6262aefe9 !  5:  35f47644c luks2: Add slot_key member to struct 
> grub_luks2_keyslot/segment/digest
> @@ Metadata
>  Author: Glenn Washburn 
>  
>   ## Commit message ##
> -luks2: Add slot_key member to struct 
> grub_luks2_keyslot/segment/digest
> +luks2: Add json_slot_key member to struct 
> grub_luks2_keyslot/segment/digest
>  
>  This allows code using these structs to know the named key 
> associated with
>  these json data structures. In the future we can use these to 
> provide better
>  error messages to the user.
>  
> -Get rid of idx variable in luks2_get_keyslot which was overloaded to 
> be used
> -for both keyslot and segment slot keys.
> +Get rid of idx variable in luks2_get_keyslot() which was overloaded 
> to be
> +used for both keyslot and segment slot keys.
>  
>   ## grub-core/disk/luks2.c ##
>  @@ grub-core/disk/luks2.c: typedef struct grub_luks2_header 
> grub_luks2_header_t;
>   
>   struct grub_luks2_keyslot
>   {
> -+  grub_uint64_t slot_key;
> ++  /* The integer 

Re: [PATCH v7 11/17] cryptodisk: Properly handle non-512 byte sized sectors

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:40AM -0600, Glenn Washburn wrote:
> By default, dm-crypt internally uses an IV that corresponds to 512-byte
> sectors, even when a larger sector size is specified. What this means is
> that when using a larger sector size, the IV is incremented every sector.
> However, the amount the IV is incremented is the number of 512 byte blocks
> in a sector (ie 8 for 4K sectors). Confusingly the IV does not corespond to
> the number of, for example, 4K sectors. So each 512 byte cipher block in a
> sector will be encrypted with the same IV and the IV will be incremented
> afterwards by the number of 512 byte cipher blocks in the sector.
> 
> There are some encryption utilities which do it the intuitive way and have
> the IV equal to the sector number regardless of sector size (ie. the fifth
> sector would have an IV of 4 for each cipher block). And this is supported
> by dm-crypt with the iv_large_sectors option and also cryptsetup as of 2.3.3
> with the --iv-large-sectors, though not with LUKS headers (only with --type
> plain). However, support for this has not been included as grub does not
> support plain devices right now.
> 
> One gotcha here is that the encrypted split keys are encrypted with a hard-
> coded 512-byte sector size. So even if your data is encrypted with 4K sector
> sizes, the split key encrypted area must be decrypted with a block size of
> 512 (ie the IV increments every 512 bytes). This made these changes less
> aestetically pleasing than desired.
> 
> Reviewed-by: Daniel Kiper 
> Signed-off-by: Glenn Washburn 

Reviewed-by: Patrick Steinhardt 

> ---
>  grub-core/disk/cryptodisk.c | 55 +++--
>  grub-core/disk/luks.c   |  5 ++--
>  grub-core/disk/luks2.c  |  7 -
>  include/grub/cryptodisk.h   |  8 +-
>  4 files changed, 50 insertions(+), 25 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 5aa0c4720..b62835acc 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -224,7 +224,8 @@ lrw_xor (const struct lrw_sector *sec,
>  static gcry_err_code_t
>  grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
>  grub_uint8_t * data, grub_size_t len,
> -grub_disk_addr_t sector, int do_encrypt)
> +grub_disk_addr_t sector, grub_size_t log_sector_size,
> +int do_encrypt)
>  {
>grub_size_t i;
>gcry_err_code_t err;
> @@ -237,7 +238,7 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
>  return (do_encrypt ? grub_crypto_ecb_encrypt (dev->cipher, data, data, 
> len)
>   : grub_crypto_ecb_decrypt (dev->cipher, data, data, len));
>  
> -  for (i = 0; i < len; i += (1U << dev->log_sector_size))
> +  for (i = 0; i < len; i += (1U << log_sector_size))
>  {
>grub_size_t sz = ((dev->cipher->cipher->blocksize
>+ sizeof (grub_uint32_t) - 1)
> @@ -270,7 +271,7 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
>   if (!ctx)
> return GPG_ERR_OUT_OF_MEMORY;
>  
> - tmp = grub_cpu_to_le64 (sector << dev->log_sector_size);
> + tmp = grub_cpu_to_le64 (sector << log_sector_size);
>   dev->iv_hash->init (ctx);
>   dev->iv_hash->write (ctx, dev->iv_prefix, dev->iv_prefix_len);
>   dev->iv_hash->write (ctx, &tmp, sizeof (tmp));
> @@ -281,15 +282,27 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
> }
> break;
>   case GRUB_CRYPTODISK_MODE_IV_PLAIN64:
> -   iv[1] = grub_cpu_to_le32 (sector >> GRUB_TYPE_BITS (iv[0]));
> -   /* FALLTHROUGH */
>   case GRUB_CRYPTODISK_MODE_IV_PLAIN:
> -   iv[0] = grub_cpu_to_le32 (sector & GRUB_TYPE_U_MAX (iv[0]));
> +   /*
> +* The IV is a 32 or 64 bit value of the dm-crypt native sector
> +* number. If using 32 bit IV mode, zero out the most significant
> +* 32 bits.
> +*/
> +   {
> + grub_uint64_t iv64;
> +
> + iv64 = grub_cpu_to_le64 (sector << (log_sector_size
> +  - 
> GRUB_CRYPTODISK_IV_LOG_SIZE));
> + grub_set_unaligned64 (iv, iv64);
> + if (dev->mode_iv == GRUB_CRYPTODISK_MODE_IV_PLAIN)
> +   iv[1] = 0;
> +   }
> break;
>   case GRUB_CRYPTODISK_MODE_IV_BYTECOUNT64:
> +   /* The IV is the 64 bit byte offset of the sector. */
> iv[1] = grub_cpu_to_le32 (sector >> (GRUB_TYPE_BITS (iv[1])
> -- dev->log_sector_size));
> -   iv[0] = grub_cpu_to_le32 ((sector << dev->log_sector_size)
> +- log_sector_size));
> +   iv[0] = grub_cpu_to_le32 ((sector << log_sector_size)
>   & GRUB_TYPE_U_MAX (iv[0]));
> break;
>   case GRUB_CRYPTODISK_MODE_IV_BENBI:
> @@ -312,10 +325,10 @@ grub_cryptodisk_ende

Re: [PATCH v7 12/17] luks2: Better error handling when setting up the cryptodisk

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:41AM -0600, Glenn Washburn wrote:
> First, check to make sure that source disk has a known size. If not, print
> debug message and return error. There are 4 cases where
> GRUB_DISK_SIZE_UNKNOWN is set (biosdisk, obdisk, ofdisk, and uboot), and in
> all those cases processing continues. So this is probably a bit
> conservative. However, 3 of the cases seem pathological, and the other,
> biosdisk, happens when booting from a cd. Since I doubt booting from a LUKS2
> volume on a cd is a big use case, we'll error until someone complains.
> 
> Do some sanity checking on data coming from the luks header. If segment.size
> is "dynamic",

Nit: there's something missing here.

> Check for errors from grub_strtoull when converting segment size from
> string. If a GRUB_ERR_BAD_NUMBER error was returned, then the string was
> not a valid parsable number, so skip the key. If GRUB_ERR_OUT_OF_RANGE was
> returned, then there was an overflow in converting to a 64-bit unsigned
> integer. So this could be a very large disk (perhaps large raid array).
> In this case, we want to continue to try to use this key so long as the
> source disk's size is greater than this segment size. Otherwise, this is
> an invalid segment, and continue on to the next key.
> 
> Signed-off-by: Glenn Washburn 
> ---
>  grub-core/disk/luks2.c | 98 +-
>  include/grub/disk.h| 17 
>  2 files changed, 105 insertions(+), 10 deletions(-)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index de2e70796..1bb3a333d 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -290,7 +290,7 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>   break;
>  }
>if (i == size)
> -  return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot 
> \"%"PRIuGRUB_UINT64_T"\"", k->slot_key);
> +  return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot 
> \"%"PRIuGRUB_UINT64_T"\"", k->json_slot_key);
>  
>/* Get segment that matches the digest. */
>if (grub_json_getvalue (&segments, root, "segments") ||
> @@ -308,7 +308,7 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>   break;
>  }
>if (i == size)
> -return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest 
> \"%"PRIuGRUB_UINT64_T"\"", d->slot_key);
> +return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest 
> \"%"PRIuGRUB_UINT64_T"\"", d->json_slot_key);
>  
>return GRUB_ERR_NONE;
>  }
> @@ -600,37 +600,115 @@ luks2_recover_key (grub_disk_t source,
>goto err;
>  }
>  
> +  if (source->total_sectors == GRUB_DISK_SIZE_UNKNOWN)
> +{
> +  /* FIXME: Allow use of source disk, and maybe cause errors in read. */
> +  grub_dprintf ("luks2", "Source disk %s has an unknown size, "
> +  "conservatively returning error\n", source->name);
> +  ret = grub_error (GRUB_ERR_BUG, "Unknown size of luks2 source device");
> +  goto err;
> +}
> +
>/* Try all keyslot */
>for (i = 0; i < size; i++)
>  {
> +  typeof(source->total_sectors) max_crypt_sectors = 0;

Please bear with me if this has been discussed in a previous round, but
why exactly do we cast `max_crypt_sectors` to the type of
`source->total_sectors`? And isn't the variable always set anyway in
case the keyslot has a non-zero priority?

Patrick

> +
> +  grub_errno = GRUB_ERR_NONE;
>ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, i);
>if (ret)
>   goto err;
> +  if (grub_errno != GRUB_ERR_NONE)
> +   grub_dprintf ("luks2", "Ignoring unhandled error %d from 
> luks2_get_keyslot\n", grub_errno);
>  
>if (keyslot.priority == 0)
>   {
> -   grub_dprintf ("luks2", "Ignoring keyslot \"%"PRIuGRUB_UINT64_T"\" due 
> to priority\n", keyslot.slot_key);
> +   grub_dprintf ("luks2", "Ignoring keyslot \"%"PRIuGRUB_UINT64_T"\" due 
> to priority\n", keyslot.json_slot_key);
> continue;
>  }
>  
> -  grub_dprintf ("luks2", "Trying keyslot \"%"PRIuGRUB_UINT64_T"\"\n", 
> keyslot.slot_key);
> +  grub_dprintf ("luks2", "Trying keyslot \"%"PRIuGRUB_UINT64_T"\"\n", 
> keyslot.json_slot_key);
>  
>/* Set up disk according to keyslot's segment. */
>crypt->offset_sectors = grub_divmod64 (segment.offset, 
> segment.sector_size, NULL);
>crypt->log_sector_size = sizeof (unsigned int) * 8
>   - __builtin_clz ((unsigned int) segment.sector_size) - 1;
> +  /* Set to the source disk size, which is the maximum we allow. */
> +  max_crypt_sectors = grub_disk_convert_sector(source,
> +source->total_sectors,
> +crypt->log_sector_size);
> +
> +  if (max_crypt_sectors < crypt->offset_sectors)
> + {
> +   grub_dprintf ("luks2", "Segment

Re: [PATCH v7 13/17] luks2: Error check segment.sector_size

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:42AM -0600, Glenn Washburn wrote:
> Reviewed-by: Daniel Kiper 
> Signed-off-by: Glenn Washburn 

Reviewed-by: Patrick Steinhardt 

> ---
>  grub-core/disk/luks2.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index 1bb3a333d..499c9330b 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -629,6 +629,17 @@ luks2_recover_key (grub_disk_t source,
>  
>grub_dprintf ("luks2", "Trying keyslot \"%"PRIuGRUB_UINT64_T"\"\n", 
> keyslot.json_slot_key);
>  
> +  /* Sector size should be one of 512, 1024, 2048, or 4096. */
> +  if (!(segment.sector_size == 512 || segment.sector_size == 1024 ||
> + segment.sector_size == 2048 || segment.sector_size == 4096))
> + {
> +   grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" sector"
> +  " size %"PRIuGRUB_UINT64_T" is not one of"
> +  " 512, 1024, 2048, or 4096\n",
> +  segment.json_slot_key, segment.sector_size);
> +   continue;
> + }
> +
>/* Set up disk according to keyslot's segment. */
>crypt->offset_sectors = grub_divmod64 (segment.offset, 
> segment.sector_size, NULL);
>crypt->log_sector_size = sizeof (unsigned int) * 8
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 15/17] mips: Enable __clzdi2()

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:44AM -0600, Glenn Washburn wrote:
> This patch is similiar to commit 9dab2f51e (sparc: Enable __clzsi2() and
> __clzdi2()) but for MIPS target and __clzdi2 only, __clzsi2 was already
> enabled.
> 
> Suggested-by: Daniel Kiper 
> Signed-off-by: Glenn Washburn 

I'm not going to give my Reviewed-by on this patch as I honestly ain't
got a clue if those are supported on MIPS. So I don't feel qualified to.

Patrick

> ---
>  grub-core/kern/compiler-rt.c | 2 +-
>  include/grub/compiler-rt.h   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/kern/compiler-rt.c b/grub-core/kern/compiler-rt.c
> index a464200c6..2057c2e0c 100644
> --- a/grub-core/kern/compiler-rt.c
> +++ b/grub-core/kern/compiler-rt.c
> @@ -448,7 +448,7 @@ __clzsi2 (grub_uint32_t val)
>  }
>  #endif
>  
> -#if defined(__riscv) || defined(__sparc__)
> +#if defined(__mips__) || defined(__riscv) || defined(__sparc__)
>  int
>  __clzdi2 (grub_uint64_t val)
>  {
> diff --git a/include/grub/compiler-rt.h b/include/grub/compiler-rt.h
> index 7591980b4..17828b322 100644
> --- a/include/grub/compiler-rt.h
> +++ b/include/grub/compiler-rt.h
> @@ -115,7 +115,7 @@ int
>  EXPORT_FUNC (__clzsi2) (grub_uint32_t val);
>  #endif
>  
> -#if defined(__riscv) || defined(__sparc__)
> +#if defined(__mips__) || defined(__riscv) || defined(__sparc__)
>  int
>  EXPORT_FUNC (__clzdi2) (grub_uint64_t val);
>  #endif
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v7 17/17] luks2: Use grub_log2ull to calculate log_sector_size and improve readability

2020-12-06 Thread Patrick Steinhardt
On Fri, Dec 04, 2020 at 10:43:46AM -0600, Glenn Washburn wrote:
> Reviewed-by: Daniel Kiper 
> Signed-off-by: Glenn Washburn 
> ---
>  grub-core/disk/luks2.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index 2335ded77..4fa5a0dbc 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -642,8 +642,7 @@ luks2_recover_key (grub_disk_t source,
>  
>/* Set up disk according to keyslot's segment. */
>crypt->offset_sectors = grub_divmod64 (segment.offset, 
> segment.sector_size, NULL);
> -  crypt->log_sector_size = sizeof (unsigned int) * 8
> - - __builtin_clz ((unsigned int) segment.sector_size) - 1;
> +  crypt->log_sector_size = grub_log2ull (segment.sector_size);

I was quite confused by the fact that previously we were using an
unsigned int (which typically is 4 bytes on 64 bit systems), but now
we're using the 8 byte wide uint64. Until I realized that
`grub_log2ull(n)` uses `__builtin_clzll` instead of `__builtin_clz`. A
commit message would've helped to clear up this confusion.

Other than that:

Reviewed-by: Patrick Steinhardt 

>/* Set to the source disk size, which is the maximum we allow. */
>max_crypt_sectors = grub_disk_convert_sector(source,
>  source->total_sectors,
> -- 
> 2.27.0
> 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[ANNOUNCEMENT] FOSDEM 2021 Open Source Firmware, BMC and Bootloader Devroom Call for Participation

2020-12-06 Thread Daniel Kiper
Hi,

The Open Source Firmware, BMC and Bootloader devroom will take place
on Sunday, 7th February 2021 at FOSDEM, virtual.

Call for Participation
--
We are opening the call for participation to our devroom, with the deadline
for talk proposals set to Sunday, 20th December 2020 23:59:59 UTC.

Devroom Scope
-
The devroom will focus on various topics related to the open source firmware,
BMC and bootloaders including security issues. It will help to get together
all interested people in one place, present and discuss current developments
and issues haunting the community. Additionally, it will foster awareness
among attendees about pre-OS topics.

Possible topics: GRUB, TrustedGRUB, iPXE, TrenchBoot, coreboot, LinuxBoot,
SeaBIOS, UEFI, OVMF, TianoCore, hostboot, IPMI, OpenBMC, u-bmc, TPM,
attestation, security of firmware, BMC, bootloaders and related topics
and technologies.

Ideal submissions are actionable and opinionated. Submissions may be in the
form of 25 or 45 minutes talks, panel sessions or round-table discussions.

Dates
-
Submission Deadline: 20-Dec-2020 @ 23:59:59 UTC
Acceptance Notification: 31-Dec-2020
Final Schedule Posted:   31-Dec-2020

Recording and uploading presentations: 04-Jan-2021 - 16-Jan-2021
Processing, reviewing, fixing videos:  17-Jan-2021 - 31-Jan-2021

How to submit
--
Visit https://penta.fosdem.org/submission/FOSDEM21

1) If you do not have an account, create one here
2) Click "Create Event"
3) Enter your presentation details
4) Be sure to select the Open Source Firmware,
   BMC and Bootloader devroom track!
5) Submit

What to include
---
- The title of your submission
- A 1-paragraph abstract
- A longer description including the benefit of your talk to your target
  audience, including a definition of your target audience
- Approximate length/type of submission (talk, BoF, ...)
- Links to related websites/blogs/talk material (if any)

Administrative Notes

Since the FOSDEM 2021 is going virtual, the talks will be prerecorded. We will 
be
streaming the recordings and performing live Q&A of the Open Source Firmware,
BMC and Bootloader devroom. Presenting at FOSDEM implies permission to record
your session and distribute the recording afterwards. All videos will be made
available under the standard FOSDEM content license (CC-BY).

If you have any questions, feel free to contact the devroom organizers:
  Daniel Kiper (daniel.kiper at oracle.com) and
  Michał Żygowski (michal.zygowski at 3mdeb.com).

Daniel & Michał

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 0/7] Add xHCI USB support

2020-12-06 Thread Patrick Rudolph
Add basic support for xHCI USB controllers and non root xHCI hubs.
The motivation is to use this code on platforms that do not provide
user input by runtime services (like BIOS or UEFI platform) do.
This is the case when GRUB is used as coreboot payload for example.

The code is based on seabios implementation, but has been heavily
modified to match grubs internals.

Changes done in version 2:
 * Code cleanup
 * Code style fixes
 * Don't leak memory buffers
 * Compile without warnings
 * Add more defines
 * Add more helper functions
 * Don't assume a 1:1 virtual to physical mapping
 * Flush cachelines after writing buffers
 * Don't use hardcoded page size
 * Proper scratchpad register setup
 * xHCI bios ownership handoff

Changes done in version 3:
 * Several bug fixes for real hardware
 * Fixed a race condition detecting events, which doesn't appear on
   qemu based xHCI controllers
 * Don't accidently disable USB3.0 devices after first command
 * Support arbitrary protocol speed IDs
 * Coding style cleanup
 * Support for non root SuperSpeed hubs

Changes Tested:
 * Qemu system x86_64
   * virtual USB HID keyboard (usb-kbd)
   * virtual USB HID mass storage (usb-storage)
 * Intel C246 integrated xHCI (Supermicro X11SSH-F)
   * iKVM HID keyboard
   * USB3 HID mass storage (controller root port)
   * External USB HID keyboard

TODO:
 * Test on more hardware
 * Test on USB3 hubs
 * Support for USB 3.1 and USB 3.2 controllers

Patrick Rudolph (7):
  grub-core/bus/usb: Parse SuperSpeed companion descriptors
  usb: Add enum for xHCI
  usbtrans: Set default maximum packet size
  grub-core/bus/usb: Add function pointer for attach/detach events
  grub-core/bus/usb/usbhub: Add new private fields for xHCI controller
  grub-core/bus/usb: Add xhci support
  grub-core/bus/usb/usbhub: Add xHCI non root hub support

 Makefile.am   |2 +-
 grub-core/Makefile.core.def   |7 +
 grub-core/bus/usb/ehci.c  |2 +
 grub-core/bus/usb/ohci.c  |2 +
 grub-core/bus/usb/serial/common.c |2 +-
 grub-core/bus/usb/uhci.c  |2 +
 grub-core/bus/usb/usb.c   |   44 +-
 grub-core/bus/usb/usbhub.c|   95 +-
 grub-core/bus/usb/usbtrans.c  |6 +-
 grub-core/bus/usb/xhci-pci.c  |  195 +++
 grub-core/bus/usb/xhci.c  | 2496 +
 grub-core/commands/usbtest.c  |2 +-
 grub-core/disk/usbms.c|2 +-
 grub-core/term/usb_keyboard.c |2 +-
 include/grub/usb.h|   18 +-
 include/grub/usbdesc.h|   12 +-
 include/grub/usbtrans.h   |4 +
 17 files changed, 2852 insertions(+), 41 deletions(-)
 create mode 100644 grub-core/bus/usb/xhci-pci.c
 create mode 100644 grub-core/bus/usb/xhci.c

-- 
2.26.2


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 1/7] grub-core/bus/usb: Parse SuperSpeed companion descriptors

2020-12-06 Thread Patrick Rudolph
Parse the SS_ENDPOINT_COMPANION descriptor, which is only present on USB 3.0
capable devices and xHCI controllers. Make the descendp an array of pointers
to the endpoint descriptor as it's no longer an continous array.

Signed-off-by: Patrick Rudolph 
---
 grub-core/bus/usb/serial/common.c |  2 +-
 grub-core/bus/usb/usb.c   | 44 +++
 grub-core/bus/usb/usbhub.c| 22 
 grub-core/commands/usbtest.c  |  2 +-
 grub-core/disk/usbms.c|  2 +-
 grub-core/term/usb_keyboard.c |  2 +-
 include/grub/usb.h|  2 +-
 include/grub/usbdesc.h| 11 +++-
 8 files changed, 59 insertions(+), 28 deletions(-)

diff --git a/grub-core/bus/usb/serial/common.c 
b/grub-core/bus/usb/serial/common.c
index 8e94c7dc0..63e64cc0a 100644
--- a/grub-core/bus/usb/serial/common.c
+++ b/grub-core/bus/usb/serial/common.c
@@ -72,7 +72,7 @@ grub_usbserial_attach (grub_usb_device_t usbdev, int 
configno, int interfno,
   for (j = 0; j < interf->endpointcnt; j++)
 {
   struct grub_usb_desc_endp *endp;
-  endp = &usbdev->config[0].interf[interfno].descendp[j];
+  endp = usbdev->config[0].interf[interfno].descendp[j];
 
   if ((endp->endp_addr & 128) && (endp->attrib & 3) == 2
  && (in_endp == GRUB_USB_SERIAL_ENDPOINT_LAST_MATCHING
diff --git a/grub-core/bus/usb/usb.c b/grub-core/bus/usb/usb.c
index 8da5e4c74..6b32bbd93 100644
--- a/grub-core/bus/usb/usb.c
+++ b/grub-core/bus/usb/usb.c
@@ -115,7 +115,7 @@ grub_usb_device_initialize (grub_usb_device_t dev)
   struct grub_usb_desc_device *descdev;
   struct grub_usb_desc_config config;
   grub_usb_err_t err;
-  int i;
+  int i, j;
 
   /* First we have to read first 8 bytes only and determine
* max. size of packet */
@@ -149,6 +149,7 @@ grub_usb_device_initialize (grub_usb_device_t dev)
   int currif;
   char *data;
   struct grub_usb_desc *desc;
+  struct grub_usb_desc_endp *endp;
 
   /* First just read the first 4 bytes of the configuration
 descriptor, after that it is known how many bytes really have
@@ -192,24 +193,27 @@ grub_usb_device_initialize (grub_usb_device_t dev)
= (struct grub_usb_desc_if *) &data[pos];
  pos += dev->config[i].interf[currif].descif->length;
 
+dev->config[i].interf[currif].descendp = grub_malloc (
+dev->config[i].interf[currif].descif->endpointcnt *
+sizeof(struct grub_usb_desc_endp));
+
+j = 0;
  while (pos < config.totallen)
 {
   desc = (struct grub_usb_desc *)&data[pos];
-  if (desc->type == GRUB_USB_DESCRIPTOR_ENDPOINT)
-break;
-  if (!desc->length)
-{
-  err = GRUB_USB_ERR_BADDEVICE;
-  goto fail;
-}
-  pos += desc->length;
-}
-
- /* Point to the first endpoint.  */
- dev->config[i].interf[currif].descendp
-   = (struct grub_usb_desc_endp *) &data[pos];
- pos += (sizeof (struct grub_usb_desc_endp)
- * dev->config[i].interf[currif].descif->endpointcnt);
+  if (desc->type == GRUB_USB_DESCRIPTOR_ENDPOINT) {
+endp = (struct grub_usb_desc_endp *) &data[pos];
+dev->config[i].interf[currif].descendp[j++] = endp;
+pos += desc->length;
+  } else {
+if (!desc->length)
+  {
+err = GRUB_USB_ERR_BADDEVICE;
+goto fail;
+  }
+pos += desc->length;
+ }
+ }
}
 }
 
@@ -217,8 +221,14 @@ grub_usb_device_initialize (grub_usb_device_t dev)
 
  fail:
 
-  for (i = 0; i < 8; i++)
+  for (i = 0; i < 8; i++) {
+int currif;
+
+for (currif = 0; currif < dev->config[i].descconf->numif; currif++)
+  grub_free (dev->config[i].interf[currif].descendp);
+
 grub_free (dev->config[i].descconf);
+  }
 
   return err;
 }
diff --git a/grub-core/bus/usb/usbhub.c b/grub-core/bus/usb/usbhub.c
index a06cce302..136bf6ee4 100644
--- a/grub-core/bus/usb/usbhub.c
+++ b/grub-core/bus/usb/usbhub.c
@@ -82,8 +82,14 @@ grub_usb_hub_add_dev (grub_usb_controller_t controller,
   if (i == GRUB_USBHUB_MAX_DEVICES)
 {
   grub_error (GRUB_ERR_IO, "can't assign address to USB device");
-  for (i = 0; i < 8; i++)
-grub_free (dev->config[i].descconf);
+  for (i = 0; i < 8; i++) {
+   int currif;
+
+   for (currif = 0; currif < dev->config[i].descconf->numif; currif++)
+ grub_free (dev->config[i].interf[currif].descendp);
+
+   grub_free (dev->config[i].descconf);
+  }
   grub_free (dev);
   return NULL;
 }
@@ -96,8 +102,14 @@ grub_usb_hub_add_dev (grub_usb_controller_t controller,
  i, 0, 0, NULL);
   if (err)
 {
-  for (i = 0; i < 8; i++)
-grub_free (dev->config[i].descconf);
+  f

[PATCH v2 2/7] usb: Add enum for xHCI

2020-12-06 Thread Patrick Rudolph
Will be used in future patches.

Signed-off-by: Patrick Rudolph 
---
 include/grub/usb.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/grub/usb.h b/include/grub/usb.h
index 0527a3e2b..f65ac4f5d 100644
--- a/include/grub/usb.h
+++ b/include/grub/usb.h
@@ -47,7 +47,8 @@ typedef enum
 GRUB_USB_SPEED_NONE,
 GRUB_USB_SPEED_LOW,
 GRUB_USB_SPEED_FULL,
-GRUB_USB_SPEED_HIGH
+GRUB_USB_SPEED_HIGH,
+GRUB_USB_SPEED_SUPER
   } grub_usb_speed_t;
 
 typedef int (*grub_usb_iterate_hook_t) (grub_usb_device_t dev, void *data);
-- 
2.26.2


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 5/7] grub-core/bus/usb/usbhub: Add new private fields for xHCI controller

2020-12-06 Thread Patrick Rudolph
Store the root port number, the route, consisting out of the port ID
in each nibble, and a pointer to driver private data.

Signed-off-by: Patrick Rudolph 
---
 grub-core/bus/usb/usbhub.c | 14 ++
 include/grub/usb.h |  5 +
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/grub-core/bus/usb/usbhub.c b/grub-core/bus/usb/usbhub.c
index df1913aee..63bee1d1a 100644
--- a/grub-core/bus/usb/usbhub.c
+++ b/grub-core/bus/usb/usbhub.c
@@ -49,7 +49,9 @@ static grub_usb_controller_dev_t grub_usb_list;
 static grub_usb_device_t
 grub_usb_hub_add_dev (grub_usb_controller_t controller,
   grub_usb_speed_t speed,
-  int split_hubport, int split_hubaddr)
+  int split_hubport, int split_hubaddr,
+  int root_portno,
+  grub_uint32_t route)
 {
   grub_usb_device_t dev;
   int i;
@@ -65,6 +67,8 @@ grub_usb_hub_add_dev (grub_usb_controller_t controller,
   dev->speed = speed;
   dev->split_hubport = split_hubport;
   dev->split_hubaddr = split_hubaddr;
+  dev->root_port = root_portno;
+  dev->route = route;
 
   if (controller->dev->attach_dev) {
 err = controller->dev->attach_dev (controller, dev);
@@ -245,7 +249,7 @@ attach_root_port (struct grub_usb_hub *hub, int portno,
  and full/low speed device connected to OHCI/UHCI needs not
  transaction translation - e.g. hubport and hubaddr should be
  always none (zero) for any device connected to any root hub. */
-  dev = grub_usb_hub_add_dev (hub->controller, speed, 0, 0);
+  dev = grub_usb_hub_add_dev (hub->controller, speed, 0, 0, portno, 0);
   hub->controller->dev->pending_reset = 0;
   npending--;
   if (! dev)
@@ -673,10 +677,12 @@ poll_nonroot_hub (grub_usb_device_t dev)
split_hubport = dev->split_hubport;
split_hubaddr = dev->split_hubaddr;
  }
-   
+
+
  /* Add the device and assign a device address to it.  */
  next_dev = grub_usb_hub_add_dev (&dev->controller, speed,
-  split_hubport, split_hubaddr);
+  split_hubport, split_hubaddr, 
dev->root_port,
+  dev->route << 4 | (i & 0xf));
  if (dev->controller.dev->pending_reset)
{
  dev->controller.dev->pending_reset = 0;
diff --git a/include/grub/usb.h b/include/grub/usb.h
index 5c5fb5ec4..5a3325b7d 100644
--- a/include/grub/usb.h
+++ b/include/grub/usb.h
@@ -233,6 +233,11 @@ struct grub_usb_device
   int split_hubport;
 
   int split_hubaddr;
+
+  /* xHCI specific information */
+  int root_port;
+  grub_uint32_t route;
+  void *xhci_priv;
 };
 
 
-- 
2.26.2


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 3/7] usbtrans: Set default maximum packet size

2020-12-06 Thread Patrick Rudolph
Set the maximum packet size to 512 for SuperSpeed devices.

Signed-off-by: Patrick Rudolph 
---
 grub-core/bus/usb/usbtrans.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/grub-core/bus/usb/usbtrans.c b/grub-core/bus/usb/usbtrans.c
index 85f081fff..72eb9b598 100644
--- a/grub-core/bus/usb/usbtrans.c
+++ b/grub-core/bus/usb/usbtrans.c
@@ -128,8 +128,12 @@ grub_usb_control_msg (grub_usb_device_t dev,
   setupdata_addr = grub_dma_get_phys (setupdata_chunk);
 
   /* Determine the maximum packet size.  */
-  if (dev->descdev.maxsize0)
+  if (dev->descdev.maxsize0 && dev->speed != GRUB_USB_SPEED_SUPER)
 max = dev->descdev.maxsize0;
+  else if (dev->descdev.maxsize0 && dev->speed == GRUB_USB_SPEED_SUPER)
+max = 1UL << dev->descdev.maxsize0;
+  else if (dev->speed == GRUB_USB_SPEED_SUPER)
+max = 512;
   else
 max = 64;
 
-- 
2.26.2


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 4/7] grub-core/bus/usb: Add function pointer for attach/detach events

2020-12-06 Thread Patrick Rudolph
The xHCI code needs to be called for attaching or detaching a device.
Introduce two functions pointers and call it from the USB hub code.

Will be used in future commits, currently this doesn't change any functionality.

Signed-off-by: Patrick Rudolph 
---
 grub-core/bus/usb/ehci.c   |  2 ++
 grub-core/bus/usb/ohci.c   |  2 ++
 grub-core/bus/usb/uhci.c   |  2 ++
 grub-core/bus/usb/usbhub.c | 19 +++
 include/grub/usb.h |  4 
 5 files changed, 29 insertions(+)

diff --git a/grub-core/bus/usb/ehci.c b/grub-core/bus/usb/ehci.c
index d966fc210..6b3f2f87a 100644
--- a/grub-core/bus/usb/ehci.c
+++ b/grub-core/bus/usb/ehci.c
@@ -1812,6 +1812,8 @@ static struct grub_usb_controller_dev usb_controller = {
   .hubports = grub_ehci_hubports,
   .portstatus = grub_ehci_portstatus,
   .detect_dev = grub_ehci_detect_dev,
+  .attach_dev = NULL,
+  .detach_dev = NULL,
   /* estimated max. count of TDs for one bulk transfer */
   .max_bulk_tds = GRUB_EHCI_N_TD * 3 / 4 
 };
diff --git a/grub-core/bus/usb/ohci.c b/grub-core/bus/usb/ohci.c
index f0be533d4..44c7e94a9 100644
--- a/grub-core/bus/usb/ohci.c
+++ b/grub-core/bus/usb/ohci.c
@@ -1440,6 +1440,8 @@ static struct grub_usb_controller_dev usb_controller =
   .hubports = grub_ohci_hubports,
   .portstatus = grub_ohci_portstatus,
   .detect_dev = grub_ohci_detect_dev,
+  .attach_dev = NULL,
+  .detach_dev = NULL,
   /* estimated max. count of TDs for one bulk transfer */
   .max_bulk_tds = GRUB_OHCI_TDS * 3 / 4
 };
diff --git a/grub-core/bus/usb/uhci.c b/grub-core/bus/usb/uhci.c
index 7c5811fd6..4143d24fe 100644
--- a/grub-core/bus/usb/uhci.c
+++ b/grub-core/bus/usb/uhci.c
@@ -845,6 +845,8 @@ static struct grub_usb_controller_dev usb_controller =
   .hubports = grub_uhci_hubports,
   .portstatus = grub_uhci_portstatus,
   .detect_dev = grub_uhci_detect_dev,
+  .attach_dev = NULL,
+  .detach_dev = NULL,
   /* estimated max. count of TDs for one bulk transfer */
   .max_bulk_tds = N_TD * 3 / 4
 };
diff --git a/grub-core/bus/usb/usbhub.c b/grub-core/bus/usb/usbhub.c
index 136bf6ee4..df1913aee 100644
--- a/grub-core/bus/usb/usbhub.c
+++ b/grub-core/bus/usb/usbhub.c
@@ -66,6 +66,15 @@ grub_usb_hub_add_dev (grub_usb_controller_t controller,
   dev->split_hubport = split_hubport;
   dev->split_hubaddr = split_hubaddr;
 
+  if (controller->dev->attach_dev) {
+err = controller->dev->attach_dev (controller, dev);
+if (err)
+  {
+   grub_free (dev);
+   return NULL;
+  }
+  }
+
   err = grub_usb_device_initialize (dev);
   if (err)
 {
@@ -405,6 +414,8 @@ static void
 detach_device (grub_usb_device_t dev)
 {
   unsigned i;
+  grub_usb_err_t err;
+
   int k;
   if (!dev)
 return;
@@ -425,6 +436,14 @@ detach_device (grub_usb_device_t dev)
  if (inter && inter->detach_hook)
inter->detach_hook (dev, i, k);
}
+  if (dev->controller.dev->detach_dev) {
+err = dev->controller.dev->detach_dev (&dev->controller, dev);
+if (err)
+  {
+   // XXX
+  }
+  }
+
   grub_usb_devs[dev->addr] = 0;
 }
 
diff --git a/include/grub/usb.h b/include/grub/usb.h
index f65ac4f5d..5c5fb5ec4 100644
--- a/include/grub/usb.h
+++ b/include/grub/usb.h
@@ -122,6 +122,10 @@ struct grub_usb_controller_dev
 
   grub_usb_speed_t (*detect_dev) (grub_usb_controller_t dev, int port, int 
*changed);
 
+  grub_usb_err_t (*attach_dev) (grub_usb_controller_t ctrl, grub_usb_device_t 
dev);
+
+  grub_usb_err_t (*detach_dev) (grub_usb_controller_t ctrl, grub_usb_device_t 
dev);
+
   /* Per controller flag - port reset pending, don't do another reset */
   grub_uint64_t pending_reset;
 
-- 
2.26.2


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 7/7] grub-core/bus/usb/usbhub: Add xHCI non root hub support

2020-12-06 Thread Patrick Rudolph
Tested on Intel PCH C246, the USB3 hub can be configured by grub.

Issues:
* USB3 devices connected behind that hub are sometimes not detected.

Signed-off-by: Patrick Rudolph 
---
 grub-core/bus/usb/usbhub.c | 40 --
 include/grub/usbdesc.h |  1 +
 include/grub/usbtrans.h|  4 
 3 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/grub-core/bus/usb/usbhub.c b/grub-core/bus/usb/usbhub.c
index 63bee1d1a..841796f2f 100644
--- a/grub-core/bus/usb/usbhub.c
+++ b/grub-core/bus/usb/usbhub.c
@@ -148,19 +148,32 @@ grub_usb_hub_add_dev (grub_usb_controller_t controller,
   return dev;
 }
 
-
+static grub_usb_err_t
+grub_usb_set_hub_depth(grub_usb_device_t dev, grub_uint8_t depth)
+{
+  return grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_OUT
+   | GRUB_USB_REQTYPE_CLASS
+   | GRUB_USB_REQTYPE_TARGET_DEV),
+ GRUB_USB_HUB_REQ_SET_HUB_DEPTH, depth,
+ 0, 0, NULL);
+}
+
 static grub_usb_err_t
 grub_usb_add_hub (grub_usb_device_t dev)
 {
   struct grub_usb_usb_hubdesc hubdesc;
   grub_usb_err_t err;
+  grub_uint16_t req;
   int i;
-  
+
+  req = (dev->speed == GRUB_USB_SPEED_SUPER) ? GRUB_USB_DESCRIPTOR_SS_HUB :
+GRUB_USB_DESCRIPTOR_HUB;
+
   err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_IN
| GRUB_USB_REQTYPE_CLASS
| GRUB_USB_REQTYPE_TARGET_DEV),
-  GRUB_USB_REQ_GET_DESCRIPTOR,
- (GRUB_USB_DESCRIPTOR_HUB << 8) | 0,
+ GRUB_USB_REQ_GET_DESCRIPTOR,
+ (req << 8) | 0,
  0, sizeof (hubdesc), (char *) &hubdesc);
   if (err)
 return err;
@@ -183,6 +196,19 @@ grub_usb_add_hub (grub_usb_device_t dev)
   return GRUB_USB_ERR_INTERNAL;
 }
 
+  if (dev->speed == GRUB_USB_SPEED_SUPER)
+{
+  grub_uint8_t depth;
+  grub_uint32_t route;
+  /* Depth maximum value is 5, but root hubs doesn't count */
+  for (depth = 0, route = dev->route; (route & 0xf) > 0; route >>= 4)
+   depth++;
+
+  err = grub_usb_set_hub_depth(dev, depth);
+  if (err)
+return err;
+}
+
   /* Power on all Hub ports.  */
   for (i = 1; i <= hubdesc.portcnt; i++)
 {
@@ -637,7 +663,9 @@ poll_nonroot_hub (grub_usb_device_t dev)
  int split_hubaddr = 0;
 
  /* Determine the device speed.  */
- if (status & GRUB_USB_HUB_STATUS_PORT_LOWSPEED)
+ if (dev->speed == GRUB_USB_SPEED_SUPER)
+   speed = GRUB_USB_SPEED_SUPER;
+ else if (status & GRUB_USB_HUB_STATUS_PORT_LOWSPEED)
speed = GRUB_USB_SPEED_LOW;
  else
{
@@ -651,7 +679,7 @@ poll_nonroot_hub (grub_usb_device_t dev)
  grub_millisleep (10);
 
   /* Find correct values for SPLIT hubport and hubaddr */
- if (speed == GRUB_USB_SPEED_HIGH)
+ if (speed == GRUB_USB_SPEED_HIGH || speed == GRUB_USB_SPEED_SUPER)
{
  /* HIGH speed device needs not transaction translation */
  split_hubport = 0;
diff --git a/include/grub/usbdesc.h b/include/grub/usbdesc.h
index bb2ab2e27..1697aa465 100644
--- a/include/grub/usbdesc.h
+++ b/include/grub/usbdesc.h
@@ -30,6 +30,7 @@ typedef enum {
   GRUB_USB_DESCRIPTOR_ENDPOINT,
   GRUB_USB_DESCRIPTOR_DEBUG = 10,
   GRUB_USB_DESCRIPTOR_HUB = 0x29,
+  GRUB_USB_DESCRIPTOR_SS_HUB = 0x2a,
   GRUB_USB_DESCRIPTOR_SS_ENDPOINT_COMPANION = 0x30
 } grub_usb_descriptor_t;
 
diff --git a/include/grub/usbtrans.h b/include/grub/usbtrans.h
index aef482cb9..a5891e663 100644
--- a/include/grub/usbtrans.h
+++ b/include/grub/usbtrans.h
@@ -110,6 +110,10 @@ enum
 GRUB_USB_REQ_SET_INTERFACE = 0x0B,
 GRUB_USB_REQ_SYNC_FRAME = 0x0C
   };
+enum
+  {
+GRUB_USB_HUB_REQ_SET_HUB_DEPTH = 0x0C,
+  };
 
 #define GRUB_USB_FEATURE_ENDP_HALT 0x00
 #define GRUB_USB_FEATURE_DEV_REMOTE_WU 0x01
-- 
2.26.2


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 6/7] grub-core/bus/usb: Add xhci support

2020-12-06 Thread Patrick Rudolph
Add support for xHCI USB controllers.
The code is based on seabios implementation, but has been heavily
modified to match grubs internals.

Changes done in version 2:
* Code cleanup
* Code style fixes
* Don't leak memory buffers
* Compile without warnings
* Add more defines
* Add more helper functions
* Don't assume a 1:1 virtual to physical mapping
* Flush cachelines after writing buffers
* Don't use hardcoded page size
* Proper scratchpad register setup
* xHCI bios ownership handoff

Changes done in version 3:
* Fixed a race condition detecting events, which doesn't appear on
  qemu based xHCI controllers
* Don't accidently disable USB3.0 devices after first command
* Support arbitrary protocol speed IDs
* Coding style cleanup

Tested:
* Qemu system x86_64
  * virtual USB HID keyboard (usb-kbd)
  * virtual USB HID mass storage (usb-storage)
* init Supermicro X11SSH-F
  * iKVM HID keyboard
  * USB3 HID mass storage (controller root port)
  * USB HID keyboard

TODO:
  * Test on more hardware
  * Test on USB3 hubs
  * Support for USB 3.1 and USB 3.2 controllers

Signed-off-by: Patrick Rudolph 
Signed-off-by: sylv 
---
 Makefile.am  |2 +-
 grub-core/Makefile.core.def  |7 +
 grub-core/bus/usb/xhci-pci.c |  195 +++
 grub-core/bus/usb/xhci.c | 2496 ++
 include/grub/usb.h   |4 +
 5 files changed, 2703 insertions(+), 1 deletion(-)
 create mode 100644 grub-core/bus/usb/xhci-pci.c
 create mode 100644 grub-core/bus/usb/xhci.c

diff --git a/Makefile.am b/Makefile.am
index bf9c1ba64..81b9d1092 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -425,7 +425,7 @@ if COND_i386_coreboot
 FS_PAYLOAD_MODULES ?= $(shell cat grub-core/fs.lst)
 default_payload.elf: grub-mkstandalone grub-mkimage FORCE
test -f $@ && rm $@ || true
-   pkgdatadir=. ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O 
i386-coreboot -o $@ --modules='ahci pata ehci uhci ohci usb_keyboard usbms 
part_msdos ext2 fat at_keyboard part_gpt usbserial_usbdebug cbfs' 
--install-modules='ls linux search configfile normal cbtime cbls memrw iorw 
minicmd lsmmap lspci halt reboot hexdump pcidump regexp setpci lsacpi chain 
test serial multiboot cbmemc linux16 gzio echo help syslinuxcfg xnu 
$(FS_PAYLOAD_MODULES) password_pbkdf2 $(EXTRA_PAYLOAD_MODULES)' --fonts= 
--themes= --locales= -d grub-core/ /boot/grub/grub.cfg=$(srcdir)/coreboot.cfg
+   pkgdatadir=. ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O 
i386-coreboot -o $@ --modules='ahci pata xhci ehci uhci ohci usb_keyboard usbms 
part_msdos ext2 fat at_keyboard part_gpt usbserial_usbdebug cbfs' 
--install-modules='ls linux search configfile normal cbtime cbls memrw iorw 
minicmd lsmmap lspci halt reboot hexdump pcidump regexp setpci lsacpi chain 
test serial multiboot cbmemc linux16 gzio echo help syslinuxcfg xnu 
$(FS_PAYLOAD_MODULES) password_pbkdf2 $(EXTRA_PAYLOAD_MODULES)' --fonts= 
--themes= --locales= -d grub-core/ /boot/grub/grub.cfg=$(srcdir)/coreboot.cfg
 endif
 
 endif
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index b5f47fc41..d5f550419 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -644,6 +644,13 @@ module = {
   enable = arm_coreboot;
 };
 
+module = {
+  name = xhci;
+  common = bus/usb/xhci.c;
+  pci = bus/usb/xhci-pci.c;
+  enable = pci;
+};
+
 module = {
   name = pci;
   common = bus/pci.c;
diff --git a/grub-core/bus/usb/xhci-pci.c b/grub-core/bus/usb/xhci-pci.c
new file mode 100644
index 0..a5bd3c97d
--- /dev/null
+++ b/grub-core/bus/usb/xhci-pci.c
@@ -0,0 +1,195 @@
+/* xhci.c - XHCI Support.  */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2020 9elements Cyber Security
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define GRUB_XHCI_PCI_SBRN_REG  0x60
+#define GRUB_XHCI_ADDR_MEM_MASK(~0xff)
+
+/* USBLEGSUP bits and related OS OWNED byte offset */
+enum
+{
+  GRUB_XHCI_BIOS_OWNED = (1 << 16),
+  GRUB_XHCI_OS_OWNED = (1 << 24)
+};
+
+/* PCI iteration function... */
+static int
+grub_xhci_pci_iter (grub_pci_device_t dev, grub_pci_id_t pciid,
+   void *data __attribute__ ((unused)))
+{
+  volatile grub_uint32_t *regs;
+  grub_uint32_t base, base_h;
+  grub_uint32_t eecp_offset;
+  grub_uint32_t usblegsup = 0;
+  grub_uint64