Re: [PATCH] mm, kasan: don't poison boot memory

From: George Kennedy
Date: Tue Feb 23 2021 - 16:28:28 EST

Next message: Song Bao Hua (Barry Song): "RE: [PATCH] scripts/gdb: document lx_current is only supported by x86"
Previous message: Arnd Bergmann: "Re: [PATCH] drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO"
In reply to: George Kennedy: "Re: [PATCH] mm, kasan: don't poison boot memory"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 2/23/2021 3:09 PM, Mike Rapoport wrote:

On Tue, Feb 23, 2021 at 01:05:05PM -0500, George Kennedy wrote:

On 2/23/2021 10:47 AM, Mike Rapoport wrote:

It now crashes here:

[    0.051019] ACPI: Early table checksum verification disabled
[    0.056721] ACPI: RSDP 0x00000000BFBFA014 000024 (v02 BOCHS )
[    0.057874] ACPI: XSDT 0x00000000BFBF90E8 00004C (v01 BOCHS BXPCFACP
00000001      01000013)
[    0.059590] ACPI: FACP 0x00000000BFBF5000 000074 (v01 BOCHS BXPCFACP
00000001 BXPC 00000001)
[    0.061306] ACPI: DSDT 0x00000000BFBF6000 00238D (v01 BOCHS BXPCDSDT
00000001 BXPC 00000001)
[    0.063006] ACPI: FACS 0x00000000BFBFD000 000040
[    0.063938] ACPI: APIC 0x00000000BFBF4000 000090 (v01 BOCHS BXPCAPIC
00000001 BXPC 00000001)
[    0.065638] ACPI: HPET 0x00000000BFBF3000 000038 (v01 BOCHS BXPCHPET
00000001 BXPC 00000001)
[    0.067335] ACPI: BGRT 0x00000000BE49B000 000038 (v01 INTEL EDK2
00000002      01000013)
[    0.069030] ACPI: iBFT 0x00000000BE453000 000800 (v01 BOCHS BXPCFACP
00000000      00000000)
[    0.070734] XXX acpi_find_ibft_region:
[    0.071468] XXX iBFT, status=0
[    0.072073] XXX about to call acpi_put_table()...
ibft_addr=ffffffffff240000
[    0.073449] XXX acpi_find_ibft_region(EXIT):
PANIC: early exception 0x0e IP 10:ffffffff9259f439 error 0 cr2
0xffffffffff240004

Right, I've missed the dereference of the ibft_addr after
acpi_find_ibft_region().

With this change to iscsi_ibft_find.c instead of the previous one it should
be better:

diff --git a/drivers/firmware/iscsi_ibft_find.c b/drivers/firmware/iscsi_ibft_find.c
index 64bb94523281..1be7481d5c69 100644
--- a/drivers/firmware/iscsi_ibft_find.c
+++ b/drivers/firmware/iscsi_ibft_find.c
@@ -80,6 +80,27 @@ static int __init find_ibft_in_mem(void)
done:
return len;
}
+
+static void __init acpi_find_ibft_region(unsigned long *sizep)
+{
+ int i;
+ struct acpi_table_header *table = NULL;
+ acpi_status status;
+
+ if (acpi_disabled)
+ return;
+
+ for (i = 0; i < ARRAY_SIZE(ibft_signs) && !ibft_addr; i++) {
+ status = acpi_get_table(ibft_signs[i].sign, 0, &table);
+ if (ACPI_SUCCESS(status)) {
+ ibft_addr = (struct acpi_table_ibft *)table;
+ *sizep = PAGE_ALIGN(ibft_addr->header.length);
+ acpi_put_table(table);
+ break;
+ }
+ }
+}
+
/*
* Routine used to find the iSCSI Boot Format Table. The logical
* kernel address is set in the ibft_addr global variable.
@@ -91,14 +112,16 @@ unsigned long __init find_ibft_region(unsigned long *sizep)
/* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will
* only use ACPI for this */
- if (!efi_enabled(EFI_BOOT))
+ if (!efi_enabled(EFI_BOOT)) {
find_ibft_in_mem();
-
- if (ibft_addr) {
*sizep = PAGE_ALIGN(ibft_addr->header.length);
- return (u64)virt_to_phys(ibft_addr);
+ } else {
+ acpi_find_ibft_region(sizep);
}
+ if (ibft_addr)
+ return (u64)virt_to_phys(ibft_addr);
+
*sizep = 0;
return 0;
}

Mike,

No luck. Back to the original KASAN ibft_init crash.

I ran with only the above patch from you. Was that what you wanted? Your previous patch had a section defined out by #if 0. Was that supposed to be in there as well?

If you need the console output let me know. Got bounced because it was too large.

[   30.124650] iBFT detected.
[   30.125228] ==================================================================
[   30.126201] BUG: KASAN: use-after-free in ibft_init+0x134/0xc33
[   30.126201] Read of size 4 at addr ffff8880be453004 by task swapper/0/1
[   30.126201]
[   30.126201] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.11.0-f9593a0 #9
[   30.126201] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.0.0 02/06/2015
[   30.126201] Call Trace:
[   30.126201] dump_stack+0xdb/0x120
[   30.126201] ? ibft_init+0x134/0xc33
[   30.126201] print_address_description.constprop.7+0x41/0x60
[   30.126201] ? ibft_init+0x134/0xc33
[   30.126201] ? ibft_init+0x134/0xc33
[   30.126201] kasan_report.cold.10+0x78/0xd1
[   30.126201] ? ibft_init+0x134/0xc33
[   30.126201] __asan_report_load_n_noabort+0xf/0x20
[   30.126201] ibft_init+0x134/0xc33
[   30.126201] ? write_comp_data+0x2f/0x90
[   30.126201] ? ibft_check_initiator_for+0x159/0x159
[   30.126201] ? write_comp_data+0x2f/0x90
[   30.126201] ? ibft_check_initiator_for+0x159/0x159
[   30.126201] do_one_initcall+0xc4/0x3e0
[   30.126201] ? perf_trace_initcall_level+0x3e0/0x3e0
[   30.126201] ? unpoison_range+0x14/0x40
[   30.126201] ? ____kasan_kmalloc.constprop.5+0x8f/0xc0
[   30.126201] ? kernel_init_freeable+0x420/0x652
[   30.126201] ? __kasan_kmalloc+0x9/0x10
[   30.126201] ? __sanitizer_cov_trace_pc+0x21/0x50
[   30.126201] kernel_init_freeable+0x596/0x652
[   30.126201] ? console_on_rootfs+0x7d/0x7d
[   30.126201] ? __sanitizer_cov_trace_pc+0x21/0x50
[   30.126201] ? rest_init+0xf0/0xf0
[   30.126201] kernel_init+0x16/0x1d0
[   30.126201] ? rest_init+0xf0/0xf0
[   30.126201] ret_from_fork+0x22/0x30
[   30.126201]
[   30.126201] The buggy address belongs to the page:
[   30.126201] page:0000000091b8f2b4 refcount:0 mapcount:0 mapping:0000000000000000 index:0x1 pfn:0xbe453
[   30.126201] flags: 0xfffffc0000000()
[   30.126201] raw: 000fffffc0000000 ffffea0002fac708 ffffea0002fac748 0000000000000000
[   30.126201] raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000
[   30.126201] page dumped because: kasan: bad access detected
[   30.126201] page_owner tracks the page as freed
[   30.126201] page last allocated via order 0, migratetype Movable, gfp_mask 0x100dca(GFP_HIGHUSER_MOVABLE|__GFP_ZERO), pid 204, ts 27975563827
[   30.126201] prep_new_page+0xfb/0x140
[   30.126201] get_page_from_freelist+0x3503/0x5730
[   30.126201] __alloc_pages_nodemask+0x2d8/0x650
[   30.126201] alloc_pages_vma+0xe2/0x560
[   30.126201] __handle_mm_fault+0x930/0x26c0
[   30.126201] handle_mm_fault+0x1f9/0x810
[   30.126201] do_user_addr_fault+0x6f7/0xca0
[   30.126201] exc_page_fault+0xaf/0x1a0
[   30.126201] asm_exc_page_fault+0x1e/0x30
[   30.126201] page last free stack trace:
[   30.126201] free_pcp_prepare+0x122/0x290
[   30.126201] free_unref_page_list+0xe6/0x490
[   30.126201] release_pages+0x2ed/0x1270
[   30.126201] free_pages_and_swap_cache+0x245/0x2e0
[   30.126201] tlb_flush_mmu+0x11e/0x680
[   30.126201] tlb_finish_mmu+0xa6/0x3e0
[   30.126201] exit_mmap+0x2b3/0x540
[   30.126201] mmput+0x11d/0x450
[   30.126201] do_exit+0xaa6/0x2d40
[   30.126201] do_group_exit+0x128/0x340
[   30.126201] __x64_sys_exit_group+0x43/0x50
[   30.126201] do_syscall_64+0x37/0x50
[   30.126201] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   30.126201]
[   30.126201] Memory state around the buggy address:
[   30.126201] ffff8880be452f00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   30.126201] ffff8880be452f80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   30.126201] >ffff8880be453000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   30.126201]                    ^
[   30.126201] ffff8880be453080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   30.126201] ffff8880be453100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   30.126201] ==================================================================

This is all I ran with:

# git diff
diff --git a/drivers/firmware/iscsi_ibft_find.c b/drivers/firmware/iscsi_ibft_find.c
index 64bb945..1be7481 100644
--- a/drivers/firmware/iscsi_ibft_find.c
+++ b/drivers/firmware/iscsi_ibft_find.c
@@ -80,6 +80,27 @@ static int __init find_ibft_in_mem(void)
done:
        return len;
}
+
+static void __init acpi_find_ibft_region(unsigned long *sizep)
+{
+       int i;
+       struct acpi_table_header *table = NULL;
+       acpi_status status;
+
+       if (acpi_disabled)
+               return;
+
+       for (i = 0; i < ARRAY_SIZE(ibft_signs) && !ibft_addr; i++) {
+               status = acpi_get_table(ibft_signs[i].sign, 0, &table);
+               if (ACPI_SUCCESS(status)) {
+                       ibft_addr = (struct acpi_table_ibft *)table;
+                       *sizep = PAGE_ALIGN(ibft_addr->header.length);
+                       acpi_put_table(table);
+                       break;
+               }
+       }
+}
+
/*
* Routine used to find the iSCSI Boot Format Table. The logical
* kernel address is set in the ibft_addr global variable.
@@ -91,14 +112,16 @@ unsigned long __init find_ibft_region(unsigned long *sizep)
        /* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will
         * only use ACPI for this */

-       if (!efi_enabled(EFI_BOOT))
+       if (!efi_enabled(EFI_BOOT)) {
                find_ibft_in_mem();
-
-       if (ibft_addr) {
                *sizep = PAGE_ALIGN(ibft_addr->header.length);
-               return (u64)virt_to_phys(ibft_addr);
+       } else {
+               acpi_find_ibft_region(sizep);
        }

+       if (ibft_addr)
+               return (u64)virt_to_phys(ibft_addr);
+
        *sizep = 0;
        return 0;
}

Thank you,
George

[    0.075711] CPU: 0 PID: 0 Comm: swapper Not tainted 5.11.0-34a2105 #8
[    0.076983] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
0.0.0 02/06/2015
[    0.078579] RIP: 0010:find_ibft_region+0x470/0x577

Next message: Song Bao Hua (Barry Song): "RE: [PATCH] scripts/gdb: document lx_current is only supported by x86"
Previous message: Arnd Bergmann: "Re: [PATCH] drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO"
In reply to: George Kennedy: "Re: [PATCH] mm, kasan: don't poison boot memory"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]