Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference

Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference

[Dave Young]

Hi Mike,

Thanks for the patch!
On 08/08/18 at 04:03pm, Mike Galbraith wrote:
> When booting with efi=noruntime, we call efi_runtime_map_copy() while
> loading the kdump kernel, and trip over a NULL efi.memmap.map.  Avoid
> that and a useless allocation when the only mapping we can use (1:1)
> is not available.

At first glance, efi_get_runtime_map_size should return 0 in case
noruntime.

Also since we are here, would you mind to restructure the bzImage64_load
function, and try to move all efi related code to setup_efi_state()?


setup_boot_parameters(struct kimage *image, struct boot_params *params,
                      unsigned long params_load_addr,
                      unsigned int efi_map_offset, unsigned int efi_map_sz,
                      unsigned int efi_setup_data_offset)
{
[snip]

#ifdef CONFIG_EFI
        /* Setup EFI state */
        setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
                        efi_setup_data_offset);
#endif

[snip]
}

Currently bzImage64_load prepares the efi_map_offset, efi_map_sz,
 and efi_setup_data_offset and then pass it to setup_boot_parameters and
setup_efi_state.  It should be better to move those efi_* variables to
setup_efi_state().

So we can call setup_efi_state only when efi runtime is enabled.

> 
> Signed-off-by: Mike Galbraith <efault@gmx.de>
> ---
>  arch/x86/kernel/kexec-bzimage64.c |   22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -122,9 +122,6 @@ static int setup_efi_info_memmap(struct
>  	unsigned long efi_map_phys_addr = params_load_addr + efi_map_offset;
>  	struct efi_info *ei = &params->efi_info;
>  
> -	if (!efi_map_sz)
> -		return 0;
> -
>  	efi_runtime_map_copy(efi_map, efi_map_sz);
>  
>  	ei->efi_memmap = efi_map_phys_addr & 0xffffffff;
> @@ -176,7 +173,7 @@ setup_efi_state(struct boot_params *para
>  	 * acpi_rsdp=<addr> on kernel command line to make second kernel boot
>  	 * without efi.
>  	 */
> -	if (efi_enabled(EFI_OLD_MEMMAP))
> +	if (efi_enabled(EFI_OLD_MEMMAP) || !efi_enabled(EFI_MEMMAP))
>  		return 0;
>  
>  	ei->efi_loader_signature = current_ei->efi_loader_signature;
> @@ -338,7 +335,7 @@ static void *bzImage64_load(struct kimag
>  	struct kexec_entry64_regs regs64;
>  	void *stack;
>  	unsigned int setup_hdr_offset = offsetof(struct boot_params, hdr);
> -	unsigned int efi_map_offset, efi_map_sz, efi_setup_data_offset;
> +	unsigned int efi_map_offset = 0, efi_map_sz = 0, efi_setup_data_offset = 0;
>  	struct kexec_buf kbuf = { .image = image, .buf_max = ULONG_MAX,
>  				  .top_down = true };
>  	struct kexec_buf pbuf = { .image = image, .buf_min = MIN_PURGATORY_ADDR,
> @@ -397,19 +394,22 @@ static void *bzImage64_load(struct kimag
>  	 * have to create separate segment for each. Keeps things
>  	 * little bit simple
>  	 */
> -	efi_map_sz = efi_get_runtime_map_size();
>  	params_cmdline_sz = sizeof(struct boot_params) + cmdline_len +
>  				MAX_ELFCOREHDR_STR_LEN;
>  	params_cmdline_sz = ALIGN(params_cmdline_sz, 16);
> -	kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16) +
> -				sizeof(struct setup_data) +
> -				sizeof(struct efi_setup_data);
> +	kbuf.bufsz = params_cmdline_sz + sizeof(struct setup_data);
> +
> +	/* Now add space for the efi stuff if we have a useable 1:1 mapping. */
> +	if (!efi_enabled(EFI_OLD_MEMMAP) && efi_enabled(EFI_MEMMAP)) {
> +		efi_map_sz = efi_get_runtime_map_size();
> +		kbuf.bufsz += ALIGN(efi_map_sz, 16) + sizeof(struct efi_setup_data);
> +		efi_map_offset = params_cmdline_sz;
> +		efi_setup_data_offset = efi_map_offset + ALIGN(efi_map_sz, 16);
> +	}
>  
>  	params = kzalloc(kbuf.bufsz, GFP_KERNEL);
>  	if (!params)
>  		return ERR_PTR(-ENOMEM);
> -	efi_map_offset = params_cmdline_sz;
> -	efi_setup_data_offset = efi_map_offset + ALIGN(efi_map_sz, 16);
>  
>  	/* Copy setup header onto bootparams. Documentation/x86/boot.txt */
>  	setup_header_size = 0x0202 + kernel[0x0201] - setup_hdr_offset;

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference

[Mike Galbraith]

On Thu, 2018-08-09 at 12:21 +0800, Dave Young wrote:
> Hi Mike,
> 
> Thanks for the patch!
> On 08/08/18 at 04:03pm, Mike Galbraith wrote:
> > When booting with efi=noruntime, we call efi_runtime_map_copy() while
> > loading the kdump kernel, and trip over a NULL efi.memmap.map.  Avoid
> > that and a useless allocation when the only mapping we can use (1:1)
> > is not available.
> 
> At first glance, efi_get_runtime_map_size should return 0 in case
> noruntime.

I actually made it do that in a separate patch first, and keyed on that
in a second, but then decided to not notice anything odd in efi land
(run Forest run!), and just fix the bug that now bites latest RT due to
it turning efi runtime off by default.

> Also since we are here, would you mind to restructure the bzImage64_load
> function, and try to move all efi related code to setup_efi_state()?
> 
> 
> setup_boot_parameters(struct kimage *image, struct boot_params *params,
>                       unsigned long params_load_addr,
>                       unsigned int efi_map_offset, unsigned int efi_map_sz,
>                       unsigned int efi_setup_data_offset)
> {
> [snip]
> 
> #ifdef CONFIG_EFI
>         /* Setup EFI state */
>         setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
>                         efi_setup_data_offset);
> #endif
> 
> [snip]
> }
> 
> Currently bzImage64_load prepares the efi_map_offset, efi_map_sz,
>  and efi_setup_data_offset and then pass it to setup_boot_parameters and
> setup_efi_state.  It should be better to move those efi_* variables to
> setup_efi_state().
> 
> So we can call setup_efi_state only when efi runtime is enabled.

Yeah, I thought the same, but wanted to keep it dinky.

	-Mike

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference

[Mike Galbraith]

On Thu, 2018-08-09 at 12:21 +0800, Dave Young wrote:
> Hi Mike,
> 
> Thanks for the patch!
> On 08/08/18 at 04:03pm, Mike Galbraith wrote:
> > When booting with efi=noruntime, we call efi_runtime_map_copy() while
> > loading the kdump kernel, and trip over a NULL efi.memmap.map.  Avoid
> > that and a useless allocation when the only mapping we can use (1:1)
> > is not available.
> 
> At first glance, efi_get_runtime_map_size should return 0 in case
> noruntime.

What efi does internally at unmap time is to leave everything except
efi.mmap.map untouched, setting it to NULL and turning off EFI_MEMMAP,
rendering efi.mmap.map accessors useless/unsafe without first checking
EFI_MEMMAP.

	-Mike

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference

[Dave Young]

On 08/09/18 at 09:33am, Mike Galbraith wrote:
> On Thu, 2018-08-09 at 12:21 +0800, Dave Young wrote:
> > Hi Mike,
> > 
> > Thanks for the patch!
> > On 08/08/18 at 04:03pm, Mike Galbraith wrote:
> > > When booting with efi=noruntime, we call efi_runtime_map_copy() while
> > > loading the kdump kernel, and trip over a NULL efi.memmap.map.  Avoid
> > > that and a useless allocation when the only mapping we can use (1:1)
> > > is not available.
> > 
> > At first glance, efi_get_runtime_map_size should return 0 in case
> > noruntime.
> 
> What efi does internally at unmap time is to leave everything except
> efi.mmap.map untouched, setting it to NULL and turning off EFI_MEMMAP,
> rendering efi.mmap.map accessors useless/unsafe without first checking
> EFI_MEMMAP.

Probably the x86 efi_init should reset nr_map to zero in case runtime is
disabled.  But let's see how Ard thinks about this and cc linux-efi.

As for efi_get_runtime_map_size, it was introduced for x86 kexec use.
for copying runtime maps,  so I think it is reasonable this function
return zero in case no runtime.

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference

[Ard Biesheuvel]

On 9 August 2018 at 11:13, Dave Young <dyoung@redhat.com> wrote:
> On 08/09/18 at 09:33am, Mike Galbraith wrote:
>> On Thu, 2018-08-09 at 12:21 +0800, Dave Young wrote:
>> > Hi Mike,
>> >
>> > Thanks for the patch!
>> > On 08/08/18 at 04:03pm, Mike Galbraith wrote:
>> > > When booting with efi=noruntime, we call efi_runtime_map_copy() while
>> > > loading the kdump kernel, and trip over a NULL efi.memmap.map.  Avoid
>> > > that and a useless allocation when the only mapping we can use (1:1)
>> > > is not available.
>> >
>> > At first glance, efi_get_runtime_map_size should return 0 in case
>> > noruntime.
>>
>> What efi does internally at unmap time is to leave everything except
>> efi.mmap.map untouched, setting it to NULL and turning off EFI_MEMMAP,
>> rendering efi.mmap.map accessors useless/unsafe without first checking
>> EFI_MEMMAP.
>
> Probably the x86 efi_init should reset nr_map to zero in case runtime is
> disabled.  But let's see how Ard thinks about this and cc linux-efi.
>
> As for efi_get_runtime_map_size, it was introduced for x86 kexec use.
> for copying runtime maps,  so I think it is reasonable this function
> return zero in case no runtime.
>

I don't see the patch in the context so I cannot comment in great detail.

In any case, it is better to decouple EFI_MEMMAP from EFI_RUNTIME
dependencies. On x86, one may imply the other, but this is not
generally the case.

That means that efi_get_runtime_map_size() should probably check the
EFI_RUNTIME flag, and return 0 if it is cleared. Perhaps there are
other places where EFI_MEMMAP flag checks are missing, but I consider
that a separate issue.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference

[Dave Young]

On 08/21/18 at 03:39pm, Ard Biesheuvel wrote:
> On 9 August 2018 at 11:13, Dave Young <dyoung@redhat.com> wrote:
> > On 08/09/18 at 09:33am, Mike Galbraith wrote:
> >> On Thu, 2018-08-09 at 12:21 +0800, Dave Young wrote:
> >> > Hi Mike,
> >> >
> >> > Thanks for the patch!
> >> > On 08/08/18 at 04:03pm, Mike Galbraith wrote:
> >> > > When booting with efi=noruntime, we call efi_runtime_map_copy() while
> >> > > loading the kdump kernel, and trip over a NULL efi.memmap.map.  Avoid
> >> > > that and a useless allocation when the only mapping we can use (1:1)
> >> > > is not available.
> >> >
> >> > At first glance, efi_get_runtime_map_size should return 0 in case
> >> > noruntime.
> >>
> >> What efi does internally at unmap time is to leave everything except
> >> efi.mmap.map untouched, setting it to NULL and turning off EFI_MEMMAP,
> >> rendering efi.mmap.map accessors useless/unsafe without first checking
> >> EFI_MEMMAP.
> >
> > Probably the x86 efi_init should reset nr_map to zero in case runtime is
> > disabled.  But let's see how Ard thinks about this and cc linux-efi.
> >
> > As for efi_get_runtime_map_size, it was introduced for x86 kexec use.
> > for copying runtime maps,  so I think it is reasonable this function
> > return zero in case no runtime.
> >
> 
> I don't see the patch in the context so I cannot comment in great detail.

The patch is below:
https://lore.kernel.org/lkml/1533737025.4936.3.camel@gmx.de

> 
> In any case, it is better to decouple EFI_MEMMAP from EFI_RUNTIME
> dependencies. On x86, one may imply the other, but this is not
> generally the case.
> 
> That means that efi_get_runtime_map_size() should probably check the
> EFI_RUNTIME flag, and return 0 if it is cleared. Perhaps there are
> other places where EFI_MEMMAP flag checks are missing, but I consider
> that a separate issue.

Yes, I also agree with to check EFI_RUNTIME_SERVICES. There is no point for
efi_get_runtime_map_size to return a value other than 0 in case EFI_RUNTIME_SERVICES
is not set ie. via efi=noruntime

Is below patch acceptable?  The copy function can be changed to return
an error in case map size == 0, but that can be done later along with
the caller size cleanups in kexec code
---------------------------------------------------------------------------

efi: check EFI_RUNTIME_SERVICES flag in runtime map copying code

Mike reported a kexec_file_load NULL pointer dereference bug like below:
[    5.878262] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
[    5.879868] PGD 800000013c1f1067 P4D 800000013c1f1067 PUD 13aea7067 PMD 0 
[    5.881225] Oops: 0000 [#1] SMP PTI
[    5.882068] Modules linked in:
[    5.882851] CPU: 0 PID: 394 Comm: kexec Kdump: loaded Not tainted 4.17.0-rc2+ #648
[    5.884333] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
[    5.885843] RIP: 0010:memcpy_erms+0x6/0x10
[    5.886789] RSP: 0018:ffffc9000058bd00 EFLAGS: 00010246
[    5.887899] RAX: ffff880138e050b0 RBX: 00000000000980b0 RCX: 0000000000000ba0
[    5.889304] RDX: 0000000000000ba0 RSI: 0000000000000000 RDI: ffff880138e050b0
[    5.890977] RBP: ffff880138e04000 R08: 0000000000000017 R09: 0000000000000002
[    5.892524] R10: 0000000000099000 R11: 00000000000052d0 R12: 0000000039400200
[    5.893967] R13: ffff880138e05000 R14: 0000000000000ba0 R15: ffffc90000a4d000
[    5.895378] FS:  00007f167c9e6740(0000) GS:ffff88013fc00000(0000) knlGS:0000000000000000
[    5.896953] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    5.898143] CR2: 0000000000000000 CR3: 000000013c3ec002 CR4: 00000000001606f0
[    5.899542] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    5.900962] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[    5.902552] Call Trace:
[    5.903267]  efi_runtime_map_copy+0x28/0x30
[    5.904956]  bzImage64_load+0x59d/0x736
[    5.906881]  ? arch_kexec_kernel_image_load+0x6d/0x70
[    5.908243]  ? __se_sys_kexec_file_load+0x24b/0x750
[    5.909352]  ? _cond_resched+0x19/0x30
[    5.910286]  ? do_syscall_64+0x65/0x180
[    5.911229]  ? entry_SYSCALL_64_after_hwframe+0x44/0xa9
[    5.912365] Code: 90 90 90 90 90 eb 1e 0f 1f 00 48 89 f8 48 89 d1 48 c1 e9 03 83 e2 07 f3 48 a5 89 d1 f3 a4 c3 66 0f 1f 44 00 00 48 89 f8 48 89 d1 <f3> a4 c3 0f 1f 80 00 00 00 00 48 89 f8 48 83 fa 20 72 7e 40 38 
[    5.916235] RIP: memcpy_erms+0x6/0x10 RSP: ffffc9000058bd00
[    5.917507] CR2: 0000000000000000
[    5.918762] ---[ end trace 5cf4c4b3b93d7fdd ]---

Changing efi_get_runtime_map_size to return 0 in case runtime is
disabled.

Also moving to check EFI_RUNTIME_SERVICES in efi_runtime_map_copy 

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 drivers/firmware/efi/runtime-map.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

--- linux-x86.orig/drivers/firmware/efi/runtime-map.c
+++ linux-x86/drivers/firmware/efi/runtime-map.c
@@ -138,12 +138,18 @@ add_sysfs_runtime_map_entry(struct kobje
 
 int efi_get_runtime_map_size(void)
 {
-	return efi.memmap.nr_map * efi.memmap.desc_size;
+	if (efi_enabled(EFI_RUNTIME_SERVICES))
+		return efi.memmap.nr_map * efi.memmap.desc_size;
+
+	return 0;
 }
 
 int efi_get_runtime_map_desc_size(void)
 {
-	return efi.memmap.desc_size;
+	if (efi_enabled(EFI_RUNTIME_SERVICES))
+		return efi.memmap.desc_size;
+
+	return 0;
 }
 
 int efi_runtime_map_copy(void *buf, size_t bufsz)
@@ -163,7 +169,7 @@ int __init efi_runtime_map_init(struct k
 	struct efi_runtime_map_entry *entry;
 	efi_memory_desc_t *md;
 
-	if (!efi_enabled(EFI_MEMMAP))
+	if (!efi_enabled(EFI_RUNTIME_SERVICES))
 		return 0;
 
 	map_entries = kcalloc(efi.memmap.nr_map, sizeof(entry), GFP_KERNEL);


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference

[Dave Young]

On 08/22/18 at 06:23pm, Dave Young wrote:
> On 08/21/18 at 03:39pm, Ard Biesheuvel wrote:
> > On 9 August 2018 at 11:13, Dave Young <dyoung@redhat.com> wrote:
> > > On 08/09/18 at 09:33am, Mike Galbraith wrote:
> > >> On Thu, 2018-08-09 at 12:21 +0800, Dave Young wrote:
> > >> > Hi Mike,
> > >> >
> > >> > Thanks for the patch!
> > >> > On 08/08/18 at 04:03pm, Mike Galbraith wrote:
> > >> > > When booting with efi=noruntime, we call efi_runtime_map_copy() while
> > >> > > loading the kdump kernel, and trip over a NULL efi.memmap.map.  Avoid
> > >> > > that and a useless allocation when the only mapping we can use (1:1)
> > >> > > is not available.
> > >> >
> > >> > At first glance, efi_get_runtime_map_size should return 0 in case
> > >> > noruntime.
> > >>
> > >> What efi does internally at unmap time is to leave everything except
> > >> efi.mmap.map untouched, setting it to NULL and turning off EFI_MEMMAP,
> > >> rendering efi.mmap.map accessors useless/unsafe without first checking
> > >> EFI_MEMMAP.
> > >
> > > Probably the x86 efi_init should reset nr_map to zero in case runtime is
> > > disabled.  But let's see how Ard thinks about this and cc linux-efi.
> > >
> > > As for efi_get_runtime_map_size, it was introduced for x86 kexec use.
> > > for copying runtime maps,  so I think it is reasonable this function
> > > return zero in case no runtime.
> > >
> > 
> > I don't see the patch in the context so I cannot comment in great detail.
> 
> The patch is below:
> https://lore.kernel.org/lkml/1533737025.4936.3.camel@gmx.de
> 
> > 
> > In any case, it is better to decouple EFI_MEMMAP from EFI_RUNTIME
> > dependencies. On x86, one may imply the other, but this is not
> > generally the case.
> > 
> > That means that efi_get_runtime_map_size() should probably check the
> > EFI_RUNTIME flag, and return 0 if it is cleared. Perhaps there are
> > other places where EFI_MEMMAP flag checks are missing, but I consider
> > that a separate issue.
> 
> Yes, I also agree with to check EFI_RUNTIME_SERVICES. There is no point for
> efi_get_runtime_map_size to return a value other than 0 in case EFI_RUNTIME_SERVICES
> is not set ie. via efi=noruntime
> 
> Is below patch acceptable?  The copy function can be changed to return
> an error in case map size == 0, but that can be done later along with
> the caller size cleanups in kexec code

Forgot to add Mike's reported-by tag..

Mike, since we are going this way, I'm working on a kexec code cleanup,
but it needs careful testing so still need some time.

Can you help test below efi fix and provide you tested-by if it works?

> ---------------------------------------------------------------------------
> 
> efi: check EFI_RUNTIME_SERVICES flag in runtime map copying code
> 
> Mike reported a kexec_file_load NULL pointer dereference bug like below:
> [    5.878262] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
> [    5.879868] PGD 800000013c1f1067 P4D 800000013c1f1067 PUD 13aea7067 PMD 0 
> [    5.881225] Oops: 0000 [#1] SMP PTI
> [    5.882068] Modules linked in:
> [    5.882851] CPU: 0 PID: 394 Comm: kexec Kdump: loaded Not tainted 4.17.0-rc2+ #648
> [    5.884333] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
> [    5.885843] RIP: 0010:memcpy_erms+0x6/0x10
> [    5.886789] RSP: 0018:ffffc9000058bd00 EFLAGS: 00010246
> [    5.887899] RAX: ffff880138e050b0 RBX: 00000000000980b0 RCX: 0000000000000ba0
> [    5.889304] RDX: 0000000000000ba0 RSI: 0000000000000000 RDI: ffff880138e050b0
> [    5.890977] RBP: ffff880138e04000 R08: 0000000000000017 R09: 0000000000000002
> [    5.892524] R10: 0000000000099000 R11: 00000000000052d0 R12: 0000000039400200
> [    5.893967] R13: ffff880138e05000 R14: 0000000000000ba0 R15: ffffc90000a4d000
> [    5.895378] FS:  00007f167c9e6740(0000) GS:ffff88013fc00000(0000) knlGS:0000000000000000
> [    5.896953] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [    5.898143] CR2: 0000000000000000 CR3: 000000013c3ec002 CR4: 00000000001606f0
> [    5.899542] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [    5.900962] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [    5.902552] Call Trace:
> [    5.903267]  efi_runtime_map_copy+0x28/0x30
> [    5.904956]  bzImage64_load+0x59d/0x736
> [    5.906881]  ? arch_kexec_kernel_image_load+0x6d/0x70
> [    5.908243]  ? __se_sys_kexec_file_load+0x24b/0x750
> [    5.909352]  ? _cond_resched+0x19/0x30
> [    5.910286]  ? do_syscall_64+0x65/0x180
> [    5.911229]  ? entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [    5.912365] Code: 90 90 90 90 90 eb 1e 0f 1f 00 48 89 f8 48 89 d1 48 c1 e9 03 83 e2 07 f3 48 a5 89 d1 f3 a4 c3 66 0f 1f 44 00 00 48 89 f8 48 89 d1 <f3> a4 c3 0f 1f 80 00 00 00 00 48 89 f8 48 83 fa 20 72 7e 40 38 
> [    5.916235] RIP: memcpy_erms+0x6/0x10 RSP: ffffc9000058bd00
> [    5.917507] CR2: 0000000000000000
> [    5.918762] ---[ end trace 5cf4c4b3b93d7fdd ]---
> 
> Changing efi_get_runtime_map_size to return 0 in case runtime is
> disabled.
> 
> Also moving to check EFI_RUNTIME_SERVICES in efi_runtime_map_copy 
> 
> Signed-off-by: Dave Young <dyoung@redhat.com>
> ---
>  drivers/firmware/efi/runtime-map.c |   12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> --- linux-x86.orig/drivers/firmware/efi/runtime-map.c
> +++ linux-x86/drivers/firmware/efi/runtime-map.c
> @@ -138,12 +138,18 @@ add_sysfs_runtime_map_entry(struct kobje
>  
>  int efi_get_runtime_map_size(void)
>  {
> -	return efi.memmap.nr_map * efi.memmap.desc_size;
> +	if (efi_enabled(EFI_RUNTIME_SERVICES))
> +		return efi.memmap.nr_map * efi.memmap.desc_size;
> +
> +	return 0;
>  }
>  
>  int efi_get_runtime_map_desc_size(void)
>  {
> -	return efi.memmap.desc_size;
> +	if (efi_enabled(EFI_RUNTIME_SERVICES))
> +		return efi.memmap.desc_size;
> +
> +	return 0;
>  }
>  
>  int efi_runtime_map_copy(void *buf, size_t bufsz)
> @@ -163,7 +169,7 @@ int __init efi_runtime_map_init(struct k
>  	struct efi_runtime_map_entry *entry;
>  	efi_memory_desc_t *md;
>  
> -	if (!efi_enabled(EFI_MEMMAP))
> +	if (!efi_enabled(EFI_RUNTIME_SERVICES))
>  		return 0;
>  
>  	map_entries = kcalloc(efi.memmap.nr_map, sizeof(entry), GFP_KERNEL);
> 

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference

[Mike Galbraith]

On Thu, 2018-08-23 at 11:57 +0800, Dave Young wrote:
> 
> Mike, since we are going this way, I'm working on a kexec code cleanup,
> but it needs careful testing so still need some time.
> 
> Can you help test below efi fix and provide you tested-by if it works?

Sure.  I'm buried to the eyebrows atm (why no patchlet has appeared in
your inbox), but I'll get to it.

	-Mike

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference

[Mike Galbraith]

On Thu, 2018-08-23 at 11:57 +0800, Dave Young wrote:
> 
> Mike, since we are going this way, I'm working on a kexec code cleanup,
> but it needs careful testing so still need some time.
> 
> Can you help test below efi fix and provide you tested-by if it works?

While it averts the efi=noruntime oops on kdump kernel load, the kernel
does not boot when kdump is triggered.  Bailing in setup_efi_state() in
addition restores functionality.

	-Mike

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference

[Dave Young]

On 08/24/18 at 06:48am, Mike Galbraith wrote:
> On Thu, 2018-08-23 at 11:57 +0800, Dave Young wrote:
> > 
> > Mike, since we are going this way, I'm working on a kexec code cleanup,
> > but it needs careful testing so still need some time.
> > 
> > Can you help test below efi fix and provide you tested-by if it works?
> 
> While it averts the efi=noruntime oops on kdump kernel load, the kernel
> does not boot when kdump is triggered.  Bailing in setup_efi_state() in
> addition restores functionality.

Yes, that is expected.  Kexec code need fix and cleanup in other
patches.

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec