Re: [PATCH 3/7] x86/kexec: add new struct kimage_segment

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/06/2026 4:45 pm, Kevin Lampis wrote:
> diff --git a/xen/common/kexec.c b/xen/common/kexec.c
> index c920bc6d8a..00346fe616 100644
> --- a/xen/common/kexec.c
> +++ b/xen/common/kexec.c
> @@ -1119,14 +1120,18 @@ static int kexec_load(XEN_GUEST_HANDLE_PARAM(void) 
> uarg)
>      if ( load.nr_segments >= KEXEC_SEGMENT_MAX )
>          return -EINVAL;
>  
> -    segments = xmalloc_array(xen_kexec_segment_t, load.nr_segments);
> +    segments = xmalloc_array(struct kimage_segment, load.nr_segments);
>      if ( segments == NULL )
>          return -ENOMEM;
>  
> -    if ( copy_from_guest(segments, load.segments.h, load.nr_segments) )
> +    for ( i = 0; i < load.nr_segments; i++ )
>      {
> -        ret = -EFAULT;
> -        goto error;
> +        if ( copy_from_guest_offset((xen_kexec_segment_t *)&segments[i],
> +                                    load.segments.h, i, 1) )

This cast is unsafe.

It relies on struct kimage_segment having xen_kexec_segment_t at the
start of it, which is not mentioned or enforced anywhere.

In the next patch when you add dest_offset, this leaves parts of
segments[] uninitialised.

Read into a local xen_kexec_segment_t variable and fill in segments[i]
manually as you're wanting to change it's type.

> diff --git a/xen/include/xen/kimage.h b/xen/include/xen/kimage.h
> index ad52551ba2..258349d774 100644
> --- a/xen/include/xen/kimage.h
> +++ b/xen/include/xen/kimage.h
> @@ -18,12 +18,22 @@
>  
>  typedef paddr_t kimage_entry_t;
>  
> +struct kimage_segment {
> +    union {
> +        XEN_GUEST_HANDLE(const_void) h;
> +        uint64_t _pad;

After separating the types, get rid of this union and _pad.  It's ABI
abuse which needs to not infect anything it doesn't strictly need to.

~Andrew