Re: [Xen-devel] [PATCH] kexec: clear kexec_image slot when unloading kexec image

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Mon, Apr 03, 2017 at 10:44:54AM -0700, Bhavesh Davda wrote:
> While theoretically this bug can be tickled simply by a sequence of 'kexec -p'
> to load a kexec crash image followed by two back-to-back 'kexec -p -u' to
> unload the kexec crash image, I found the following perl script to be useful 
> to
> reliably reproduce Xen panics as well as verify that the fix works. YMMV.
>
> -snip-
> #!/usr/bin/perl -w
>
> use strict;
> use warnings;
> use threads;
>
> sub threaded_task {
>     threads->create(sub {
>         my $thr_id = threads->self->tid;
>         print "Starting load thread $thr_id\n";
>         system("/sbin/kexec  -p --command-line=\"placeholder 
> root=/dev/mapper/root ro rhbg console=tty0 console=hvc0 earlyprintk=xen 
> nomodeset printk.time=1 irqpoll maxcpus=1 nr_cpus=1 reset_devices 
> cgroup_disable=memory mce=off selinux=0 console=ttyS1,115200n8\" 
> --initrd=/boot/initrd.x86_64kdump.img /boot/vmlinuz.x86_64");
>         print "Ending load thread $thr_id\n";
>         threads->detach(); #End thread.
>     });
>     threads->create(sub {
>         my $thr_id = threads->self->tid;
>         print "Starting unload thread $thr_id\n";
>         system("/sbin/kexec  -p -u");
>         print "Ending unload thread $thr_id\n";
>         threads->detach(); #End thread.
>     });
> }
>
> for my $i (0..99)
> {
>     threaded_task();
> }
> -snip-
>
> ---
> When kexec_do_unload calls kexec_swap_images to get the old kexec_image to
> free, it passes NULL for the new kexec_image pointer. The new slot wasn't 
> being
> cleared in such a case, leading to a stale pointer being left behind in the
> kexec_image array and Xen panics in subsequent load/unload operations.
>
> Signed-off-by: Bhavesh Davda <bhavesh.davda@xxxxxxxxxx>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Reviewed-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
> ---
>  xen/common/kexec.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/xen/common/kexec.c b/xen/common/kexec.c
> index 940fc7ec94..072cc8e0db 100644
> --- a/xen/common/kexec.c
> +++ b/xen/common/kexec.c
> @@ -837,11 +837,9 @@ static int kexec_swap_images(int type, struct 
> kexec_image *new,
>      old_slot = base + pos;
>      new_slot = base + !pos;
>
> +    kexec_image[new_slot] = new;
>      if ( new )
> -    {
> -        kexec_image[new_slot] = new;
>          set_bit(new_slot, &kexec_flags);
> -    }
>      change_bit(bit, &kexec_flags);
>
>      clear_bit(old_slot, &kexec_flags);

Bhavesh, thanks for posting this.

Jan, Andrew, IMO, this is Xen stable material too.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel