This patch series, fixes issues with v7 series. Some of those patches had changes applied to the wrong patch. The added structure member was renamed again, this time to idx. And there is an added patch that renames some json index variables to note that they are such.
Glenn Glenn Washburn (18): disk: Rename grub_disk_get_size to grub_disk_native_sectors misc: Add parentheses around ALIGN_UP and ALIGN_DOWN arguments whitespace: convert 8 spaces to tabs luks2: Remove unused argument in grub_error luks2: Make sure all fields of output argument in luks2_parse_digest() are written to luks2: Add idx member to struct grub_luks2_keyslot/segment/digest luks2: Use more intuitive object name instead of json index in user messages luks2: Rename json index variables to names that they are obviously json indexes 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 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 | 184 ++++++++++++++++++++++------- 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, 261 insertions(+), 111 deletions(-) Range-diff against v7: 1: a80929b15 ! 1: f4062e4f6 disk: Rename grub_disk_get_size to grub_disk_native_sectors @@ Commit message size. Rename to something more appropriate. Suggested-by: Daniel Kiper <daniel.ki...@oracle.com> + Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> + Reviewed-by: Patrick Steinhardt <p...@pks.im> ## grub-core/disk/diskfilter.c ## @@ grub-core/disk/diskfilter.c: scan_disk_partition_iter (grub_disk_t disk, grub_partition_t p, void *data) 2: d4b462fd3 ! 2: 17b1ef195 misc: Add parentheses around ALIGN_UP and ALIGN_DOWN arguments @@ Commit message This ensures that expected order of operations is preserved when arguments are expressions. + Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> + Reviewed-by: Patrick Steinhardt <p...@pks.im> + ## include/grub/misc.h ## @@ #include <grub/compiler.h> -: --------- > 3: eb8625d9f whitespace: convert 8 spaces to tabs 3: d1a23ff0a ! 4: 0f28628e2 luks2: Remove unused argument in grub_error @@ Commit message luks2: Remove unused argument in grub_error Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> + Reviewed-by: Patrick Steinhardt <p...@pks.im> ## grub-core/disk/luks2.c ## @@ grub-core/disk/luks2.c: luks2_parse_segment (grub_luks2_segment_t *out, const grub_json_t *segment) 4: 61fcebc1a ! 5: 705548bc2 luks2: Make sure all fields of output argument in luks2_parse_digest() are written to @@ Commit message Otherwise, the digest could say it belongs to keyslots and segments that it does not. + Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> + Reviewed-by: Patrick Steinhardt <p...@pks.im> + ## grub-core/disk/luks2.c ## @@ grub-core/disk/luks2.c: luks2_parse_digest (grub_luks2_digest_t *out, const grub_json_t *digest) return grub_error (GRUB_ERR_BAD_ARGUMENT, 5: f6a6befdb ! 6: 56d4c397a luks2: Add json_slot_key member to struct grub_luks2_keyslot/segment/digest @@ Metadata Author: Glenn Washburn <developm...@efficientek.com> ## Commit message ## - luks2: Add json_slot_key member to struct grub_luks2_keyslot/segment/digest + luks2: Add idx 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 local 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; @@ grub-core/disk/luks2.c: 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_uint64_t idx; grub_int64_t key_size; grub_int64_t priority; struct @@ grub-core/disk/luks2.c: typedef struct grub_luks2_keyslot grub_luks2_keyslot_t; struct grub_luks2_segment { -+ grub_uint64_t json_slot_key; ++ grub_uint64_t idx; grub_uint64_t offset; const char *size; const char *encryption; @@ grub-core/disk/luks2.c: typedef struct grub_luks2_segment grub_luks2_segment_t; struct grub_luks2_digest { -+ grub_uint64_t json_slot_key; ++ grub_uint64_t idx; /* Both keyslots and segments are interpreted as bitfields here */ grub_uint64_t keyslots; grub_uint64_t segments; @@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k, grub_luks2_d 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_getuint64 (&k->idx, &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); @@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k, grub_luks2_d 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)) ++ grub_json_getuint64 (&d->idx, &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))) ++ if ((d->keyslots & (1 << k->idx))) break; } if (i == size) @@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k, grub_luks2_d { if (grub_json_getchild (&segment, &segments, i) || - grub_json_getuint64 (&idx, &segment, NULL) || -+ grub_json_getuint64 (&s->json_slot_key, &segment, NULL) || ++ grub_json_getuint64 (&s->idx, &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 %"PRIuGRUB_SIZE, i); - if ((d->segments & (1 << idx))) -+ if ((d->segments & (1 << s->json_slot_key))) ++ if ((d->segments & (1 << s->idx))) break; } if (i == size) 6: 5c53328e4 ! 7: 1991eca5a luks2: Use more intuitive slot key instead of index in user messages @@ Metadata Author: Glenn Washburn <developm...@efficientek.com> ## Commit message ## - luks2: Use more intuitive slot key instead of index in user messages + luks2: Use more intuitive object name instead of json index in user messages - Use the slot key name in the json array rather than the 0 based index in the + Use the object 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 @@ Commit message ## grub-core/disk/luks2.c ## @@ grub-core/disk/luks2.c: 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); ++ return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot \"%"PRIuGRUB_UINT64_T"\"", k->idx); /* Get segment that matches the digest. */ if (grub_json_getvalue (&segments, root, "segments") || @@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k, grub_luks2_d } 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_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest \"%"PRIuGRUB_UINT64_T"\"", d->idx); return GRUB_ERR_NONE; } @@ grub-core/disk/luks2.c: 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); ++ grub_dprintf ("luks2", "Ignoring keyslot \"%"PRIuGRUB_UINT64_T"\" due to priority\n", keyslot.idx); continue; - } + } - grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_SIZE"\n", i); -+ grub_dprintf ("luks2", "Trying keyslot \"%"PRIuGRUB_UINT64_T"\"\n", keyslot.slot_key); ++ grub_dprintf ("luks2", "Trying keyslot \"%"PRIuGRUB_UINT64_T"\"\n", keyslot.idx); /* Set up disk according to keyslot's segment. */ crypt->offset_sectors = grub_divmod64 (segment.offset, segment.sector_size, NULL); @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, - 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); ++ keyslot.idx, grub_errmsg); continue; } @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, - 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); ++ keyslot.idx, grub_errmsg); continue; } @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, * 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); ++ grub_printf_ (N_("Slot \"%"PRIuGRUB_UINT64_T"\" opened\n"), keyslot.idx); candidate_key_len = keyslot.key_size; break; 7: a4e5e65de < -: --------- luks2: Add string "index" to user strings using a json index. -: --------- > 8: c1af2d286 luks2: Rename json index variables to names that they are obviously json indexes -: --------- > 9: 984166770 luks2: Add string "index" to user strings using a json index. 8: d143d8eac = 10: acc4a205c cryptodisk: Add macro GRUB_TYPE_BITS() to replace some literals 9: 905cb3912 ! 11: c39083e56 cryptodisk: Add macros GRUB_TYPE_U_MAX/MIN(type) to replace literals @@ include/grub/types.h: typedef grub_int32_t grub_ssize_t; # 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. ++ * 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 10: 5dfb095fd = 12: dee584996 luks2: grub_cryptodisk_t->total_sectors is the max number of device native sectors 11: 78949263b = 13: 06cd1951f cryptodisk: Properly handle non-512 byte sized sectors 12: f4277024e ! 14: 9fbd815f9 luks2: Better error handling when setting up the cryptodisk @@ Commit message 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", - - Check for errors from grub_strtoull when converting segment size from + is "dynamic", verify that the offset is not past the end of disk. Otherwise, + 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. + In this case, we want to continue to try to use this key, but only allow + access up to the end of the source disk. ## grub-core/disk/luks2.c ## -@@ grub-core/disk/luks2.c: 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") || -@@ grub-core/disk/luks2.c: 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; - } @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, goto err; } @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, + } + /* Try all keyslot */ - for (i = 0; i < size; i++) + for (json_idx = 0; json_idx < size; json_idx++) { + typeof(source->total_sectors) max_crypt_sectors = 0; + + grub_errno = GRUB_ERR_NONE; - ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, i); + ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, json_idx); if (ret) goto err; + if (grub_errno != GRUB_ERR_NONE) @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, 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. */ +@@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, 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; @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, + " %"PRIuGRUB_UINT64_T" which is greater than" + " source disk size %"PRIuGRUB_UINT64_T"," + " skipping\n", -+ segment.json_slot_key, crypt->offset_sectors, ++ segment.idx, crypt->offset_sectors, + max_crypt_sectors); + continue; + } @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, + { + grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" size" + " \"%s\" is not a parsable number\n", -+ segment.json_slot_key, segment.size); ++ segment.idx, segment.size); + continue; + } + else if (grub_errno == GRUB_ERR_OUT_OF_RANGE) @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, + " %s overflowed 64-bit unsigned integer," + " the end of the crypto device will be" + " inaccessible\n", -+ segment.json_slot_key, segment.size); ++ segment.idx, segment.size); + if (crypt->total_sectors > max_crypt_sectors) + crypt->total_sectors = max_crypt_sectors; + } @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, + { + grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" has zero" + " sectors, skipping\n", -+ segment.json_slot_key); ++ segment.idx); + continue; + } + else if (max_crypt_sectors < (crypt->offset_sectors + crypt->total_sectors)) @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, + " data position greater than source disk size," + " the end of the crypto device will be" + " inaccessible\n", -+ segment.json_slot_key); ++ segment.idx); + /* Allow decryption up to the end of the source disk. */ + crypt->total_sectors = max_crypt_sectors - crypt->offset_sectors; + } ret = luks2_decrypt_key (candidate_key, source, crypt, &keyslot, (const grub_uint8_t *) passphrase, grub_strlen (passphrase)); - if (ret) - { - grub_dprintf ("luks2", "Decryption with keyslot \"%"PRIuGRUB_UINT64_T"\" failed: %s\n", -- keyslot.slot_key, grub_errmsg); -+ keyslot.json_slot_key, grub_errmsg); - continue; - } - -@@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, - if (ret) - { - grub_dprintf ("luks2", "Could not open keyslot \"%"PRIuGRUB_UINT64_T"\": %s\n", -- keyslot.slot_key, grub_errmsg); -+ keyslot.json_slot_key, grub_errmsg); - continue; - } - -@@ grub-core/disk/luks2.c: 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_UINT64_T"\" opened\n"), keyslot.slot_key); -+ grub_printf_ (N_("Slot \"%"PRIuGRUB_UINT64_T"\" opened\n"), keyslot.json_slot_key); - - candidate_key_len = keyslot.key_size; - break; ## include/grub/disk.h ## @@ 13: 65255f2d4 ! 15: 09c5b15f3 luks2: Error check segment.sector_size @@ Metadata ## Commit message ## luks2: Error check segment.sector_size - Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> - ## grub-core/disk/luks2.c ## @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, - grub_dprintf ("luks2", "Trying keyslot \"%"PRIuGRUB_UINT64_T"\"\n", keyslot.json_slot_key); + grub_dprintf ("luks2", "Trying keyslot \"%"PRIuGRUB_UINT64_T"\"\n", keyslot.idx); + /* Sector size should be one of 512, 1024, 2048, or 4096. */ + if (!(segment.sector_size == 512 || segment.sector_size == 1024 || @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source, + 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); ++ segment.idx, segment.sector_size); + continue; + } + 14: 20357ebd6 < -: --------- whitespace: convert 8 spaces to tabs 15: 600b89d70 = 16: 60e37ca9f mips: Enable __clzdi2() 16: a2d394c95 = 17: 302862ec1 misc: Add grub_log2ull macro for calculating log base 2 of 64-bit integers 17: 68e0dac97 = 18: baa1de127 luks2: Use grub_log2ull to calculate log_sector_size and improve readability -- 2.27.0 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel