[PATCH v2] luks2: Add support for LUKS2 in (proc)/luks_script

[PATCH v2] luks2: 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 5
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.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
Range-diff against v1:
1:  14160a8991f6 ! 1:  dbf695eef1ab luks2: Add support for LUKS2 in (proc)/luks_script
    @@ grub-core/disk/cryptodisk.c: luks_script_get (grub_size_t *sz)
     -	grub_snprintf (ptr, 21, "%" PRIuGRUB_UINT64_T " ", i->offset_sectors);
     -	while (*ptr)
     -	  ptr++;
    -+	ptr += grub_snprintf (ptr, 21, "%" PRIuGRUB_UINT64_T " ",
    ++	ptr += grub_snprintf (ptr, 21, "%" PRIxGRUB_OFFSET " ",
     +			      i->offset_sectors);
    -+	ptr += grub_snprintf (ptr, 7, "%d ", 1 << i->log_sector_size);
    ++	ptr += grub_snprintf (ptr, 7, "%u ", 1 << i->log_sector_size);
      	for (iptr = i->cipher->cipher->name; *iptr; iptr++)
      	  *ptr++ = grub_tolower (*iptr);
      	switch (i->mode)

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

diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 34b67a705fbc..7f3170abc887 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 16 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;
 	if (i->essiv_hash)
 	  size += grub_strlen (i->essiv_hash->name);
 	size += i->keysize * 2;
@@ -1496,16 +1506,18 @@ 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++ = ' ';
-	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);
 	for (iptr = i->cipher->cipher->name; *iptr; iptr++)
 	  *ptr++ = grub_tolower (*iptr);
 	switch (i->mode)
-- 
2.34.1

Re: [PATCH v2] luks2: Add support for LUKS2 in (proc)/luks_script

[Daniel Kiper]

On Mon, Jul 10, 2023 at 03:20:48PM -0500, Glenn Washburn wrote:
> The sector size in bytes is added to each line and it is allowed to be 5
> 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.

You say "1 space added" here...

> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
> Range-diff against v1:
> 1:  14160a8991f6 ! 1:  dbf695eef1ab luks2: Add support for LUKS2 in (proc)/luks_script
>     @@ grub-core/disk/cryptodisk.c: luks_script_get (grub_size_t *sz)
>      -	grub_snprintf (ptr, 21, "%" PRIuGRUB_UINT64_T " ", i->offset_sectors);
>      -	while (*ptr)
>      -	  ptr++;
>     -+	ptr += grub_snprintf (ptr, 21, "%" PRIuGRUB_UINT64_T " ",
>     ++	ptr += grub_snprintf (ptr, 21, "%" PRIxGRUB_OFFSET " ",
>      +			      i->offset_sectors);
>     -+	ptr += grub_snprintf (ptr, 7, "%d ", 1 << i->log_sector_size);
>     ++	ptr += grub_snprintf (ptr, 7, "%u ", 1 << i->log_sector_size);
>       	for (iptr = i->cipher->cipher->name; *iptr; iptr++)
>       	  *ptr++ = grub_tolower (*iptr);
>       	switch (i->mode)
>
>  grub-core/disk/cryptodisk.c | 28 ++++++++++++++++++++--------
>  1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 34b67a705fbc..7f3170abc887 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

... and "trailing newline" here. Something is off...

> +	 * an upper bound on the size of this data. There are 16 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;
>  	if (i->essiv_hash)
>  	  size += grub_strlen (i->essiv_hash->name);
>  	size += i->keysize * 2;
> @@ -1496,16 +1506,18 @@ 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++ = ' ';
> -	grub_snprintf (ptr, 21, "%" PRIuGRUB_UINT64_T " ", i->offset_sectors);
> -	while (*ptr)
> -	  ptr++;
> +	ptr += grub_snprintf (ptr, 21, "%" PRIxGRUB_OFFSET " ",
> +			      i->offset_sectors);

I think this change begs for separate patch. Or at least explanation in
the commit message. And you do not need wrap the line here...

Daniel