Re: [PATCH 1/7] x86/kexec: add digest checks

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/06/2026 4:45 pm, Kevin Lampis wrote:
> From: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
>
> During kexec load a sha256 digest is calculated of all the kexec
> segments combined. This digest is stored and verified again during kexec
> execution.
>
> This is a requirement for Secure Boot to ensure that kexec data has not
> been tampered with or corrupted between signature verification and
> actual execution.
>
> Only kexec crash is supported. The segments for normal kexec are stored
> in temporary buffers and moved to their intended destination during
> execution. To calculate/verify the normal kexec data before relocation
> would involve walking the relocation table to find every temporary
> buffer which was out of scope for Secure Boot work.

I'd suggest reordering the information for clarity.  How about this:

"In order to support UEFI Secure Boot, we must confirm that the kexec
data has not been tampered with or corrupted between signature
verification and actual execution.  However, it's a good check to
perform generally.

During kexec load, calculate a sha2_256 digest over all the kexec
segments.  This is stored and verified again later prior to entering the
image.

For now, only kexec crash images are support.  The segments for normal
kexec are stored in temporary buffers and moved to their intended
destination during execution. The logic to reconstruct the normal kexec
images runs long after Xen could cleanly cope with a digest failure."


"out of scope" is not really something that wants to end up in the
commit message.

> diff --git a/xen/common/kexec.c b/xen/common/kexec.c
> index 65776a95fd..c920bc6d8a 100644
> --- a/xen/common/kexec.c
> +++ b/xen/common/kexec.c
> @@ -383,6 +383,12 @@ void kexec_crash(enum crash_reason reason)
>      if ( !test_bit(KEXEC_IMAGE_CRASH_BASE + pos, &kexec_flags) )
>          return;
>  
> +    if ( kimage_verify_digest(kexec_image[KEXEC_IMAGE_CRASH_BASE + pos]) != 
> 0 )
> +    {
> +        printk(XENLOG_ERR "kexec digest failed, won't boot corrupted 
> image\n");

"kexec digest failed, aborting kexec crash transfer\n".

The !=0 can be dropped.

> diff --git a/xen/common/kimage.c b/xen/common/kimage.c
> index 6202491f7e..018ef66451 100644
> --- a/xen/common/kimage.c
> +++ b/xen/common/kimage.c
> @@ -20,9 +20,12 @@
>  #include <xen/mm.h>
>  #include <xen/kexec.h>
>  #include <xen/kimage.h>
> +#include <xen/sha2.h>

You don't ned to include sha2.h here in kimiage.c, seeing as you need it
in kimage.h for the struct change.

> @@ -820,6 +823,101 @@ int kimage_alloc(struct kexec_image **rimage, uint8_t 
> type, uint16_t arch,
>      return result;
>  }
>  
> +static int kimage_calc_one_digest(struct sha2_256_state *ctx,
> +                                  xen_kexec_segment_t *segment)
> +{
> +    paddr_t dest;
> +    unsigned long sbytes;
> +    int ret = 0;
> +
> +    sbytes = segment->buf_size;
> +    dest = segment->dest_maddr;
> +
> +    while ( sbytes )
> +    {
> +        unsigned long dest_mfn;
> +        void *dest_va;
> +        size_t schunk, dchunk;
> +
> +        dest_mfn = dest >> PAGE_SHIFT;
> +
> +        dchunk = PAGE_SIZE;
> +        schunk = min(dchunk, sbytes);
> +
> +        dest_va = map_domain_page(_mfn(dest_mfn));
> +        if ( !dest_va )
> +            return -EINVAL;

map_domain_page() doesn't fail.

> +
> +        sha2_256_update(ctx, dest_va, schunk);
> +
> +        unmap_domain_page(dest_va);
> +        if ( ret )
> +            return -EFAULT;

ret is always 0.  (This is dead logic from copying the form using
copy_from_guest()).

With these dropped, the function can become void.

> +
> +        sbytes -= schunk;
> +        dest += dchunk;
> +    }
> +    return 0;
> +}
> +
> +int kimage_calc_digest(const struct kexec_image *image,
> +                       uint8_t digest[SHA2_256_DIGEST_SIZE])
> +{
> +    int ret;
> +    struct sha2_256_state ctx;
> +    unsigned int s;
> +
> +    if ( image->type == KEXEC_TYPE_DEFAULT )
> +    {
> +        /* TODO implement digest calculation for normal kexec */
> +        return 0;
> +    }
> +
> +    if ( image->nr_segments > KIMAGE_SHA256_REGIONS )
> +    {
> +        dprintk(XENLOG_DEBUG, "More segments than allocated SHA256 
> regions\n");
> +        return -E2BIG;
> +    }

This is the only use of KIMAGE_SHA256_REGIONS, but I think it's stale
from the version of digest checking in the patchqueue.  I think you can
drop it.

> +
> +
> +    sha2_256_init(&ctx);
> +
> +    for ( s = 0; s < image->nr_segments; s++ ) {

Style.  Brace on next line.  (The code you copied from was wrong.)

> +        ret = kimage_calc_one_digest(&ctx, &image->segments[s]);
> +        if ( ret )
> +            return ret;
> +    }
> +
> +    sha2_256_final(&ctx, digest);
> +    return 0;

With kimage_calc_one_digest() becoming void, so can kimage_calc_digest().

> +}
> +
> +int kimage_verify_digest(const struct kexec_image *image)
> +{
> +    uint8_t digest[SHA2_256_DIGEST_SIZE];
> +    int ret;
> +
> +    if ( image->type == KEXEC_TYPE_DEFAULT )
> +    {
> +        /* TODO implement digest check for normal kexec */
> +        return 0;
> +    }
> +
> +    ret = kimage_calc_digest(image, digest);
> +    if ( ret )
> +        return ret;
> +
> +    if ( memcmp(digest, image->digest, sizeof(digest)) != 0 )
> +    {
> +        printk(XENLOG_ERR "kexec digest failed expected %*phN but got 
> %*phN\n",

"kexec digest failed:\n"
"  expected %" STR(SHA2_256_DIGEST_SIZE) "phN\n"
"       got %" STR(SHA2_256_DIGEST_SIZE) "phN\n"

The newlines and tabulation are important for legibility if this really
triggers.

In hindsight I should have pointed you at check_digest() for AMD
microcode, which is even closer to what you needed.

> +               SHA2_256_DIGEST_SIZE, image->digest,
> +               SHA2_256_DIGEST_SIZE, digest);
> +        return 1;

return -ENODATA or -EBADMSG.

Error handling needs to be either truly boolean (in which case the
function wants to be bool), or int with -error.

~Andrew