Re: [PATCH v19 22/33] key_protector: Add TPM2 Key Protector

[Gary Lin]

On Wed, Sep 18, 2024 at 11:22:16AM -0400, Stefan Berger wrote:
> 
> 
> On 9/6/24 5:11 AM, Gary Lin via Grub-devel wrote:
> > From: Hernan Gatta <hegatta@linux.microsoft.com>
> > 
> > The TPM2 key protector is a module that enables the automatic retrieval
> > of a fully-encrypted disk's unlocking key from a TPM 2.0.
> > 
> > The theory of operation is such that the module accepts various
> > arguments, most of which are optional and therefore possess reasonable
> > defaults. One of these arguments is the keyfile/tpm2key parameter, which
> > is mandatory. There are two supported key formats:
> > 
> 
> > +}
> > +
> > +grub_err_t
> > +grub_tpm2_protector_parse_tpm_handle (const char *value, TPM_HANDLE_t 
> > *handle)
> > +{
> > +  grub_uint64_t num;
> > +  const char *str_end;
> > +
> > +  grub_errno = GRUB_ERR_NONE;
> > +  num = grub_strtoul (value, &str_end, 0);
> > +  if (*value == '\0' || *str_end != '\0')
> > +    return grub_error (GRUB_ERR_BAD_NUMBER, "TPM handle value '%s' is not 
> > a number", value);
> > +
> > +  if (num > GRUB_UINT_MAX)
> > +    return grub_error (GRUB_ERR_OUT_OF_RANGE, "Value %lu is too large to 
> > be a TPM handle, TPM handles are unsigned 32-bit integers", num);
> 
> commands/tpm2_key_protector/args.c: In function
> ‘grub_tpm2_protector_parse_tpm_handle’:
> commands/tpm2_key_protector/args.c:124:56: error: format ‘%lu’ expects
> argument of type ‘long unsigned int’, but argument 3 has type
> ‘grub_uint64_t’ {aka ‘long long unsigned int’} [-Werror=format=]
>   124 |     return grub_error (GRUB_ERR_OUT_OF_RANGE, "Value %lu is too
> large to be a TPM handle, TPM handles are unsigned 32-bit integers", num);
>       |                                                      ~~^
> ~~~
>       |                                                        |
> |
>       |                                                        long unsigned
> int grub_uint64_t {aka long long unsigned int}
>       |                                                      %llu
> cc1: all warnings being treated as errors
> 
> 
> Changing this to %llu leads to:
> 
> grub-core/commands/tpm2_key_protector/args.c: In function
> ‘grub_tpm2_protector_parse_tpm_handle’:
> grub-core/commands/tpm2_key_protector/args.c:124:57: error: format ‘%llu’
> expects argument of type ‘long long unsigned int’, but argument 3 has type
> ‘grub_uint64_t’ {aka ‘long unsigned int’} [-Werror=format=]
>   124 |     return grub_error (GRUB_ERR_OUT_OF_RANGE, "Value %llu is too
> large to be a TPM handle, TPM handles are unsigned 32-bit integers", num);
>       |                                                      ~~~^
> ~~~
>       |                                                         |
> |
>       |                                                         long long
> unsigned int grub_uint64_t {aka long unsigned int}
>       |                                                      %lu
> cc1: all warnings being treated as errors
> 
> 
> Can't win . This works:
> 
> diff --git a/grub-core/commands/tpm2_key_protector/args.c
> b/grub-core/commands/tpm2_key_protector/args.c
> index c58cbe307..749db81a5 100644
> --- a/grub-core/commands/tpm2_key_protector/args.c
> +++ b/grub-core/commands/tpm2_key_protector/args.c
> @@ -121,7 +121,7 @@ grub_tpm2_protector_parse_tpm_handle (const char *value,
> TPM_HANDLE_t *handle)
>      return grub_error (GRUB_ERR_BAD_NUMBER, "TPM handle value '%s' is not a
> number", value);
> 
>    if (num > GRUB_UINT_MAX)
> -    return grub_error (GRUB_ERR_OUT_OF_RANGE, "Value %lu is too large to be
> a TPM handle, TPM handles are unsigned 32-bit integers", num);
> +    return grub_error (GRUB_ERR_OUT_OF_RANGE, "Value %lu is too large to be
> a TPM handle, TPM handles are unsigned 32-bit integers", (unsigned
> long)num);
> 
>    *handle = (TPM_HANDLE_t) num;
> 
How about this change?

- return grub_error (GRUB_ERR_OUT_OF_RANGE, "Value %lu is too large to be a TPM 
handle, TPM handles are unsigned 32-bit integers", num);
+ return grub_error (GRUB_ERR_OUT_OF_RANGE, "Value " PRIuGRUB_UINT64_T " is too 
large to be a TPM handle, TPM handles are unsigned 32-bit integers", num);

Here we use the type macro to choose the correct type string for
'grub_uint64_t' to avoid the potential compiler error.

Gary Lin