Messages in this thread | | | Subject | Re: [PATCH] mm, kasan: don't poison boot memory | From | George Kennedy <> | Date | Thu, 25 Feb 2021 10:22:44 -0500 |
| |
On 2/25/2021 9:57 AM, Mike Rapoport wrote: > On Thu, Feb 25, 2021 at 07:38:19AM -0500, George Kennedy wrote: >> On 2/25/2021 3:53 AM, Mike Rapoport wrote: >>> Hi George, >>> >>>> On 2/24/2021 5:37 AM, Mike Rapoport wrote: >>>>> On Tue, Feb 23, 2021 at 04:46:28PM -0500, George Kennedy wrote: >>>>>> Mike, >>>>>> >>>>>> Still no luck. >>>>>> >>>>>> [ 30.193723] iscsi: registered transport (iser) >>>>>> [ 30.195970] iBFT detected. >>>>>> [ 30.196571] BUG: unable to handle page fault for address: ffffffffff240004 >>>>> Hmm, we cannot set ibft_addr to early pointer to the ACPI table. >>>>> Let's try something more disruptive and move the reservation back to >>>>> iscsi_ibft_find.c. >>>>> >>>>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c >>>>> index 7bdc0239a943..c118dd54a747 100644 >>>>> --- a/arch/x86/kernel/acpi/boot.c >>>>> +++ b/arch/x86/kernel/acpi/boot.c >>>>> @@ -1551,6 +1551,7 @@ void __init acpi_boot_table_init(void) >>>>> if (acpi_disabled) >>>>> return; >>>>> +#if 0 >>>>> /* >>>>> * Initialize the ACPI boot-time table parser. >>>>> */ >>>>> @@ -1558,6 +1559,7 @@ void __init acpi_boot_table_init(void) >>>>> disable_acpi(); >>>>> return; >>>>> } >>>>> +#endif >>>>> acpi_table_parse(ACPI_SIG_BOOT, acpi_parse_sbf); >>>>> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c >>>>> index d883176ef2ce..c615ce96c9a2 100644 >>>>> --- a/arch/x86/kernel/setup.c >>>>> +++ b/arch/x86/kernel/setup.c >>>>> @@ -570,16 +570,6 @@ void __init reserve_standard_io_resources(void) >>>>> } >>>>> -static __init void reserve_ibft_region(void) >>>>> -{ >>>>> - unsigned long addr, size = 0; >>>>> - >>>>> - addr = find_ibft_region(&size); >>>>> - >>>>> - if (size) >>>>> - memblock_reserve(addr, size); >>>>> -} >>>>> - >>>>> static bool __init snb_gfx_workaround_needed(void) >>>>> { >>>>> #ifdef CONFIG_PCI >>>>> @@ -1032,6 +1022,12 @@ void __init setup_arch(char **cmdline_p) >>>>> */ >>>>> find_smp_config(); >>>>> + /* >>>>> + * Initialize the ACPI boot-time table parser. >>>>> + */ >>>>> + if (acpi_table_init()) >>>>> + disable_acpi(); >>>>> + >>>>> reserve_ibft_region(); >>>>> early_alloc_pgt_buf(); >>>>> diff --git a/drivers/firmware/iscsi_ibft_find.c b/drivers/firmware/iscsi_ibft_find.c >>>>> index 64bb94523281..01be513843d6 100644 >>>>> --- a/drivers/firmware/iscsi_ibft_find.c >>>>> +++ b/drivers/firmware/iscsi_ibft_find.c >>>>> @@ -47,7 +47,25 @@ static const struct { >>>>> #define VGA_MEM 0xA0000 /* VGA buffer */ >>>>> #define VGA_SIZE 0x20000 /* 128kB */ >>>>> -static int __init find_ibft_in_mem(void) >>>>> +static void __init *acpi_find_ibft_region(void) >>>>> +{ >>>>> + int i; >>>>> + struct acpi_table_header *table = NULL; >>>>> + acpi_status status; >>>>> + >>>>> + if (acpi_disabled) >>>>> + return NULL; >>>>> + >>>>> + 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)) >>>>> + return table; >>>>> + } >>>>> + >>>>> + return NULL; >>>>> +} >>>>> + >>>>> +static void __init *find_ibft_in_mem(void) >>>>> { >>>>> unsigned long pos; >>>>> unsigned int len = 0; >>>>> @@ -70,35 +88,44 @@ static int __init find_ibft_in_mem(void) >>>>> /* if the length of the table extends past 1M, >>>>> * the table cannot be valid. */ >>>>> if (pos + len <= (IBFT_END-1)) { >>>>> - ibft_addr = (struct acpi_table_ibft *)virt; >>>>> pr_info("iBFT found at 0x%lx.\n", pos); >>>>> - goto done; >>>>> + return virt; >>>>> } >>>>> } >>>>> } >>>>> } >>>>> -done: >>>>> - return len; >>>>> + >>>>> + return NULL; >>>>> } >>>>> + >>>>> +static void __init *find_ibft(void) >>>>> +{ >>>>> + /* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will >>>>> + * only use ACPI for this */ >>>>> + if (!efi_enabled(EFI_BOOT)) >>>>> + return find_ibft_in_mem(); >>>>> + else >>>>> + return acpi_find_ibft_region(); >>>>> +} >>>>> + >>>>> /* >>>>> * Routine used to find the iSCSI Boot Format Table. The logical >>>>> * kernel address is set in the ibft_addr global variable. >>>>> */ >>>>> -unsigned long __init find_ibft_region(unsigned long *sizep) >>>>> +void __init reserve_ibft_region(void) >>>>> { >>>>> - ibft_addr = NULL; >>>>> + struct acpi_table_ibft *table; >>>>> + unsigned long size; >>>>> - /* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will >>>>> - * only use ACPI for this */ >>>>> + table = find_ibft(); >>>>> + if (!table) >>>>> + return; >>>>> - 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); >>>>> - } >>>>> + size = PAGE_ALIGN(table->header.length); >>>>> + memblock_reserve(virt_to_phys(table), size); >>>>> - *sizep = 0; >>>>> - return 0; >>>>> + if (efi_enabled(EFI_BOOT)) >>>>> + acpi_put_table(&table->header); >>>>> + else >>>>> + ibft_addr = table; >>>>> } >>>>> diff --git a/include/linux/iscsi_ibft.h b/include/linux/iscsi_ibft.h >>>>> index b7b45ca82bea..da813c891990 100644 >>>>> --- a/include/linux/iscsi_ibft.h >>>>> +++ b/include/linux/iscsi_ibft.h >>>>> @@ -26,13 +26,9 @@ extern struct acpi_table_ibft *ibft_addr; >>>>> * mapped address is set in the ibft_addr variable. >>>>> */ >>>>> #ifdef CONFIG_ISCSI_IBFT_FIND >>>>> -unsigned long find_ibft_region(unsigned long *sizep); >>>>> +void reserve_ibft_region(void); >>>>> #else >>>>> -static inline unsigned long find_ibft_region(unsigned long *sizep) >>>>> -{ >>>>> - *sizep = 0; >>>>> - return 0; >>>>> -} >>>>> +static inline void reserve_ibft_region(void) {} >>>>> #endif >>>>> #endif /* ISCSI_IBFT_H */ >>>> Still no luck Mike, >>>> >>>> We're back to the original problem where the only thing that worked was to >>>> run "SetPageReserved(page)" before calling "kmap(page)". The page is being >>>> "freed" before ibft_init() is called as a result of the recent buddy page >>>> freeing changes. >>> I keep missing some little details each time :( >> No worries. Thanks for all your help. Does this patch go on top of your >> previous patch or is it standalone? > This is standalone. > >> George >>> Ok, let's try from the different angle. >>> >>> diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c >>> index 4b9b329a5a92..ec43e1447336 100644 >>> --- a/drivers/acpi/acpica/tbutils.c >>> +++ b/drivers/acpi/acpica/tbutils.c >>> @@ -7,6 +7,8 @@ >>> * >>> *****************************************************************************/ >>> +#include <linux/memblock.h> >>> + >>> #include <acpi/acpi.h> >>> #include "accommon.h" >>> #include "actables.h" >>> @@ -339,6 +341,21 @@ acpi_tb_parse_root_table(acpi_physical_address rsdp_address) >>> acpi_tb_parse_fadt(); >>> } >>> + if (ACPI_SUCCESS(status) && >>> + ACPI_COMPARE_NAMESEG(&acpi_gbl_root_table_list. >>> + tables[table_index].signature, >>> + ACPI_SIG_IBFT)) { >>> + struct acpi_table_header *ibft; >>> + struct acpi_table_desc *desc; >>> + >>> + desc = &acpi_gbl_root_table_list.tables[table_index]; >>> + status = acpi_tb_get_table(desc, &ibft); >>> + if (ACPI_SUCCESS(status)) { >>> + memblock_reserve(address, ibft->length); >>> + acpi_tb_put_table(desc); >>> + >>> + } >>> + >>> next_table: >>> table_entry += table_entry_size; >>> >>> Applied just your latest patch, but same failure.
I thought there was an earlier comment (which I can't find now) that stated that memblock_reserve() wouldn't reserve the page, which is what's needed here.
[ 30.308229] iBFT detected.. [ 30.308796] ================================================================== [ 30.308890] BUG: KASAN: use-after-free in ibft_init+0x134/0xc33 [ 30.308890] Read of size 4 at addr ffff8880be453004 by task swapper/0/1 [ 30.308890] [ 30.308890] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.11.0-f9593a0 #12 [ 30.308890] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.0.0 02/06/2015 [ 30.308890] Call Trace: [ 30.308890] dump_stack+0xdb/0x120 [ 30.308890] ? ibft_init+0x134/0xc33 [ 30.308890] print_address_description.constprop.7+0x41/0x60 [ 30.308890] ? ibft_init+0x134/0xc33 [ 30.308890] ? ibft_init+0x134/0xc33 [ 30.308890] kasan_report.cold.10+0x78/0xd1 [ 30.308890] ? ibft_init+0x134/0xc33 [ 30.308890] __asan_report_load_n_noabort+0xf/0x20 [ 30.308890] ibft_init+0x134/0xc33 [ 30.308890] ? write_comp_data+0x2f/0x90 [ 30.308890] ? ibft_check_initiator_for+0x159/0x159 [ 30.308890] ? write_comp_data+0x2f/0x90 [ 30.308890] ? ibft_check_initiator_for+0x159/0x159 [ 30.308890] do_one_initcall+0xc4/0x3e0 [ 30.308890] ? perf_trace_initcall_level+0x3e0/0x3e0 [ 30.308890] ? unpoison_range+0x14/0x40 [ 30.308890] ? ____kasan_kmalloc.constprop.5+0x8f/0xc0 [ 30.308890] ? kernel_init_freeable+0x420/0x652 [ 30.308890] ? __kasan_kmalloc+0x9/0x10 [ 30.308890] ? __sanitizer_cov_trace_pc+0x21/0x50 [ 30.308890] kernel_init_freeable+0x596/0x652 [ 30.308890] ? console_on_rootfs+0x7d/0x7d [ 30.308890] ? __sanitizer_cov_trace_pc+0x21/0x50 [ 30.308890] ? rest_init+0xf0/0xf0 [ 30.308890] kernel_init+0x16/0x1d0 [ 30.308890] ? rest_init+0xf0/0xf0 [ 30.308890] ret_from_fork+0x22/0x30 [ 30.308890] [ 30.308890] The buggy address belongs to the page: [ 30.308890] page:0000000001b7b17c refcount:0 mapcount:0 mapping:0000000000000000 index:0x1 pfn:0xbe453 [ 30.308890] flags: 0xfffffc0000000() [ 30.308890] raw: 000fffffc0000000 ffffea0002ef9788 ffffea0002f91488 0000000000000000 [ 30.308890] raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000 [ 30.308890] page dumped because: kasan: bad access detected [ 30.308890] page_owner tracks the page as freed [ 30.308890] page last allocated via order 0, migratetype Movable, gfp_mask 0x100dca(GFP_HIGHUSER_MOVABLE|__GFP_ZERO), pid 204, ts 28121288605 [ 30.308890] prep_new_page+0xfb/0x140 [ 30.308890] get_page_from_freelist+0x3503/0x5730 [ 30.308890] __alloc_pages_nodemask+0x2d8/0x650 [ 30.308890] alloc_pages_vma+0xe2/0x560 [ 30.308890] __handle_mm_fault+0x930/0x26c0 [ 30.308890] handle_mm_fault+0x1f9/0x810 [ 30.308890] do_user_addr_fault+0x6f7/0xca0 [ 30.308890] exc_page_fault+0xaf/0x1a0 [ 30.308890] asm_exc_page_fault+0x1e/0x30 [ 30.308890] page last free stack trace: [ 30.308890] free_pcp_prepare+0x122/0x290 [ 30.308890] free_unref_page_list+0xe6/0x490 [ 30.308890] release_pages+0x2ed/0x1270 [ 30.308890] free_pages_and_swap_cache+0x245/0x2e0 [ 30.308890] tlb_flush_mmu+0x11e/0x680 [ 30.308890] tlb_finish_mmu+0xa6/0x3e0 [ 30.308890] exit_mmap+0x2b3/0x540 [ 30.308890] mmput+0x11d/0x450 [ 30.308890] do_exit+0xaa6/0x2d40 [ 30.308890] do_group_exit+0x128/0x340 [ 30.308890] __x64_sys_exit_group+0x43/0x50 [ 30.308890] do_syscall_64+0x37/0x50 [ 30.308890] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 30.308890] [ 30.308890] Memory state around the buggy address: [ 30.308890] ffff8880be452f00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 30.308890] ffff8880be452f80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 30.308890] >ffff8880be453000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 30.308890] ^ [ 30.308890] ffff8880be453080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 30.308890] ffff8880be453100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 30.308890] ==================================================================
George
| |