efi: Fix the size not consistent issue when unmapping memory map

When using kdump, SOMETIMES the "size not consistent" warning message shows up when the crash kernel boots with early_ioremap_debug parameter:

WARNING: CPU: 0 PID: 0 at ../mm/early_ioremap.c:182 early_iounmap+0x4f/0x12c()

early_iounmap(ffffffffff200180, 00000118) [0] size not consistent 00000120

The root cause is that the unmapping size of memory map doesn't match with the original size when mapping:

in __efi_memmap_init()

	map.map = early_memremap(phys_map, data->size);

in efi_memmap_unmap()

        size = efi.memmap.desc_size * efi.memmap.nr_map;
        early_memunmap(efi.memmap.map, size);

But the efi.memmap.nr_map is from __efi_memmap_init(). The remainder of size was discarded when calculating the nr_map:

        map.nr_map = data->size / data->desc_size;

When the original size of memory map region does not equal to the result of multiplication. The "size not consistent" warning will be triggered.

This issue sometimes was hit by kdump because kexec set the efi map size to align with 16 when loading crash kernel image:

in bzImage64_load()

        efi_map_sz = efi_get_runtime_map_size();
        efi_map_sz = ALIGN(efi_map_sz, 16);

This patch changes the logic in the unmapping function. Using the end address of map to calcuate original size.

Thank Randy Wright for his report and testing. And also thank Takashi Iwai for his help to trace issue.

Cc Ard Biesheuvel <[email protected]>

Cc Randy Wright <[email protected]>

Cc Takashi Iwai <[email protected]>

Cc Vivek Goyal <[email protected]>

Cc Ingo Molnar <[email protected]>

Signed-off-by "Lee, Chun-Yi" <[email protected]>

---
 drivers/firmware/efi/memmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/firmware/efi/memmap.c +1 -1

index 5fc7052..1f592d8 100644
@@ -121,7 +121,7 @@ void __init efi_memmap_unmap(void)
 	if (!efi.memmap.late) {
 		unsigned long size;
 
-		size = efi.memmap.desc_size * efi.memmap.nr_map;
+		size = efi.memmap.map_end - efi.memmap.map;
 		early_memunmap(efi.memmap.map, size);
 	} else {
 		memunmap(efi.memmap.map);

Signature · 7 lines

2.10.2


_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Quoted 34 lines · Chun-Yi

When using kdump, SOMETIMES the "size not consistent" warning message shows up when the crash kernel boots with early_ioremap_debug parameter:

WARNING: CPU: 0 PID: 0 at ../mm/early_ioremap.c:182 early_iounmap+0x4f/0x12c()

early_iounmap(ffffffffff200180, 00000118) [0] size not consistent 00000120

The root cause is that the unmapping size of memory map doesn't match with the original size when mapping:

in __efi_memmap_init()

	map.map = early_memremap(phys_map, data->size);

in efi_memmap_unmap()

        size = efi.memmap.desc_size * efi.memmap.nr_map;
        early_memunmap(efi.memmap.map, size);

But the efi.memmap.nr_map is from __efi_memmap_init(). The remainder of size was discarded when calculating the nr_map:

        map.nr_map = data->size / data->desc_size;

When the original size of memory map region does not equal to the result of multiplication. The "size not consistent" warning will be triggered.

This issue sometimes was hit by kdump because kexec set the efi map size to align with 16 when loading crash kernel image:

in bzImage64_load()

        efi_map_sz = efi_get_runtime_map_size();
        efi_map_sz = ALIGN(efi_map_sz, 16);

This patch changes the logic in the unmapping function. Using the end address of map to calcuate original size.

Quoted 2 lines

Thank Randy Wright for his report and testing. And also thank Takashi Iwai for his help to trace issue.

Good catch. The kexec code need to be fixed to use a separate buffer so avoid the alignment like what kexec-tools did. I can submit a fix for that.

Quoted 7 lines · Dave Young

Quoted 6 lines · Chun-Yi

When using kdump, SOMETIMES the "size not consistent" warning message shows up when the crash kernel boots with early_ioremap_debug parameter:

WARNING: CPU: 0 PID: 0 at ../mm/early_ioremap.c:182 early_iounmap+0x4f/0x12c()

early_iounmap(ffffffffff200180, 00000118) [0] size not consistent 00000120

Quoted 4 lines

Good catch. The kexec code need to be fixed to use a separate buffer so avoid the alignment like what kexec-tools did. I can submit a fix for that.

Can you try below code, see if it works?

arch/x86/kernel/kexec-bzimage64.c +2 -3

index 3182908b7e6c..eaee37c54b7b 100644
@@ -398,11 +398,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
 	 * little bit simple
 	 */
 	efi_map_sz = efi_get_runtime_map_size();
-	efi_map_sz = ALIGN(efi_map_sz, 16);
 	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 + efi_map_sz +
+	kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16)+
 				sizeof(struct setup_data) +
 				sizeof(struct efi_setup_data);
 

@@ -410,7 +409,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
 	if (!params)
 		return ERR_PTR(-ENOMEM);
 	efi_map_offset = params_cmdline_sz;
-	efi_setup_data_offset = efi_map_offset + efi_map_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;

Hi Randy,
...
Randy, do you want to try Dave's kexec patch on your environment? Please remove my patch first.

Thanks a lot!
Joey Lee

Quoted 7 lines · joeyli

Hi Randy,
...
Randy, do you want to try Dave's kexec patch on your environment? Please remove my patch first.

Thanks a lot!
Joey Lee

Hi Joey,

I tried Dave's patch to kexec-bzimage64.c on my build of the SuSE
4.12.14-15 kernel. I ran the same test as I did with your patch: I
verified the early_ioremap.c warnings occurred with a crash triggered from a kexec boot of the unmodified kernel. Then I applied the patch to kexec-bzimage64.c, rebuilt, re-ran the test to crash from the kexec'ed kernel, and verified the warnings are no longer seen.

I'm out of time today, but I will plan to run the same test tomorrow on a build of the SuSE 4.4.120-94.17 kernel, on which I had also reported the original bug.

Quoted 7 lines · joeyli

Hi Randy,
...
Randy, do you want to try Dave's kexec patch on your environment? Please remove my patch first.

Thanks a lot!
Joey Lee

Hi Joey,

I tried Dave's patch to kexec-bzimage64.c on my build of the SuSE
4.12.14-15 kernel. I ran the same test as I did with your patch: I
verified the early_ioremap.c warnings occurred with a crash triggered from a kexec boot of the unmodified kernel. Then I applied the patch to kexec-bzimage64.c, rebuilt, re-ran the test to crash from the kexec'ed kernel, and verified the warnings are no longer seen.

... I will plan to run the same test tomorrow on
a build of the SuSE 4.4.120-94.17 kernel, on which I had also reported the original bug.

Quoted 34 lines · Chun-Yi

When using kdump, SOMETIMES the "size not consistent" warning message shows up when the crash kernel boots with early_ioremap_debug parameter:

WARNING: CPU: 0 PID: 0 at ../mm/early_ioremap.c:182 early_iounmap+0x4f/0x12c()

early_iounmap(ffffffffff200180, 00000118) [0] size not consistent 00000120

The root cause is that the unmapping size of memory map doesn't match with the original size when mapping:

in __efi_memmap_init()

	map.map = early_memremap(phys_map, data->size);

in efi_memmap_unmap()

        size = efi.memmap.desc_size * efi.memmap.nr_map;
        early_memunmap(efi.memmap.map, size);

But the efi.memmap.nr_map is from __efi_memmap_init(). The remainder of size was discarded when calculating the nr_map:

        map.nr_map = data->size / data->desc_size;

When the original size of memory map region does not equal to the result of multiplication. The "size not consistent" warning will be triggered.

This issue sometimes was hit by kdump because kexec set the efi map size to align with 16 when loading crash kernel image:

in bzImage64_load()

        efi_map_sz = efi_get_runtime_map_size();
        efi_map_sz = ALIGN(efi_map_sz, 16);

This patch changes the logic in the unmapping function. Using the end address of map to calcuate original size.

Quoted 2 lines

Thank Randy Wright for his report and testing. And also thank Takashi Iwai for his help to trace issue.

Good catch. The kexec code need to be fixed to use a separate buffer so avoid the alignment like what kexec-tools did. I can submit a fix for that.

But this issue could be a potential issue even if kexec get fixed so it looks worth a fix in efi code as well. How about mapping only nr_maps *desc_size in __efi_memmap_init? It looks easier to understand.