On Fri, Jul 03, 2020 at 01:00:21PM +0200, Heinrich Schuchardt wrote:
> On 09.06.20 07:09, AKASHI Takahiro wrote:
> > Since the size check against an entry in efi_search_siglist() is
> > incorrect, this function will never find out a to-be-matched certificate
> > and its associated revocation time in the signature list.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
> > ---
> > lib/efi_loader/efi_signature.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> > index a05c75472721..f22dc151971f 100644
> > --- a/lib/efi_loader/efi_signature.c
> > +++ b/lib/efi_loader/efi_signature.c
> > @@ -434,10 +434,11 @@ static bool efi_search_siglist(struct
> > x509_certificate *cert,
> > * time64_t revocation_time;
> > * };
> > */
> > - if ((sig_data->size == SHA256_SUM_LEN) &&
> > - !memcmp(sig_data->data, hash, SHA256_SUM_LEN)) {
> > + if ((sig_data->size >= SHA256_SUM_LEN + sizeof(time64_t)) &&
> > + !memcmp(sig_data->data, msg, SHA256_SUM_LEN)) {
> > memcpy(revoc_time, sig_data->data + SHA256_SUM_LEN,
> > sizeof(*revoc_time));
> > + EFI_PRINT("revocation time: %llu\n", *revoc_time);
>
> *revoc_time is of type __s64. So this must be %lld. Cf.
>
> include/linux/time.h:156
I know that because I added the definition.
Interestingly, linux added another type, timeu64_t, later on
to avoid an overflow in some calculation.
While I don't think the current format is harmful, I will change it.
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> > found = true;
> > goto out;
> > }
> >
>
