[PATCH v3 0/2] Add LUKS2 support to luks_script

[PATCH v3 0/2] Add LUKS2 support to luks_script

[Glenn Washburn]

v3:
 * Fix inconsistencies noted in v2
 * Add a separate patch for optimization and changes in v2
 * Use 6 digits for sector size, instead of previous 5, for really really
   future proofing
v2:
 * Use PRIxGRUB_OFFSET instead of PRIuGRUB_UINT64_T
 * use %u for unsigned log sector size

Glenn

Glenn Washburn (2):
  cryptodisk: Optimize luks_script_get
  cryptodisk: Add support for LUKS2 in (proc)/luks_script

 grub-core/disk/cryptodisk.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

Range-diff against v2:
-:  ------------ > 1:  7a32d780a2f7 cryptodisk: Optimize luks_script_get
1:  dbf695eef1ab ! 2:  fe0164ae1db0 luks2: Add support for LUKS2 in (proc)/luks_script
    @@ Metadata
     Author: Glenn Washburn <development@efficientek.com>
     
      ## Commit message ##
    -    luks2: Add support for LUKS2 in (proc)/luks_script
    +    cryptodisk: Add support for LUKS2 in (proc)/luks_script
     
    -    The sector size in bytes is added to each line and it is allowed to be 5
    +    The sector size in bytes is added to each line and it is allowed to be 6
         decimal digits long, which covers the most common cases of 512 and 4096
    -    byte sectors with space for an additional digit as future-proofing. The
    -    size allocation is updated to reflect this additional field, allow up to
    -    5 characters and 1 space added.
    +    byte sectors with space for two additional digits as future-proofing. The
    +    size allocation is updated to reflect this additional field. Also make
    +    clearer the size allocation calculation.
     
      ## grub-core/disk/cryptodisk.c ##
     @@ grub-core/disk/cryptodisk.c: luks_script_get (grub_size_t *sz)
    @@ grub-core/disk/cryptodisk.c: luks_script_get (grub_size_t *sz)
     +	/*
     +	 * Add space in the line for (in order) spaces, cipher mode, cipher IV
     +	 * mode, sector offset, sector size and the trailing newline. This is
    -+	 * an upper bound on the size of this data. There are 16 extra bytes
    ++	 * an upper bound on the size of this data. There are 15 extra bytes
     +	 * in an earlier version of this code that are unaccounted for. It is
     +	 * left in the calculations in case it is needed. At worst, its short-
     +	 * lived wasted space.
     +	 */
    -+	size += 5 + 5 + 8 + 20 + 5 + 1 + 16;
    ++	size += 5 + 5 + 8 + 20 + 6 + 1 + 15;
      	if (i->essiv_hash)
      	  size += grub_strlen (i->essiv_hash->name);
      	size += i->keysize * 2;
    @@ grub-core/disk/cryptodisk.c: luks_script_get (grub_size_t *sz)
     +	ptr = grub_stpcpy (ptr, "_mount ");
      	ptr = grub_stpcpy (ptr, i->uuid);
      	*ptr++ = ' ';
    --	grub_snprintf (ptr, 21, "%" PRIuGRUB_UINT64_T " ", i->offset_sectors);
    --	while (*ptr)
    --	  ptr++;
    -+	ptr += grub_snprintf (ptr, 21, "%" PRIxGRUB_OFFSET " ",
    -+			      i->offset_sectors);
    -+	ptr += grub_snprintf (ptr, 7, "%u ", 1 << i->log_sector_size);
    + 	ptr += grub_snprintf (ptr, 21, "%" PRIxGRUB_OFFSET, i->offset_sectors);
    + 	*ptr++ = ' ';
    ++	ptr += grub_snprintf (ptr, 7, "%u", 1 << i->log_sector_size);
    ++	*ptr++ = ' ';
      	for (iptr = i->cipher->cipher->name; *iptr; iptr++)
      	  *ptr++ = grub_tolower (*iptr);
      	switch (i->mode)
-- 
2.34.1

[PATCH v3 1/2] cryptodisk: Optimize luks_script_get

[Glenn Washburn]

Use the return value of grub_snprintf() to move the string pointer forward,
instead of incrementing the string pointer iteratively until a NULL byte is
reached. Move the space out of the format string argument, a small
optimization, but also makes the spacing clearer. Also, use the new
PRIxGRUB_OFFSET instead of PRIuGRUB_UINT64_T to accurately reflect the
format string for this type.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/disk/cryptodisk.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 34b67a705fbc..c62656a0faea 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -1503,9 +1503,8 @@ luks_script_get (grub_size_t *sz)
 	ptr = grub_stpcpy (ptr, "luks_mount ");
 	ptr = grub_stpcpy (ptr, i->uuid);
 	*ptr++ = ' ';
-	grub_snprintf (ptr, 21, "%" PRIuGRUB_UINT64_T " ", i->offset_sectors);
-	while (*ptr)
-	  ptr++;
+	ptr += grub_snprintf (ptr, 21, "%" PRIxGRUB_OFFSET, i->offset_sectors);
+	*ptr++ = ' ';
 	for (iptr = i->cipher->cipher->name; *iptr; iptr++)
 	  *ptr++ = grub_tolower (*iptr);
 	switch (i->mode)
-- 
2.34.1

[PATCH v3 2/2] cryptodisk: Add support for LUKS2 in (proc)/luks_script

[Glenn Washburn]

The sector size in bytes is added to each line and it is allowed to be 6
decimal digits long, which covers the most common cases of 512 and 4096
byte sectors with space for two additional digits as future-proofing. The
size allocation is updated to reflect this additional field. Also make
clearer the size allocation calculation.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/disk/cryptodisk.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index c62656a0faea..066cec566aec 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -1478,12 +1478,22 @@ luks_script_get (grub_size_t *sz)
   *sz = 0;
 
   for (i = cryptodisk_list; i != NULL; i = i->next)
-    if (grub_strcmp (i->modname, "luks") == 0)
+    if (grub_strcmp (i->modname, "luks") == 0 ||
+	grub_strcmp (i->modname, "luks2") == 0)
       {
-	size += sizeof ("luks_mount ");
+	size += grub_strlen (i->modname);
+	size += sizeof ("_mount");
 	size += grub_strlen (i->uuid);
 	size += grub_strlen (i->cipher->cipher->name);
-	size += 54;
+	/*
+	 * Add space in the line for (in order) spaces, cipher mode, cipher IV
+	 * mode, sector offset, sector size and the trailing newline. This is
+	 * an upper bound on the size of this data. There are 15 extra bytes
+	 * in an earlier version of this code that are unaccounted for. It is
+	 * left in the calculations in case it is needed. At worst, its short-
+	 * lived wasted space.
+	 */
+	size += 5 + 5 + 8 + 20 + 6 + 1 + 15;
 	if (i->essiv_hash)
 	  size += grub_strlen (i->essiv_hash->name);
 	size += i->keysize * 2;
@@ -1496,15 +1506,19 @@ luks_script_get (grub_size_t *sz)
   ptr = ret;
 
   for (i = cryptodisk_list; i != NULL; i = i->next)
-    if (grub_strcmp (i->modname, "luks") == 0)
+    if (grub_strcmp (i->modname, "luks") == 0 ||
+	grub_strcmp (i->modname, "luks2") == 0)
       {
 	unsigned j;
 	const char *iptr;
-	ptr = grub_stpcpy (ptr, "luks_mount ");
+	ptr = grub_stpcpy (ptr, i->modname);
+	ptr = grub_stpcpy (ptr, "_mount ");
 	ptr = grub_stpcpy (ptr, i->uuid);
 	*ptr++ = ' ';
 	ptr += grub_snprintf (ptr, 21, "%" PRIxGRUB_OFFSET, i->offset_sectors);
 	*ptr++ = ' ';
+	ptr += grub_snprintf (ptr, 7, "%u", 1 << i->log_sector_size);
+	*ptr++ = ' ';
 	for (iptr = i->cipher->cipher->name; *iptr; iptr++)
 	  *ptr++ = grub_tolower (*iptr);
 	switch (i->mode)
-- 
2.34.1

Re: [PATCH v3 0/2] Add LUKS2 support to luks_script

[Daniel Kiper]

On Fri, Jul 14, 2023 at 03:49:16PM -0500, Glenn Washburn wrote:
> v3:
>  * Fix inconsistencies noted in v2
>  * Add a separate patch for optimization and changes in v2
>  * Use 6 digits for sector size, instead of previous 5, for really really
>    future proofing
> v2:
>  * Use PRIxGRUB_OFFSET instead of PRIuGRUB_UINT64_T
>  * use %u for unsigned log sector size
>
> Glenn
>
> Glenn Washburn (2):
>   cryptodisk: Optimize luks_script_get
>   cryptodisk: Add support for LUKS2 in (proc)/luks_script

For both patches Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...

Daniel

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