[PATCH] error: Do compile-time format string checking on grub_error.

[PATCH] error: Do compile-time format string checking on grub_error.

[Glenn Washburn]

Fix the many issues this uncovered mostly related to incorrect length
modifiers.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/commands/pgp.c           | 2 +-
 grub-core/disk/ata.c               | 4 ++--
 grub-core/disk/cryptodisk.c        | 8 ++++----
 grub-core/disk/dmraid_nvidia.c     | 2 +-
 grub-core/fs/hfsplus.c             | 2 +-
 grub-core/fs/zfs/zfs.c             | 2 +-
 grub-core/kern/efi/efi.c           | 2 +-
 grub-core/kern/efi/mm.c            | 2 +-
 grub-core/kern/x86_64/dl.c         | 2 +-
 grub-core/loader/efi/chainloader.c | 4 ++--
 grub-core/loader/i386/bsd.c        | 2 +-
 grub-core/loader/i386/pc/linux.c   | 4 ++--
 grub-core/net/tftp.c               | 2 +-
 grub-core/parttool/msdospart.c     | 4 ++--
 grub-core/script/lexer.c           | 2 +-
 grub-core/video/bochs.c            | 4 ++--
 include/grub/err.h                 | 3 ++-
 17 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/grub-core/commands/pgp.c b/grub-core/commands/pgp.c
index bbf6871fe..7c2fef6d0 100644
--- a/grub-core/commands/pgp.c
+++ b/grub-core/commands/pgp.c
@@ -633,7 +633,7 @@ grub_verify_signature_real (struct grub_pubkey_context *ctxt,
   if (!sk)
     {
       /* TRANSLATORS: %08x is 32-bit key id.  */
-      grub_error (GRUB_ERR_BAD_SIGNATURE, N_("public key %08x not found"),
+      grub_error (GRUB_ERR_BAD_SIGNATURE, N_("public key %08lx not found"),
 		  keyid);
       goto fail;
     }
diff --git a/grub-core/disk/ata.c b/grub-core/disk/ata.c
index 685f33a19..f401a27cf 100644
--- a/grub-core/disk/ata.c
+++ b/grub-core/disk/ata.c
@@ -219,7 +219,7 @@ grub_ata_setaddress (struct grub_ata *dev,
 	if (dev->sectors_per_track == 0
 	    || dev->heads == 0)
 	  return grub_error (GRUB_ERR_OUT_OF_RANGE,
-			     "sector %d cannot be addressed "
+			     "sector %ld cannot be addressed "
 			     "using CHS addressing", sector);
 
 	/* Calculate the sector, cylinder and head to use.  */
@@ -232,7 +232,7 @@ grub_ata_setaddress (struct grub_ata *dev,
 	    || cylinder > dev->cylinders
 	    || head > dev->heads)
 	  return grub_error (GRUB_ERR_OUT_OF_RANGE,
-			     "sector %d cannot be addressed "
+			     "sector %ld cannot be addressed "
 			     "using CHS addressing", sector);
 	
 	parms->taskfile.disk = 0xE0 | head;
diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 4921d031d..9d2f0d635 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -476,13 +476,13 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const char *ciphername, cons
       }
       if (cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
 	{
-	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported XTS block size: %d",
+	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported XTS block size: %ld",
 			    cipher->cipher->blocksize);
 	  goto err;
 	}
       if (secondary_cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
 	{
-	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported XTS block size: %d",
+	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported XTS block size: %ld",
 			    secondary_cipher->cipher->blocksize);
 	  goto err;
 	}
@@ -493,7 +493,7 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const char *ciphername, cons
       cipheriv = ciphermode + sizeof ("lrw-") - 1;
       if (cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
 	{
-	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported LRW block size: %d",
+	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported LRW block size: %ld",
 			    cipher->cipher->blocksize);
 	  goto err;
 	}
@@ -515,7 +515,7 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const char *ciphername, cons
     {
       if (cipher->cipher->blocksize & (cipher->cipher->blocksize - 1)
 	  || cipher->cipher->blocksize == 0)
-	grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported benbi blocksize: %d",
+	grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported benbi blocksize: %ld",
 		    cipher->cipher->blocksize);
 	/* FIXME should we return an error here? */
       for (benbi_log = 0;
diff --git a/grub-core/disk/dmraid_nvidia.c b/grub-core/disk/dmraid_nvidia.c
index 060279124..014581df0 100644
--- a/grub-core/disk/dmraid_nvidia.c
+++ b/grub-core/disk/dmraid_nvidia.c
@@ -122,7 +122,7 @@ grub_dmraid_nv_detect (grub_disk_t disk,
   if (sb.version != NV_VERSION)
     {
       grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-		  "unknown version: %d.%d", sb.version);
+		  "unknown version: %d", sb.version);
       return NULL;
     }
 
diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
index 9c4e4c88c..112334504 100644
--- a/grub-core/fs/hfsplus.c
+++ b/grub-core/fs/hfsplus.c
@@ -188,7 +188,7 @@ grub_hfsplus_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
 	  || !nnode)
 	{
 	  grub_error (GRUB_ERR_READ_ERROR,
-		      "no block found for the file id 0x%x and the block offset 0x%x",
+		      "no block found for the file id 0x%x and the block offset 0x%lx",
 		      node->fileid, fileblock);
 	  break;
 	}
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index 41ef0ff57..9ad26e876 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -1869,7 +1869,7 @@ zio_read (blkptr_t *bp, grub_zfs_endian_t endian, void **buf,
     {
       if (BPE_GET_ETYPE(bp) != BP_EMBEDDED_TYPE_DATA)
 	return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-			   "unsupported embedded BP (type=%u)\n",
+			   "unsupported embedded BP (type=%llu)\n",
 			   BPE_GET_ETYPE(bp));
       lsize = BPE_GET_LSIZE(bp);
       psize = BF64_GET_SB(grub_zfs_to_cpu64 ((bp)->blk_prop, endian), 25, 7, 0, 1);
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index e0165e74c..a56be538c 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -491,7 +491,7 @@ grub_efi_duplicate_device_path (const grub_efi_device_path_t *dp)
       if (len < 4)
 	{
 	  grub_error (GRUB_ERR_OUT_OF_RANGE,
-		      "malformed EFI Device Path node has length=%d", len);
+		      "malformed EFI Device Path node has length=%ld", len);
 	  return NULL;
 	}
 
diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index 457772d57..22f89b6af 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -127,7 +127,7 @@ grub_efi_allocate_pages_real (grub_efi_physical_address_t address,
   if (address > GRUB_EFI_MAX_USABLE_ADDRESS)
     {
       grub_error (GRUB_ERR_BAD_ARGUMENT,
-		  N_("invalid memory address (0x%llx > 0x%llx)"),
+		  N_("invalid memory address (0x%lx > 0x%x)"),
 		  address, GRUB_EFI_MAX_USABLE_ADDRESS);
       return NULL;
     }
diff --git a/grub-core/kern/x86_64/dl.c b/grub-core/kern/x86_64/dl.c
index 3a73e6e6c..aa01cc167 100644
--- a/grub-core/kern/x86_64/dl.c
+++ b/grub-core/kern/x86_64/dl.c
@@ -107,7 +107,7 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
 
 	default:
 	  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-			     N_("relocation 0x%x is not implemented yet"),
+			     N_("relocation 0x%lx is not implemented yet"),
 			     ELF_R_TYPE (rel->r_info));
 	}
     }
diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c
index 7b31c3fb9..eab78269c 100644
--- a/grub-core/loader/efi/chainloader.c
+++ b/grub-core/loader/efi/chainloader.c
@@ -90,7 +90,7 @@ grub_chainloader_boot (void)
 	      *grub_utf16_to_utf8 ((grub_uint8_t *) buf,
 				   exit_data, exit_data_size) = 0;
 
-	      grub_error (GRUB_ERR_BAD_OS, buf);
+	      grub_error (GRUB_ERR_BAD_OS, "%s", buf);
 	      grub_free (buf);
 	    }
 	}
@@ -164,7 +164,7 @@ make_file_path (grub_efi_device_path_t *dp, const char *filename)
       if (len < 4)
 	{
 	  grub_error (GRUB_ERR_OUT_OF_RANGE,
-		      "malformed EFI Device Path node has length=%d", len);
+		      "malformed EFI Device Path node has length=%ld", len);
 	  return NULL;
 	}
 
diff --git a/grub-core/loader/i386/bsd.c b/grub-core/loader/i386/bsd.c
index f5bf7f89e..9d00884ae 100644
--- a/grub-core/loader/i386/bsd.c
+++ b/grub-core/loader/i386/bsd.c
@@ -2110,7 +2110,7 @@ grub_cmd_openbsd_ramdisk (grub_command_t cmd __attribute__ ((unused)),
     {
       grub_file_close (file);
       return grub_error (GRUB_ERR_BAD_OS, "your kOpenBSD supports ramdisk only"
-			 " up to %u bytes, however you supplied a %u bytes one",
+			 " up to %lu bytes, however you supplied a %lu bytes one",
 			 openbsd_ramdisk.max_size, size);
     }
 
diff --git a/grub-core/loader/i386/pc/linux.c b/grub-core/loader/i386/pc/linux.c
index 814988ab9..0bc5d6807 100644
--- a/grub-core/loader/i386/pc/linux.c
+++ b/grub-core/loader/i386/pc/linux.c
@@ -230,9 +230,9 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
       && GRUB_LINUX_ZIMAGE_ADDR + grub_linux16_prot_size
       > grub_linux_real_target)
     {
-      grub_error (GRUB_ERR_BAD_OS, "too big zImage (0x%x > 0x%x), use bzImage instead",
+      grub_error (GRUB_ERR_BAD_OS, "too big zImage (%p > %p), use bzImage instead",
 		  (char *) GRUB_LINUX_ZIMAGE_ADDR + grub_linux16_prot_size,
-		  (grub_size_t) grub_linux_real_target);
+		  (void *) grub_linux_real_target);
       goto fail;
     }
 
diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c
index 644135caf..dd2462bb0 100644
--- a/grub-core/net/tftp.c
+++ b/grub-core/net/tftp.c
@@ -241,7 +241,7 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)),
     case TFTP_ERROR:
       data->have_oack = 1;
       grub_netbuff_free (nb);
-      grub_error (GRUB_ERR_IO, (char *) tftph->u.err.errmsg);
+      grub_error (GRUB_ERR_IO, "%s", tftph->u.err.errmsg);
       grub_error_save (&data->save_err);
       return GRUB_ERR_NONE;
     default:
diff --git a/grub-core/parttool/msdospart.c b/grub-core/parttool/msdospart.c
index dcbf74e3b..2e2e9d527 100644
--- a/grub-core/parttool/msdospart.c
+++ b/grub-core/parttool/msdospart.c
@@ -127,8 +127,8 @@ static grub_err_t grub_pcpart_type (const grub_device_t dev,
     {
       dev->disk->partition = part;
       return grub_error (GRUB_ERR_BAD_ARGUMENT,
-			 N_("the partition type 0x%x isn't "
-			    "valid"));
+			 N_("the partition type 0x%x isn't valid"),
+			    type);
     }
 
   mbr.entries[index].type = type;
diff --git a/grub-core/script/lexer.c b/grub-core/script/lexer.c
index 5fb0cbd0b..27daad791 100644
--- a/grub-core/script/lexer.c
+++ b/grub-core/script/lexer.c
@@ -349,7 +349,7 @@ void
 grub_script_yyerror (struct grub_parser_param *state, char const *err)
 {
   if (err)
-    grub_error (GRUB_ERR_INVALID_COMMAND, err);
+    grub_error (GRUB_ERR_INVALID_COMMAND, "%s", err);
 
   grub_print_error ();
   state->err++;
diff --git a/grub-core/video/bochs.c b/grub-core/video/bochs.c
index 3bcfa53a9..6d640c0e2 100644
--- a/grub-core/video/bochs.c
+++ b/grub-core/video/bochs.c
@@ -249,11 +249,11 @@ grub_video_bochs_setup (unsigned int width, unsigned int height,
     }
 
   if (width > BOCHS_MAX_WIDTH)
-    return grub_error (GRUB_ERR_IO, "width must be at most",
+    return grub_error (GRUB_ERR_IO, "width must be at most %d",
 		       BOCHS_MAX_WIDTH);
 
   if (height > BOCHS_MAX_HEIGHT)
-    return grub_error (GRUB_ERR_IO, "height must be at most",
+    return grub_error (GRUB_ERR_IO, "height must be at most %d",
 		       BOCHS_MAX_HEIGHT);
 
   if (width & (BOCHS_WIDTH_ALIGN - 1))
diff --git a/include/grub/err.h b/include/grub/err.h
index 24ba9f5f5..b08d5d0de 100644
--- a/include/grub/err.h
+++ b/include/grub/err.h
@@ -85,7 +85,8 @@ struct grub_error_saved
 extern grub_err_t EXPORT_VAR(grub_errno);
 extern char EXPORT_VAR(grub_errmsg)[GRUB_MAX_ERRMSG];
 
-grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *fmt, ...);
+grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *fmt, ...)
+    __attribute__ ((format (GNU_PRINTF, 2, 3)));
 void EXPORT_FUNC(grub_fatal) (const char *fmt, ...) __attribute__ ((noreturn));
 void EXPORT_FUNC(grub_error_push) (void);
 int EXPORT_FUNC(grub_error_pop) (void);
-- 
2.25.1

[PATCH] cryptodisk: Use cipher name instead of object in error message.

[Glenn Washburn]

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

diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 9d2f0d635..4a35c0478 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -470,8 +470,8 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const char *ciphername, cons
       secondary_cipher = grub_crypto_cipher_open (ciph);
       if (!secondary_cipher)
       {
-	  ret = grub_error (GRUB_ERR_FILE_NOT_FOUND, "Secondary cipher %s isn't available",
-			    secondary_cipher);
+	  ret = grub_error (GRUB_ERR_FILE_NOT_FOUND,
+			    "Secondary cipher %s isn't available", ciphername);
 	  goto err;
       }
       if (cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
-- 
2.25.1

Re: [PATCH] cryptodisk: Use cipher name instead of object in error message.

[Daniel Kiper]

On Fri, Jul 31, 2020 at 09:33:19AM -0500, Glenn Washburn wrote:
> Signed-off-by: Glenn Washburn <development@efficientek.com>

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

Daniel

[PATCH] lexer: char const * should be const char *.

[Glenn Washburn]

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/script/lexer.c | 2 +-
 include/grub/script_sh.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/grub-core/script/lexer.c b/grub-core/script/lexer.c
index 27daad791..52004d059 100644
--- a/grub-core/script/lexer.c
+++ b/grub-core/script/lexer.c
@@ -346,7 +346,7 @@ grub_script_yylex (union YYSTYPE *value,
 }
 
 void
-grub_script_yyerror (struct grub_parser_param *state, char const *err)
+grub_script_yyerror (struct grub_parser_param *state, const char *err)
 {
   if (err)
     grub_error (GRUB_ERR_INVALID_COMMAND, "%s", err);
diff --git a/include/grub/script_sh.h b/include/grub/script_sh.h
index 6c48e0751..e5b4d8450 100644
--- a/include/grub/script_sh.h
+++ b/include/grub/script_sh.h
@@ -318,7 +318,7 @@ void *grub_script_malloc (struct grub_parser_param *state, grub_size_t size);
 union YYSTYPE;
 int grub_script_yylex (union YYSTYPE *, struct grub_parser_param *);
 int grub_script_yyparse (struct grub_parser_param *);
-void grub_script_yyerror (struct grub_parser_param *, char const *);
+void grub_script_yyerror (struct grub_parser_param *, const char *);
 
 /* Commands to execute, don't use these directly.  */
 grub_err_t grub_script_execute_cmdline (struct grub_script_cmd *cmd);
-- 
2.25.1

Re: [PATCH] lexer: char const * should be const char *.

[Daniel Kiper]

On Fri, Jul 31, 2020 at 09:33:20AM -0500, Glenn Washburn wrote:
> Signed-off-by: Glenn Washburn <development@efficientek.com>

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

Daniel

[PATCH] docs/grub: Support for loading and concatenating multiple initrds.

[Glenn Washburn]

This has been available since January of 2012, but has not been documented.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 docs/grub.texi | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index 1ce9993a5..935aa1bf3 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4498,22 +4498,22 @@ about each of the commands whose names begin with those @var{patterns}.
 @node initrd
 @subsection initrd
 
-@deffn Command initrd file
-Load an initial ramdisk for a Linux kernel image, and set the appropriate
-parameters in the Linux setup area in memory.  This may only be used after
-the @command{linux} command (@pxref{linux}) has been run.  See also
-@ref{GNU/Linux}.
+@deffn Command initrd file [file @dots{}]
+Load, in order, all initial ramdisks for a Linux kernel image, and set
+the appropriate parameters in the Linux setup area in memory.  This may only
+be used after the @command{linux} command (@pxref{linux}) has been run.  See
+also @ref{GNU/Linux}.
 @end deffn
 
 
 @node initrd16
 @subsection initrd16
 
-@deffn Command initrd16 file
-Load an initial ramdisk for a Linux kernel image to be booted in 16-bit
-mode, and set the appropriate parameters in the Linux setup area in memory.
-This may only be used after the @command{linux16} command (@pxref{linux16})
-has been run.  See also @ref{GNU/Linux}.
+@deffn Command initrd16 file [file @dots{}]
+Load, in order, all initial ramdisks for a Linux kernel image to be booted in
+16-bit mode, and set the appropriate parameters in the Linux setup area in
+memory.  This may only be used after the @command{linux16} command
+(@pxref{linux16}) has been run.  See also @ref{GNU/Linux}.
 
 This command is only available on x86 systems.
 @end deffn
-- 
2.25.1

Re: [PATCH] docs/grub: Support for loading and concatenating multiple initrds.

[Daniel Kiper]

On Fri, Jul 31, 2020 at 09:33:21AM -0500, Glenn Washburn wrote:
> This has been available since January of 2012, but has not been documented.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

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

Daniel

[PATCH] script: Do not allow a delimiter between function name and block start.

[Glenn Washburn]

Currently the following is valid syntax, but should be a syntax error:

grub> function f; { echo HERE; }
grub> f
HERE

This fix is not backward compatible, but current syntax is not documented
either and has no functional value. So any scripts with this unintended
syntax are technically syntactically incorrect and should not be relying
on this behavior.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/script/parser.y | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/script/parser.y b/grub-core/script/parser.y
index f80b86b6f..4a18ab7ba 100644
--- a/grub-core/script/parser.y
+++ b/grub-core/script/parser.y
@@ -279,7 +279,7 @@ function: "function" "name"
 	    $<scripts>$ = state->scripts;
 	    state->scripts = 0;
           }
-          delimiters0 "{" commands1 delimiters1 "}"
+          newlines0 "{" commands1 delimiters1 "}"
           {
             struct grub_script *script;
             state->func_mem = grub_script_mem_record_stop (state,
-- 
2.25.1

Re: [PATCH] script: Do not allow a delimiter between function name and block start.

[Daniel Kiper]

On Fri, Jul 31, 2020 at 09:33:22AM -0500, Glenn Washburn wrote:
> Currently the following is valid syntax, but should be a syntax error:
>
> grub> function f; { echo HERE; }
> grub> f
> HERE
>
> This fix is not backward compatible, but current syntax is not documented
> either and has no functional value. So any scripts with this unintended
> syntax are technically syntactically incorrect and should not be relying
> on this behavior.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

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

Daniel

[PATCH] crypto: Remove GPG_ERROR_CFLAGS from gpg_err_code_t enum.

[Glenn Washburn]

This was probably added by accident when originally creating the file.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 include/grub/crypto.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/grub/crypto.h b/include/grub/crypto.h
index a24e89dd9..21cd1f75a 100644
--- a/include/grub/crypto.h
+++ b/include/grub/crypto.h
@@ -56,7 +56,6 @@ typedef enum
     GPG_ERR_NOT_FOUND,
     GPG_ERR_NOT_IMPLEMENTED,
     GPG_ERR_NOT_SUPPORTED,
-    GPG_ERROR_CFLAGS,
     GPG_ERR_PUBKEY_ALGO,
     GPG_ERR_SELFTEST_FAILED,
     GPG_ERR_TOO_SHORT,
-- 
2.25.1

Re: [PATCH] crypto: Remove GPG_ERROR_CFLAGS from gpg_err_code_t enum.

[Daniel Kiper]

On Fri, Jul 31, 2020 at 09:33:23AM -0500, Glenn Washburn wrote:
> This was probably added by accident when originally creating the file.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

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

Daniel

Re: [PATCH] error: Do compile-time format string checking on grub_error.

[Daniel Kiper]

On Fri, Jul 31, 2020 at 09:33:18AM -0500, Glenn Washburn wrote:
> Fix the many issues this uncovered mostly related to incorrect length
> modifiers.

Did you test the 32-bit and 64-bit builds with this patch?

Daniel

Re: [PATCH] error: Do compile-time format string checking on grub_error.

[Glenn Washburn]

On Thu, 17 Sep 2020 16:29:23 +0200
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Fri, Jul 31, 2020 at 09:33:18AM -0500, Glenn Washburn wrote:
> > Fix the many issues this uncovered mostly related to incorrect
> > length modifiers.
> 
> Did you test the 32-bit and 64-bit builds with this patch?
> 
> Daniel

I have tested that this patch does indeed compile on i386-pc and
x86_64-efi. Is that what you're wondering?  Is the patch not compiling
for you?

You can verify that these are all issues by solely applying the patch
to include/grub/err.h and building under a variety of target arches and
platforms.

Glenn

Re: [PATCH] error: Do compile-time format string checking on grub_error.

[Glenn Washburn]

On Sat, 3 Oct 2020 17:34:29 -0500
Glenn Washburn <development@efficientek.com> wrote:

> On Thu, 17 Sep 2020 16:29:23 +0200
> Daniel Kiper <dkiper@net-space.pl> wrote:
> 
> > On Fri, Jul 31, 2020 at 09:33:18AM -0500, Glenn Washburn wrote:
> > > Fix the many issues this uncovered mostly related to incorrect
> > > length modifiers.
> > 
> > Did you test the 32-bit and 64-bit builds with this patch?
> > 
> > Daniel
> 
> I have tested that this patch does indeed compile on i386-pc and
> x86_64-efi. Is that what you're wondering?  Is the patch not compiling
> for you?
> 
> You can verify that these are all issues by solely applying the patch
> to include/grub/err.h and building under a variety of target arches
> and platforms.

I notice now that both 32-bit and 64-bit builds will fail with this
patch applied to master.  This patch assumes some code changes in my
patch titled "luks2: Fix use of incorrect index and some grub_error()
messages." that has been sent to this list and changes from another
patch I forgot to include.  I'll resend the patch with the changes from
the missing patch, but still require changes from the previously
mentioned patch sent to the list.

Glenn

[PATCH] error: Do compile-time format string checking on grub_error.

[Glenn Washburn]

Fix the many issues this uncovered mostly related to incorrect length
modifiers.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/commands/pgp.c           |  2 +-
 grub-core/disk/ata.c               |  4 ++--
 grub-core/disk/cryptodisk.c        | 14 ++++++++------
 grub-core/disk/dmraid_nvidia.c     |  2 +-
 grub-core/disk/luks2.c             |  6 +++---
 grub-core/efiemu/i386/loadcore64.c |  3 ++-
 grub-core/fs/hfsplus.c             |  3 ++-
 grub-core/fs/zfs/zfs.c             |  2 +-
 grub-core/kern/efi/efi.c           |  2 +-
 grub-core/kern/efi/mm.c            |  2 +-
 grub-core/kern/x86_64/dl.c         |  2 +-
 grub-core/loader/efi/chainloader.c |  4 ++--
 grub-core/loader/i386/bsd.c        |  3 ++-
 grub-core/loader/i386/pc/linux.c   |  4 ++--
 grub-core/net/tftp.c               |  2 +-
 grub-core/parttool/msdospart.c     |  4 ++--
 grub-core/script/lexer.c           |  2 +-
 grub-core/video/bochs.c            |  4 ++--
 include/grub/err.h                 |  3 ++-
 19 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/grub-core/commands/pgp.c b/grub-core/commands/pgp.c
index bbf6871fe..150cb6fe9 100644
--- a/grub-core/commands/pgp.c
+++ b/grub-core/commands/pgp.c
@@ -633,7 +633,7 @@ grub_verify_signature_real (struct grub_pubkey_context *ctxt,
   if (!sk)
     {
       /* TRANSLATORS: %08x is 32-bit key id.  */
-      grub_error (GRUB_ERR_BAD_SIGNATURE, N_("public key %08x not found"),
+      grub_error (GRUB_ERR_BAD_SIGNATURE, N_("public key %08"PRIxGRUB_UINT64_T" not found"),
 		  keyid);
       goto fail;
     }
diff --git a/grub-core/disk/ata.c b/grub-core/disk/ata.c
index 685f33a19..c8f350ed3 100644
--- a/grub-core/disk/ata.c
+++ b/grub-core/disk/ata.c
@@ -219,7 +219,7 @@ grub_ata_setaddress (struct grub_ata *dev,
 	if (dev->sectors_per_track == 0
 	    || dev->heads == 0)
 	  return grub_error (GRUB_ERR_OUT_OF_RANGE,
-			     "sector %d cannot be addressed "
+			     "sector %"PRIxGRUB_UINT64_T" cannot be addressed "
 			     "using CHS addressing", sector);
 
 	/* Calculate the sector, cylinder and head to use.  */
@@ -232,7 +232,7 @@ grub_ata_setaddress (struct grub_ata *dev,
 	    || cylinder > dev->cylinders
 	    || head > dev->heads)
 	  return grub_error (GRUB_ERR_OUT_OF_RANGE,
-			     "sector %d cannot be addressed "
+			     "sector %"PRIxGRUB_UINT64_T" cannot be addressed "
 			     "using CHS addressing", sector);
 	
 	parms->taskfile.disk = 0xE0 | head;
diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 13af84dd1..9c043a23f 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -470,13 +470,15 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const char *ciphername, cons
       }
       if (cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
 	{
-	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported XTS block size: %d",
+	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
+			    "Unsupported XTS block size: %"PRIuGRUB_SIZE,
 			    cipher->cipher->blocksize);
 	  goto err;
 	}
       if (secondary_cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
 	{
-	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported XTS block size: %d",
+	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
+			    "Unsupported XTS block size: %"PRIuGRUB_SIZE,
 			    secondary_cipher->cipher->blocksize);
 	  goto err;
 	}
@@ -487,8 +489,8 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const char *ciphername, cons
       cipheriv = ciphermode + sizeof ("lrw-") - 1;
       if (cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
 	{
-	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported LRW block size: %d",
-			    cipher->cipher->blocksize);
+	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported LRW block size: %"
+			    PRIuGRUB_SIZE, cipher->cipher->blocksize);
 	  goto err;
 	}
     }
@@ -509,8 +511,8 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const char *ciphername, cons
     {
       if (cipher->cipher->blocksize & (cipher->cipher->blocksize - 1)
 	  || cipher->cipher->blocksize == 0)
-	grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported benbi blocksize: %d",
-		    cipher->cipher->blocksize);
+	grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported benbi blocksize: %"
+		    PRIuGRUB_SIZE, cipher->cipher->blocksize);
 	/* FIXME should we return an error here? */
       for (benbi_log = 0;
 	   (cipher->cipher->blocksize << benbi_log) < GRUB_DISK_SECTOR_SIZE;
diff --git a/grub-core/disk/dmraid_nvidia.c b/grub-core/disk/dmraid_nvidia.c
index 060279124..014581df0 100644
--- a/grub-core/disk/dmraid_nvidia.c
+++ b/grub-core/disk/dmraid_nvidia.c
@@ -122,7 +122,7 @@ grub_dmraid_nv_detect (grub_disk_t disk,
   if (sb.version != NV_VERSION)
     {
       grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-		  "unknown version: %d.%d", sb.version);
+		  "unknown version: %d", sb.version);
       return NULL;
     }
 
diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index 31d7166fc..fdf6cd1e7 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++)
     {
diff --git a/grub-core/efiemu/i386/loadcore64.c b/grub-core/efiemu/i386/loadcore64.c
index 18facf47f..3e9a71cfd 100644
--- a/grub-core/efiemu/i386/loadcore64.c
+++ b/grub-core/efiemu/i386/loadcore64.c
@@ -122,7 +122,8 @@ grub_arch_efiemu_relocate_symbols64 (grub_efiemu_segment_t segs,
                     break;
 		  default:
 		    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-				       N_("relocation 0x%x is not implemented yet"),
+				       N_("relocation 0x%"PRIxGRUB_UINT64_T
+					  " is not implemented yet"),
 				       ELF_R_TYPE (rel->r_info));
 		  }
 	      }
diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
index 9c4e4c88c..b538f52e0 100644
--- a/grub-core/fs/hfsplus.c
+++ b/grub-core/fs/hfsplus.c
@@ -188,7 +188,8 @@ grub_hfsplus_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
 	  || !nnode)
 	{
 	  grub_error (GRUB_ERR_READ_ERROR,
-		      "no block found for the file id 0x%x and the block offset 0x%x",
+		      "no block found for the file id 0x%x and the block"
+		      " offset 0x%"PRIuGRUB_UINT64_T,
 		      node->fileid, fileblock);
 	  break;
 	}
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index 41ef0ff57..9ad26e876 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -1869,7 +1869,7 @@ zio_read (blkptr_t *bp, grub_zfs_endian_t endian, void **buf,
     {
       if (BPE_GET_ETYPE(bp) != BP_EMBEDDED_TYPE_DATA)
 	return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-			   "unsupported embedded BP (type=%u)\n",
+			   "unsupported embedded BP (type=%llu)\n",
 			   BPE_GET_ETYPE(bp));
       lsize = BPE_GET_LSIZE(bp);
       psize = BF64_GET_SB(grub_zfs_to_cpu64 ((bp)->blk_prop, endian), 25, 7, 0, 1);
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index e0165e74c..a56be538c 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -491,7 +491,7 @@ grub_efi_duplicate_device_path (const grub_efi_device_path_t *dp)
       if (len < 4)
 	{
 	  grub_error (GRUB_ERR_OUT_OF_RANGE,
-		      "malformed EFI Device Path node has length=%d", len);
+		      "malformed EFI Device Path node has length=%ld", len);
 	  return NULL;
 	}
 
diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index 457772d57..22f89b6af 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -127,7 +127,7 @@ grub_efi_allocate_pages_real (grub_efi_physical_address_t address,
   if (address > GRUB_EFI_MAX_USABLE_ADDRESS)
     {
       grub_error (GRUB_ERR_BAD_ARGUMENT,
-		  N_("invalid memory address (0x%llx > 0x%llx)"),
+		  N_("invalid memory address (0x%lx > 0x%x)"),
 		  address, GRUB_EFI_MAX_USABLE_ADDRESS);
       return NULL;
     }
diff --git a/grub-core/kern/x86_64/dl.c b/grub-core/kern/x86_64/dl.c
index 3a73e6e6c..aa01cc167 100644
--- a/grub-core/kern/x86_64/dl.c
+++ b/grub-core/kern/x86_64/dl.c
@@ -107,7 +107,7 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
 
 	default:
 	  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-			     N_("relocation 0x%x is not implemented yet"),
+			     N_("relocation 0x%lx is not implemented yet"),
 			     ELF_R_TYPE (rel->r_info));
 	}
     }
diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c
index 7b31c3fb9..b8d35f41e 100644
--- a/grub-core/loader/efi/chainloader.c
+++ b/grub-core/loader/efi/chainloader.c
@@ -90,7 +90,7 @@ grub_chainloader_boot (void)
 	      *grub_utf16_to_utf8 ((grub_uint8_t *) buf,
 				   exit_data, exit_data_size) = 0;
 
-	      grub_error (GRUB_ERR_BAD_OS, buf);
+	      grub_error (GRUB_ERR_BAD_OS, "%s", buf);
 	      grub_free (buf);
 	    }
 	}
@@ -164,7 +164,7 @@ make_file_path (grub_efi_device_path_t *dp, const char *filename)
       if (len < 4)
 	{
 	  grub_error (GRUB_ERR_OUT_OF_RANGE,
-		      "malformed EFI Device Path node has length=%d", len);
+		      "malformed EFI Device Path node has length=%"PRIuGRUB_SIZE, len);
 	  return NULL;
 	}
 
diff --git a/grub-core/loader/i386/bsd.c b/grub-core/loader/i386/bsd.c
index f5bf7f89e..fe20b339b 100644
--- a/grub-core/loader/i386/bsd.c
+++ b/grub-core/loader/i386/bsd.c
@@ -2110,7 +2110,8 @@ grub_cmd_openbsd_ramdisk (grub_command_t cmd __attribute__ ((unused)),
     {
       grub_file_close (file);
       return grub_error (GRUB_ERR_BAD_OS, "your kOpenBSD supports ramdisk only"
-			 " up to %u bytes, however you supplied a %u bytes one",
+			 " up to %"PRIuGRUB_SIZE" bytes, however you supplied"
+			 " a %"PRIuGRUB_SIZE" bytes one",
 			 openbsd_ramdisk.max_size, size);
     }
 
diff --git a/grub-core/loader/i386/pc/linux.c b/grub-core/loader/i386/pc/linux.c
index 814988ab9..0bc5d6807 100644
--- a/grub-core/loader/i386/pc/linux.c
+++ b/grub-core/loader/i386/pc/linux.c
@@ -230,9 +230,9 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
       && GRUB_LINUX_ZIMAGE_ADDR + grub_linux16_prot_size
       > grub_linux_real_target)
     {
-      grub_error (GRUB_ERR_BAD_OS, "too big zImage (0x%x > 0x%x), use bzImage instead",
+      grub_error (GRUB_ERR_BAD_OS, "too big zImage (%p > %p), use bzImage instead",
 		  (char *) GRUB_LINUX_ZIMAGE_ADDR + grub_linux16_prot_size,
-		  (grub_size_t) grub_linux_real_target);
+		  (void *) grub_linux_real_target);
       goto fail;
     }
 
diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c
index 7e659a78b..592bfee34 100644
--- a/grub-core/net/tftp.c
+++ b/grub-core/net/tftp.c
@@ -252,7 +252,7 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)),
     case TFTP_ERROR:
       data->have_oack = 1;
       grub_netbuff_free (nb);
-      grub_error (GRUB_ERR_IO, (char *) tftph->u.err.errmsg);
+      grub_error (GRUB_ERR_IO, "%s", tftph->u.err.errmsg);
       grub_error_save (&data->save_err);
       return GRUB_ERR_NONE;
     default:
diff --git a/grub-core/parttool/msdospart.c b/grub-core/parttool/msdospart.c
index dcbf74e3b..2e2e9d527 100644
--- a/grub-core/parttool/msdospart.c
+++ b/grub-core/parttool/msdospart.c
@@ -127,8 +127,8 @@ static grub_err_t grub_pcpart_type (const grub_device_t dev,
     {
       dev->disk->partition = part;
       return grub_error (GRUB_ERR_BAD_ARGUMENT,
-			 N_("the partition type 0x%x isn't "
-			    "valid"));
+			 N_("the partition type 0x%x isn't valid"),
+			    type);
     }
 
   mbr.entries[index].type = type;
diff --git a/grub-core/script/lexer.c b/grub-core/script/lexer.c
index 57778f881..52004d059 100644
--- a/grub-core/script/lexer.c
+++ b/grub-core/script/lexer.c
@@ -349,7 +349,7 @@ void
 grub_script_yyerror (struct grub_parser_param *state, const char *err)
 {
   if (err)
-    grub_error (GRUB_ERR_INVALID_COMMAND, err);
+    grub_error (GRUB_ERR_INVALID_COMMAND, "%s", err);
 
   grub_print_error ();
   state->err++;
diff --git a/grub-core/video/bochs.c b/grub-core/video/bochs.c
index 7a249eb21..30ea1bd82 100644
--- a/grub-core/video/bochs.c
+++ b/grub-core/video/bochs.c
@@ -249,11 +249,11 @@ grub_video_bochs_setup (unsigned int width, unsigned int height,
     }
 
   if (width > BOCHS_MAX_WIDTH)
-    return grub_error (GRUB_ERR_IO, "width must be at most",
+    return grub_error (GRUB_ERR_IO, "width must be at most %d",
 		       BOCHS_MAX_WIDTH);
 
   if (height > BOCHS_MAX_HEIGHT)
-    return grub_error (GRUB_ERR_IO, "height must be at most",
+    return grub_error (GRUB_ERR_IO, "height must be at most %d",
 		       BOCHS_MAX_HEIGHT);
 
   if (width & (BOCHS_WIDTH_ALIGN - 1))
diff --git a/include/grub/err.h b/include/grub/err.h
index 24ba9f5f5..b08d5d0de 100644
--- a/include/grub/err.h
+++ b/include/grub/err.h
@@ -85,7 +85,8 @@ struct grub_error_saved
 extern grub_err_t EXPORT_VAR(grub_errno);
 extern char EXPORT_VAR(grub_errmsg)[GRUB_MAX_ERRMSG];
 
-grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *fmt, ...);
+grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *fmt, ...)
+    __attribute__ ((format (GNU_PRINTF, 2, 3)));
 void EXPORT_FUNC(grub_fatal) (const char *fmt, ...) __attribute__ ((noreturn));
 void EXPORT_FUNC(grub_error_push) (void);
 int EXPORT_FUNC(grub_error_pop) (void);
-- 
2.27.0

[PATCH v2 0/2] error: Do compile-time format string checking on grub_error.

[Glenn Washburn]

I've separated this into two patches from the previous one, but nothing else
has changed (except the commit message). I think at least the first patch should
be applied. Since I don't have a setup to test every architecture+platform
build of grub, I can't gaurantee that the second patch won't cause a build error
on some other architecture+platform combo. However, if it does, I'll bet it'll
get fixed quickly.  However, for that reason, I suggest the second patch be
applied after the 2.06 release (might be moot at this point). This series has
successfully built for me for i386-pc and x86_64-efi.

Glenn

Glenn Washburn (2):
  error: Fix issues mostly related to incorrect length modifiers in
    calls to grub_error().
  error: Do compile-time format string checking on grub_error.

 grub-core/commands/pgp.c           |  2 +-
 grub-core/disk/ata.c               |  4 ++--
 grub-core/disk/cryptodisk.c        | 14 ++++++++------
 grub-core/disk/dmraid_nvidia.c     |  2 +-
 grub-core/disk/luks2.c             |  6 +++---
 grub-core/efiemu/i386/loadcore64.c |  3 ++-
 grub-core/fs/hfsplus.c             |  3 ++-
 grub-core/fs/zfs/zfs.c             |  2 +-
 grub-core/kern/efi/efi.c           |  2 +-
 grub-core/kern/efi/mm.c            |  2 +-
 grub-core/kern/x86_64/dl.c         |  2 +-
 grub-core/loader/efi/chainloader.c |  4 ++--
 grub-core/loader/i386/bsd.c        |  3 ++-
 grub-core/loader/i386/pc/linux.c   |  4 ++--
 grub-core/net/tftp.c               |  2 +-
 grub-core/parttool/msdospart.c     |  4 ++--
 grub-core/script/lexer.c           |  2 +-
 grub-core/video/bochs.c            |  4 ++--
 include/grub/err.h                 |  3 ++-
 19 files changed, 37 insertions(+), 31 deletions(-)

-- 
2.27.0

[PATCH v2 1/2] error: Fix issues mostly related to incorrect length modifiers in calls to grub_error().

[Glenn Washburn]

The issues fixed here do not produce crashes that I'm aware of. However,
they do fix unintended error string output.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/commands/pgp.c           |  2 +-
 grub-core/disk/ata.c               |  4 ++--
 grub-core/disk/cryptodisk.c        | 14 ++++++++------
 grub-core/disk/dmraid_nvidia.c     |  2 +-
 grub-core/disk/luks2.c             |  6 +++---
 grub-core/efiemu/i386/loadcore64.c |  3 ++-
 grub-core/fs/hfsplus.c             |  3 ++-
 grub-core/fs/zfs/zfs.c             |  2 +-
 grub-core/kern/efi/efi.c           |  2 +-
 grub-core/kern/efi/mm.c            |  2 +-
 grub-core/kern/x86_64/dl.c         |  2 +-
 grub-core/loader/efi/chainloader.c |  4 ++--
 grub-core/loader/i386/bsd.c        |  3 ++-
 grub-core/loader/i386/pc/linux.c   |  4 ++--
 grub-core/net/tftp.c               |  2 +-
 grub-core/parttool/msdospart.c     |  4 ++--
 grub-core/script/lexer.c           |  2 +-
 grub-core/video/bochs.c            |  4 ++--
 18 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/grub-core/commands/pgp.c b/grub-core/commands/pgp.c
index bbf6871fe..150cb6fe9 100644
--- a/grub-core/commands/pgp.c
+++ b/grub-core/commands/pgp.c
@@ -633,7 +633,7 @@ grub_verify_signature_real (struct grub_pubkey_context *ctxt,
   if (!sk)
     {
       /* TRANSLATORS: %08x is 32-bit key id.  */
-      grub_error (GRUB_ERR_BAD_SIGNATURE, N_("public key %08x not found"),
+      grub_error (GRUB_ERR_BAD_SIGNATURE, N_("public key %08"PRIxGRUB_UINT64_T" not found"),
 		  keyid);
       goto fail;
     }
diff --git a/grub-core/disk/ata.c b/grub-core/disk/ata.c
index 685f33a19..c8f350ed3 100644
--- a/grub-core/disk/ata.c
+++ b/grub-core/disk/ata.c
@@ -219,7 +219,7 @@ grub_ata_setaddress (struct grub_ata *dev,
 	if (dev->sectors_per_track == 0
 	    || dev->heads == 0)
 	  return grub_error (GRUB_ERR_OUT_OF_RANGE,
-			     "sector %d cannot be addressed "
+			     "sector %"PRIxGRUB_UINT64_T" cannot be addressed "
 			     "using CHS addressing", sector);
 
 	/* Calculate the sector, cylinder and head to use.  */
@@ -232,7 +232,7 @@ grub_ata_setaddress (struct grub_ata *dev,
 	    || cylinder > dev->cylinders
 	    || head > dev->heads)
 	  return grub_error (GRUB_ERR_OUT_OF_RANGE,
-			     "sector %d cannot be addressed "
+			     "sector %"PRIxGRUB_UINT64_T" cannot be addressed "
 			     "using CHS addressing", sector);
 	
 	parms->taskfile.disk = 0xE0 | head;
diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index a3d672f68..59707766e 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -470,13 +470,15 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const char *ciphername, cons
       }
       if (cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
 	{
-	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported XTS block size: %d",
+	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
+			    "Unsupported XTS block size: %"PRIuGRUB_SIZE,
 			    cipher->cipher->blocksize);
 	  goto err;
 	}
       if (secondary_cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
 	{
-	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported XTS block size: %d",
+	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
+			    "Unsupported XTS block size: %"PRIuGRUB_SIZE,
 			    secondary_cipher->cipher->blocksize);
 	  goto err;
 	}
@@ -487,8 +489,8 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const char *ciphername, cons
       cipheriv = ciphermode + sizeof ("lrw-") - 1;
       if (cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
 	{
-	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported LRW block size: %d",
-			    cipher->cipher->blocksize);
+	  ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported LRW block size: %"
+			    PRIuGRUB_SIZE, cipher->cipher->blocksize);
 	  goto err;
 	}
     }
@@ -509,8 +511,8 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const char *ciphername, cons
     {
       if (cipher->cipher->blocksize & (cipher->cipher->blocksize - 1)
 	  || cipher->cipher->blocksize == 0)
-	grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported benbi blocksize: %d",
-		    cipher->cipher->blocksize);
+	grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported benbi blocksize: %"
+		    PRIuGRUB_SIZE, cipher->cipher->blocksize);
 	/* FIXME should we return an error here? */
       for (benbi_log = 0;
 	   (cipher->cipher->blocksize << benbi_log) < GRUB_DISK_SECTOR_SIZE;
diff --git a/grub-core/disk/dmraid_nvidia.c b/grub-core/disk/dmraid_nvidia.c
index 060279124..014581df0 100644
--- a/grub-core/disk/dmraid_nvidia.c
+++ b/grub-core/disk/dmraid_nvidia.c
@@ -122,7 +122,7 @@ grub_dmraid_nv_detect (grub_disk_t disk,
   if (sb.version != NV_VERSION)
     {
       grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-		  "unknown version: %d.%d", sb.version);
+		  "unknown version: %d", sb.version);
       return NULL;
     }
 
diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index 31d7166fc..fdf6cd1e7 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++)
     {
diff --git a/grub-core/efiemu/i386/loadcore64.c b/grub-core/efiemu/i386/loadcore64.c
index 18facf47f..3e9a71cfd 100644
--- a/grub-core/efiemu/i386/loadcore64.c
+++ b/grub-core/efiemu/i386/loadcore64.c
@@ -122,7 +122,8 @@ grub_arch_efiemu_relocate_symbols64 (grub_efiemu_segment_t segs,
                     break;
 		  default:
 		    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-				       N_("relocation 0x%x is not implemented yet"),
+				       N_("relocation 0x%"PRIxGRUB_UINT64_T
+					  " is not implemented yet"),
 				       ELF_R_TYPE (rel->r_info));
 		  }
 	      }
diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
index 9c4e4c88c..b538f52e0 100644
--- a/grub-core/fs/hfsplus.c
+++ b/grub-core/fs/hfsplus.c
@@ -188,7 +188,8 @@ grub_hfsplus_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
 	  || !nnode)
 	{
 	  grub_error (GRUB_ERR_READ_ERROR,
-		      "no block found for the file id 0x%x and the block offset 0x%x",
+		      "no block found for the file id 0x%x and the block"
+		      " offset 0x%"PRIuGRUB_UINT64_T,
 		      node->fileid, fileblock);
 	  break;
 	}
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index 41ef0ff57..9ad26e876 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -1869,7 +1869,7 @@ zio_read (blkptr_t *bp, grub_zfs_endian_t endian, void **buf,
     {
       if (BPE_GET_ETYPE(bp) != BP_EMBEDDED_TYPE_DATA)
 	return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-			   "unsupported embedded BP (type=%u)\n",
+			   "unsupported embedded BP (type=%llu)\n",
 			   BPE_GET_ETYPE(bp));
       lsize = BPE_GET_LSIZE(bp);
       psize = BF64_GET_SB(grub_zfs_to_cpu64 ((bp)->blk_prop, endian), 25, 7, 0, 1);
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index e0165e74c..a56be538c 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -491,7 +491,7 @@ grub_efi_duplicate_device_path (const grub_efi_device_path_t *dp)
       if (len < 4)
 	{
 	  grub_error (GRUB_ERR_OUT_OF_RANGE,
-		      "malformed EFI Device Path node has length=%d", len);
+		      "malformed EFI Device Path node has length=%ld", len);
 	  return NULL;
 	}
 
diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index 457772d57..22f89b6af 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -127,7 +127,7 @@ grub_efi_allocate_pages_real (grub_efi_physical_address_t address,
   if (address > GRUB_EFI_MAX_USABLE_ADDRESS)
     {
       grub_error (GRUB_ERR_BAD_ARGUMENT,
-		  N_("invalid memory address (0x%llx > 0x%llx)"),
+		  N_("invalid memory address (0x%lx > 0x%x)"),
 		  address, GRUB_EFI_MAX_USABLE_ADDRESS);
       return NULL;
     }
diff --git a/grub-core/kern/x86_64/dl.c b/grub-core/kern/x86_64/dl.c
index 3a73e6e6c..aa01cc167 100644
--- a/grub-core/kern/x86_64/dl.c
+++ b/grub-core/kern/x86_64/dl.c
@@ -107,7 +107,7 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
 
 	default:
 	  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-			     N_("relocation 0x%x is not implemented yet"),
+			     N_("relocation 0x%lx is not implemented yet"),
 			     ELF_R_TYPE (rel->r_info));
 	}
     }
diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c
index 7b31c3fb9..b8d35f41e 100644
--- a/grub-core/loader/efi/chainloader.c
+++ b/grub-core/loader/efi/chainloader.c
@@ -90,7 +90,7 @@ grub_chainloader_boot (void)
 	      *grub_utf16_to_utf8 ((grub_uint8_t *) buf,
 				   exit_data, exit_data_size) = 0;
 
-	      grub_error (GRUB_ERR_BAD_OS, buf);
+	      grub_error (GRUB_ERR_BAD_OS, "%s", buf);
 	      grub_free (buf);
 	    }
 	}
@@ -164,7 +164,7 @@ make_file_path (grub_efi_device_path_t *dp, const char *filename)
       if (len < 4)
 	{
 	  grub_error (GRUB_ERR_OUT_OF_RANGE,
-		      "malformed EFI Device Path node has length=%d", len);
+		      "malformed EFI Device Path node has length=%"PRIuGRUB_SIZE, len);
 	  return NULL;
 	}
 
diff --git a/grub-core/loader/i386/bsd.c b/grub-core/loader/i386/bsd.c
index f5bf7f89e..fe20b339b 100644
--- a/grub-core/loader/i386/bsd.c
+++ b/grub-core/loader/i386/bsd.c
@@ -2110,7 +2110,8 @@ grub_cmd_openbsd_ramdisk (grub_command_t cmd __attribute__ ((unused)),
     {
       grub_file_close (file);
       return grub_error (GRUB_ERR_BAD_OS, "your kOpenBSD supports ramdisk only"
-			 " up to %u bytes, however you supplied a %u bytes one",
+			 " up to %"PRIuGRUB_SIZE" bytes, however you supplied"
+			 " a %"PRIuGRUB_SIZE" bytes one",
 			 openbsd_ramdisk.max_size, size);
     }
 
diff --git a/grub-core/loader/i386/pc/linux.c b/grub-core/loader/i386/pc/linux.c
index 814988ab9..0bc5d6807 100644
--- a/grub-core/loader/i386/pc/linux.c
+++ b/grub-core/loader/i386/pc/linux.c
@@ -230,9 +230,9 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
       && GRUB_LINUX_ZIMAGE_ADDR + grub_linux16_prot_size
       > grub_linux_real_target)
     {
-      grub_error (GRUB_ERR_BAD_OS, "too big zImage (0x%x > 0x%x), use bzImage instead",
+      grub_error (GRUB_ERR_BAD_OS, "too big zImage (%p > %p), use bzImage instead",
 		  (char *) GRUB_LINUX_ZIMAGE_ADDR + grub_linux16_prot_size,
-		  (grub_size_t) grub_linux_real_target);
+		  (void *) grub_linux_real_target);
       goto fail;
     }
 
diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c
index 7e659a78b..592bfee34 100644
--- a/grub-core/net/tftp.c
+++ b/grub-core/net/tftp.c
@@ -252,7 +252,7 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)),
     case TFTP_ERROR:
       data->have_oack = 1;
       grub_netbuff_free (nb);
-      grub_error (GRUB_ERR_IO, (char *) tftph->u.err.errmsg);
+      grub_error (GRUB_ERR_IO, "%s", tftph->u.err.errmsg);
       grub_error_save (&data->save_err);
       return GRUB_ERR_NONE;
     default:
diff --git a/grub-core/parttool/msdospart.c b/grub-core/parttool/msdospart.c
index dcbf74e3b..2e2e9d527 100644
--- a/grub-core/parttool/msdospart.c
+++ b/grub-core/parttool/msdospart.c
@@ -127,8 +127,8 @@ static grub_err_t grub_pcpart_type (const grub_device_t dev,
     {
       dev->disk->partition = part;
       return grub_error (GRUB_ERR_BAD_ARGUMENT,
-			 N_("the partition type 0x%x isn't "
-			    "valid"));
+			 N_("the partition type 0x%x isn't valid"),
+			    type);
     }
 
   mbr.entries[index].type = type;
diff --git a/grub-core/script/lexer.c b/grub-core/script/lexer.c
index 57778f881..52004d059 100644
--- a/grub-core/script/lexer.c
+++ b/grub-core/script/lexer.c
@@ -349,7 +349,7 @@ void
 grub_script_yyerror (struct grub_parser_param *state, const char *err)
 {
   if (err)
-    grub_error (GRUB_ERR_INVALID_COMMAND, err);
+    grub_error (GRUB_ERR_INVALID_COMMAND, "%s", err);
 
   grub_print_error ();
   state->err++;
diff --git a/grub-core/video/bochs.c b/grub-core/video/bochs.c
index 7a249eb21..30ea1bd82 100644
--- a/grub-core/video/bochs.c
+++ b/grub-core/video/bochs.c
@@ -249,11 +249,11 @@ grub_video_bochs_setup (unsigned int width, unsigned int height,
     }
 
   if (width > BOCHS_MAX_WIDTH)
-    return grub_error (GRUB_ERR_IO, "width must be at most",
+    return grub_error (GRUB_ERR_IO, "width must be at most %d",
 		       BOCHS_MAX_WIDTH);
 
   if (height > BOCHS_MAX_HEIGHT)
-    return grub_error (GRUB_ERR_IO, "height must be at most",
+    return grub_error (GRUB_ERR_IO, "height must be at most %d",
 		       BOCHS_MAX_HEIGHT);
 
   if (width & (BOCHS_WIDTH_ALIGN - 1))
-- 
2.27.0

[PATCH v2 2/2] error: Do compile-time format string checking on grub_error.

[Glenn Washburn]

This should prevent the current situation where there were many format
string errors found and thus improve the quality of error reporting.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 include/grub/err.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/grub/err.h b/include/grub/err.h
index 24ba9f5f5..b08d5d0de 100644
--- a/include/grub/err.h
+++ b/include/grub/err.h
@@ -85,7 +85,8 @@ struct grub_error_saved
 extern grub_err_t EXPORT_VAR(grub_errno);
 extern char EXPORT_VAR(grub_errmsg)[GRUB_MAX_ERRMSG];
 
-grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *fmt, ...);
+grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *fmt, ...)
+    __attribute__ ((format (GNU_PRINTF, 2, 3)));
 void EXPORT_FUNC(grub_fatal) (const char *fmt, ...) __attribute__ ((noreturn));
 void EXPORT_FUNC(grub_error_push) (void);
 int EXPORT_FUNC(grub_error_pop) (void);
-- 
2.27.0

Re: [PATCH v2 0/2] error: Do compile-time format string checking on grub_error.

[Glenn Washburn]

On Sat,  7 Nov 2020 23:26:17 -0600
Glenn Washburn <development@efficientek.com> wrote:

> I've separated this into two patches from the previous one, but
> nothing else has changed (except the commit message). I think at
> least the first patch should be applied. Since I don't have a setup
> to test every architecture+platform build of grub, I can't gaurantee
> that the second patch won't cause a build error on some other
> architecture+platform combo. However, if it does, I'll bet it'll get
> fixed quickly.  However, for that reason, I suggest the second patch
> be applied after the 2.06 release (might be moot at this point). This
> series has successfully built for me for i386-pc and x86_64-efi.
> 

Daniel, have you had a chance to take a look at these?  If so, is there
anything I can do to help get these merged?

Glenn

> Glenn Washburn (2):
>   error: Fix issues mostly related to incorrect length modifiers in
>     calls to grub_error().
>   error: Do compile-time format string checking on grub_error.
> 
>  grub-core/commands/pgp.c           |  2 +-
>  grub-core/disk/ata.c               |  4 ++--
>  grub-core/disk/cryptodisk.c        | 14 ++++++++------
>  grub-core/disk/dmraid_nvidia.c     |  2 +-
>  grub-core/disk/luks2.c             |  6 +++---
>  grub-core/efiemu/i386/loadcore64.c |  3 ++-
>  grub-core/fs/hfsplus.c             |  3 ++-
>  grub-core/fs/zfs/zfs.c             |  2 +-
>  grub-core/kern/efi/efi.c           |  2 +-
>  grub-core/kern/efi/mm.c            |  2 +-
>  grub-core/kern/x86_64/dl.c         |  2 +-
>  grub-core/loader/efi/chainloader.c |  4 ++--
>  grub-core/loader/i386/bsd.c        |  3 ++-
>  grub-core/loader/i386/pc/linux.c   |  4 ++--
>  grub-core/net/tftp.c               |  2 +-
>  grub-core/parttool/msdospart.c     |  4 ++--
>  grub-core/script/lexer.c           |  2 +-
>  grub-core/video/bochs.c            |  4 ++--
>  include/grub/err.h                 |  3 ++-
>  19 files changed, 37 insertions(+), 31 deletions(-)
> 

Re: [PATCH v2 0/2] error: Do compile-time format string checking on grub_error.

[Glenn Washburn]

On Mon, 23 Nov 2020 03:31:23 -0600
Glenn Washburn <development@efficientek.com> wrote:

> On Sat,  7 Nov 2020 23:26:17 -0600
> Glenn Washburn <development@efficientek.com> wrote:
> 
> > I've separated this into two patches from the previous one, but
> > nothing else has changed (except the commit message). I think at
> > least the first patch should be applied. Since I don't have a setup
> > to test every architecture+platform build of grub, I can't gaurantee
> > that the second patch won't cause a build error on some other
> > architecture+platform combo. However, if it does, I'll bet it'll get
> > fixed quickly.  However, for that reason, I suggest the second patch
> > be applied after the 2.06 release (might be moot at this point).
> > This series has successfully built for me for i386-pc and
> > x86_64-efi.
> > 
> 
> Daniel, have you had a chance to take a look at these?  If so, is
> there anything I can do to help get these merged?

Disregard this patch series. There are some issues with it. I've
re-done the series as many patches that can stand on their own, so they
can get merged in piece meal.  I'll start a new thread for them.

Glenn

> 
> > Glenn Washburn (2):
> >   error: Fix issues mostly related to incorrect length modifiers in
> >     calls to grub_error().
> >   error: Do compile-time format string checking on grub_error.
> > 
> >  grub-core/commands/pgp.c           |  2 +-
> >  grub-core/disk/ata.c               |  4 ++--
> >  grub-core/disk/cryptodisk.c        | 14 ++++++++------
> >  grub-core/disk/dmraid_nvidia.c     |  2 +-
> >  grub-core/disk/luks2.c             |  6 +++---
> >  grub-core/efiemu/i386/loadcore64.c |  3 ++-
> >  grub-core/fs/hfsplus.c             |  3 ++-
> >  grub-core/fs/zfs/zfs.c             |  2 +-
> >  grub-core/kern/efi/efi.c           |  2 +-
> >  grub-core/kern/efi/mm.c            |  2 +-
> >  grub-core/kern/x86_64/dl.c         |  2 +-
> >  grub-core/loader/efi/chainloader.c |  4 ++--
> >  grub-core/loader/i386/bsd.c        |  3 ++-
> >  grub-core/loader/i386/pc/linux.c   |  4 ++--
> >  grub-core/net/tftp.c               |  2 +-
> >  grub-core/parttool/msdospart.c     |  4 ++--
> >  grub-core/script/lexer.c           |  2 +-
> >  grub-core/video/bochs.c            |  4 ++--
> >  include/grub/err.h                 |  3 ++-
> >  19 files changed, 37 insertions(+), 31 deletions(-)
> >