Re: [SECURITY PATCH 114/117] kern/misc: Add function to check printf() format against expected format

[Colin Watson]

On Tue, Mar 02, 2021 at 07:02:01PM +0100, Daniel Kiper wrote:
> @@ -1121,6 +1159,42 @@ grub_xasprintf (const char *fmt, ...)
>    return ret;
>  }
>  
> +grub_err_t
> +grub_printf_fmt_check (const char *fmt, const char *fmt_expected)
> +{
> +  struct printf_args args_expected, args_fmt;
> +  grub_err_t ret;
> +  grub_size_t n;
> +
> +  if (fmt == NULL || fmt_expected == NULL)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, "invalid format");
> +
> +  ret = parse_printf_arg_fmt (fmt_expected, &args_expected, 1, 
> GRUB_SIZE_MAX);
> +  if (ret != GRUB_ERR_NONE)
> +    return ret;
> +
> +  /* Limit parsing to the number of expected arguments. */
> +  ret = parse_printf_arg_fmt (fmt, &args_fmt, 1, args_expected.count);
> +  if (ret != GRUB_ERR_NONE)
> +    {
> +      free_printf_args (&args_expected);
> +      return ret;
> +    }
> +
> +  for (n = 0; n < args_fmt.count; n++)
> +    if (args_fmt.ptr[n].type != args_expected.ptr[n].type)
> +     {
> +     ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "arguments types do not 
> match");
> +     break;
> +     }
> +
> +  free_printf_args (&args_expected);
> +  free_printf_args (&args_fmt);
> +
> +  return ret;
> +}
> +
> +
>  /* Abort GRUB. This function does not return.  */
>  static void __attribute__ ((noreturn))
>  grub_abort (void)

This function is only used by gfxmenu.  Could it please be moved out of
the kernel and into a module, perhaps just gfxmenu itself?

As shown in the report from the other mail I just sent to this thread,
this patch costs an additional 273 bytes on i386-pc, where core image
size is at an extreme premium on some systems with limited space between
the MBR and the first partition; a plausible RAID setup used by some
Debian users is currently 316 bytes over the limit, so reclaiming this
space would be valuable.

Reclaiming all of those 273 bytes would probably involve duplicating
parse_printf_arg_fmt rather than having the fmt_check and max_args
arguments there, but I'm sure that even just moving the top-level
format-checking function would help.

Thanks,

-- 
Colin Watson (he/him)                              [cjwatson@debian.org]