| Message ID | 4650320.31r3eYUQgx@kreacher |
|---|---|
| State | New |
| Headers | show |
| Series | ACPI: tables: x86: Reserve memory occupied by ACPI tables | expand |
On Tue, Mar 23, 2021 at 08:26:52PM +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > The following problem has been reported by George Kennedy: > > Since commit 7fef431be9c9 ("mm/page_alloc: place pages to tail > in __free_pages_core()") the following use after free occurs > intermittently when ACPI tables are accessed. > > BUG: KASAN: use-after-free in ibft_init+0x134/0xc49 > Read of size 4 at addr ffff8880be453004 by task swapper/0/1 > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.12.0-rc1-7a7fd0d #1 > Call Trace: > dump_stack+0xf6/0x158 > print_address_description.constprop.9+0x41/0x60 > kasan_report.cold.14+0x7b/0xd4 > __asan_report_load_n_noabort+0xf/0x20 > ibft_init+0x134/0xc49 > do_one_initcall+0xc4/0x3e0 > kernel_init_freeable+0x5af/0x66b > kernel_init+0x16/0x1d0 > ret_from_fork+0x22/0x30 > > ACPI tables mapped via kmap() do not have their mapped pages > reserved and the pages can be "stolen" by the buddy allocator. > > Apparently, on the affected system, the ACPI table in question is > not located in "reserved" memory, like ACPI NVS or ACPI Data, that > will not be used by the buddy allocator, so the memory occupied by > that table has to be explicitly reserved to prevent the buddy > allocator from using it. > > In order to address this problem, rearrange the initialization of the > ACPI tables on x86 to locate the initial tables earlier and reserve > the memory occupied by them. > > The other architectures using ACPI should not be affected by this > change. > > Link: https://lore.kernel.org/linux-acpi/1614802160-29362-1-git-send-email-george.kennedy@oracle.com/ > Reported-by: George Kennedy <george.kennedy@oracle.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> FWIW: Reviewed-by: Mike Rapoport <rppt@linux.ibm.com> > --- > arch/x86/kernel/acpi/boot.c | 25 ++++++++++++------------- > arch/x86/kernel/setup.c | 8 +++----- > drivers/acpi/tables.c | 42 +++++++++++++++++++++++++++++++++++++++--- > include/linux/acpi.h | 9 ++++++++- > 4 files changed, 62 insertions(+), 22 deletions(-) > > Index: linux-pm/arch/x86/kernel/acpi/boot.c > =================================================================== > --- linux-pm.orig/arch/x86/kernel/acpi/boot.c > +++ linux-pm/arch/x86/kernel/acpi/boot.c > @@ -1554,10 +1554,18 @@ void __init acpi_boot_table_init(void) > /* > * Initialize the ACPI boot-time table parser. > */ > - if (acpi_table_init()) { > + if (acpi_locate_initial_tables()) > disable_acpi(); > - return; > - } > + else > + acpi_reserve_initial_tables(); > +} > + > +int __init early_acpi_boot_init(void) > +{ > + if (acpi_disabled) > + return 1; > + > + acpi_table_init_complete(); > > acpi_table_parse(ACPI_SIG_BOOT, acpi_parse_sbf); > > @@ -1570,18 +1578,9 @@ void __init acpi_boot_table_init(void) > } else { > printk(KERN_WARNING PREFIX "Disabling ACPI support\n"); > disable_acpi(); > - return; > + return 1; > } > } > -} > - > -int __init early_acpi_boot_init(void) > -{ > - /* > - * If acpi_disabled, bail out > - */ > - if (acpi_disabled) > - return 1; > > /* > * Process the Multiple APIC Description Table (MADT), if present > Index: linux-pm/arch/x86/kernel/setup.c > =================================================================== > --- linux-pm.orig/arch/x86/kernel/setup.c > +++ linux-pm/arch/x86/kernel/setup.c > @@ -1045,6 +1045,9 @@ void __init setup_arch(char **cmdline_p) > > cleanup_highmap(); > > + /* Look for ACPI tables and reserve memory occupied by them. */ > + acpi_boot_table_init(); > + > memblock_set_current_limit(ISA_END_ADDRESS); > e820__memblock_setup(); > > @@ -1136,11 +1139,6 @@ void __init setup_arch(char **cmdline_p) > > early_platform_quirks(); > > - /* > - * Parse the ACPI tables for possible boot-time SMP configuration. > - */ > - acpi_boot_table_init(); > - > early_acpi_boot_init(); > > initmem_init(); > Index: linux-pm/include/linux/acpi.h > =================================================================== > --- linux-pm.orig/include/linux/acpi.h > +++ linux-pm/include/linux/acpi.h > @@ -222,10 +222,14 @@ void __iomem *__acpi_map_table(unsigned > void __acpi_unmap_table(void __iomem *map, unsigned long size); > int early_acpi_boot_init(void); > int acpi_boot_init (void); > +void acpi_boot_table_prepare (void); > void acpi_boot_table_init (void); > int acpi_mps_check (void); > int acpi_numa_init (void); > > +int acpi_locate_initial_tables (void); > +void acpi_reserve_initial_tables (void); > +void acpi_table_init_complete (void); > int acpi_table_init (void); > int acpi_table_parse(char *id, acpi_tbl_table_handler handler); > int __init acpi_table_parse_entries(char *id, unsigned long table_size, > @@ -814,9 +818,12 @@ static inline int acpi_boot_init(void) > return 0; > } > > +static inline void acpi_boot_table_prepare(void) > +{ > +} > + > static inline void acpi_boot_table_init(void) > { > - return; > } > > static inline int acpi_mps_check(void) > Index: linux-pm/drivers/acpi/tables.c > =================================================================== > --- linux-pm.orig/drivers/acpi/tables.c > +++ linux-pm/drivers/acpi/tables.c > @@ -780,7 +780,7 @@ acpi_status acpi_os_table_override(struc > } > > /* > - * acpi_table_init() > + * acpi_locate_initial_tables() > * > * find RSDP, find and checksum SDT/XSDT. > * checksum all tables, print SDT/XSDT > @@ -788,7 +788,7 @@ acpi_status acpi_os_table_override(struc > * result: sdt_entry[] is initialized > */ > > -int __init acpi_table_init(void) > +int __init acpi_locate_initial_tables(void) > { > acpi_status status; > > @@ -803,9 +803,45 @@ int __init acpi_table_init(void) > status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0); > if (ACPI_FAILURE(status)) > return -EINVAL; > - acpi_table_initrd_scan(); > > + return 0; > +} > + > +void __init acpi_reserve_initial_tables(void) > +{ > + int i; > + > + for (i = 0; i < ACPI_MAX_TABLES; i++) { > + struct acpi_table_desc *table_desc = &initial_tables[i]; > + u64 start = table_desc->address; > + u64 size = table_desc->length; > + > + if (!start || !size) > + break; > + > + pr_info("Reserving %4s table memory at [mem 0x%llx-0x%llx]\n", > + table_desc->signature.ascii, start, start + size - 1); > + > + memblock_reserve(start, size); > + } > +} > + > +void __init acpi_table_init_complete(void) > +{ > + acpi_table_initrd_scan(); > check_multiple_madt(); > +} > + > +int __init acpi_table_init(void) > +{ > + int ret; > + > + ret = acpi_locate_initial_tables(); > + if (ret) > + return ret; > + > + acpi_table_init_complete(); > + > return 0; > } > > > > -- Sincerely yours, Mike.
On Wed, Mar 24, 2021 at 9:24 AM Mike Rapoport <rppt@linux.ibm.com> wrote: > > On Tue, Mar 23, 2021 at 08:26:52PM +0100, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > The following problem has been reported by George Kennedy: > > > > Since commit 7fef431be9c9 ("mm/page_alloc: place pages to tail > > in __free_pages_core()") the following use after free occurs > > intermittently when ACPI tables are accessed. > > > > BUG: KASAN: use-after-free in ibft_init+0x134/0xc49 > > Read of size 4 at addr ffff8880be453004 by task swapper/0/1 > > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.12.0-rc1-7a7fd0d #1 > > Call Trace: > > dump_stack+0xf6/0x158 > > print_address_description.constprop.9+0x41/0x60 > > kasan_report.cold.14+0x7b/0xd4 > > __asan_report_load_n_noabort+0xf/0x20 > > ibft_init+0x134/0xc49 > > do_one_initcall+0xc4/0x3e0 > > kernel_init_freeable+0x5af/0x66b > > kernel_init+0x16/0x1d0 > > ret_from_fork+0x22/0x30 > > > > ACPI tables mapped via kmap() do not have their mapped pages > > reserved and the pages can be "stolen" by the buddy allocator. > > > > Apparently, on the affected system, the ACPI table in question is > > not located in "reserved" memory, like ACPI NVS or ACPI Data, that > > will not be used by the buddy allocator, so the memory occupied by > > that table has to be explicitly reserved to prevent the buddy > > allocator from using it. > > > > In order to address this problem, rearrange the initialization of the > > ACPI tables on x86 to locate the initial tables earlier and reserve > > the memory occupied by them. > > > > The other architectures using ACPI should not be affected by this > > change. > > > > Link: https://lore.kernel.org/linux-acpi/1614802160-29362-1-git-send-email-george.kennedy@oracle.com/ > > Reported-by: George Kennedy <george.kennedy@oracle.com> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > FWIW: > Reviewed-by: Mike Rapoport <rppt@linux.ibm.com> Thank you! George, can you please try this patch on the affected system? > > --- > > arch/x86/kernel/acpi/boot.c | 25 ++++++++++++------------- > > arch/x86/kernel/setup.c | 8 +++----- > > drivers/acpi/tables.c | 42 +++++++++++++++++++++++++++++++++++++++--- > > include/linux/acpi.h | 9 ++++++++- > > 4 files changed, 62 insertions(+), 22 deletions(-) > > > > Index: linux-pm/arch/x86/kernel/acpi/boot.c > > =================================================================== > > --- linux-pm.orig/arch/x86/kernel/acpi/boot.c > > +++ linux-pm/arch/x86/kernel/acpi/boot.c > > @@ -1554,10 +1554,18 @@ void __init acpi_boot_table_init(void) > > /* > > * Initialize the ACPI boot-time table parser. > > */ > > - if (acpi_table_init()) { > > + if (acpi_locate_initial_tables()) > > disable_acpi(); > > - return; > > - } > > + else > > + acpi_reserve_initial_tables(); > > +} > > + > > +int __init early_acpi_boot_init(void) > > +{ > > + if (acpi_disabled) > > + return 1; > > + > > + acpi_table_init_complete(); > > > > acpi_table_parse(ACPI_SIG_BOOT, acpi_parse_sbf); > > > > @@ -1570,18 +1578,9 @@ void __init acpi_boot_table_init(void) > > } else { > > printk(KERN_WARNING PREFIX "Disabling ACPI support\n"); > > disable_acpi(); > > - return; > > + return 1; > > } > > } > > -} > > - > > -int __init early_acpi_boot_init(void) > > -{ > > - /* > > - * If acpi_disabled, bail out > > - */ > > - if (acpi_disabled) > > - return 1; > > > > /* > > * Process the Multiple APIC Description Table (MADT), if present > > Index: linux-pm/arch/x86/kernel/setup.c > > =================================================================== > > --- linux-pm.orig/arch/x86/kernel/setup.c > > +++ linux-pm/arch/x86/kernel/setup.c > > @@ -1045,6 +1045,9 @@ void __init setup_arch(char **cmdline_p) > > > > cleanup_highmap(); > > > > + /* Look for ACPI tables and reserve memory occupied by them. */ > > + acpi_boot_table_init(); > > + > > memblock_set_current_limit(ISA_END_ADDRESS); > > e820__memblock_setup(); > > > > @@ -1136,11 +1139,6 @@ void __init setup_arch(char **cmdline_p) > > > > early_platform_quirks(); > > > > - /* > > - * Parse the ACPI tables for possible boot-time SMP configuration. > > - */ > > - acpi_boot_table_init(); > > - > > early_acpi_boot_init(); > > > > initmem_init(); > > Index: linux-pm/include/linux/acpi.h > > =================================================================== > > --- linux-pm.orig/include/linux/acpi.h > > +++ linux-pm/include/linux/acpi.h > > @@ -222,10 +222,14 @@ void __iomem *__acpi_map_table(unsigned > > void __acpi_unmap_table(void __iomem *map, unsigned long size); > > int early_acpi_boot_init(void); > > int acpi_boot_init (void); > > +void acpi_boot_table_prepare (void); > > void acpi_boot_table_init (void); > > int acpi_mps_check (void); > > int acpi_numa_init (void); > > > > +int acpi_locate_initial_tables (void); > > +void acpi_reserve_initial_tables (void); > > +void acpi_table_init_complete (void); > > int acpi_table_init (void); > > int acpi_table_parse(char *id, acpi_tbl_table_handler handler); > > int __init acpi_table_parse_entries(char *id, unsigned long table_size, > > @@ -814,9 +818,12 @@ static inline int acpi_boot_init(void) > > return 0; > > } > > > > +static inline void acpi_boot_table_prepare(void) > > +{ > > +} > > + > > static inline void acpi_boot_table_init(void) > > { > > - return; > > } > > > > static inline int acpi_mps_check(void) > > Index: linux-pm/drivers/acpi/tables.c > > =================================================================== > > --- linux-pm.orig/drivers/acpi/tables.c > > +++ linux-pm/drivers/acpi/tables.c > > @@ -780,7 +780,7 @@ acpi_status acpi_os_table_override(struc > > } > > > > /* > > - * acpi_table_init() > > + * acpi_locate_initial_tables() > > * > > * find RSDP, find and checksum SDT/XSDT. > > * checksum all tables, print SDT/XSDT > > @@ -788,7 +788,7 @@ acpi_status acpi_os_table_override(struc > > * result: sdt_entry[] is initialized > > */ > > > > -int __init acpi_table_init(void) > > +int __init acpi_locate_initial_tables(void) > > { > > acpi_status status; > > > > @@ -803,9 +803,45 @@ int __init acpi_table_init(void) > > status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0); > > if (ACPI_FAILURE(status)) > > return -EINVAL; > > - acpi_table_initrd_scan(); > > > > + return 0; > > +} > > + > > +void __init acpi_reserve_initial_tables(void) > > +{ > > + int i; > > + > > + for (i = 0; i < ACPI_MAX_TABLES; i++) { > > + struct acpi_table_desc *table_desc = &initial_tables[i]; > > + u64 start = table_desc->address; > > + u64 size = table_desc->length; > > + > > + if (!start || !size) > > + break; > > + > > + pr_info("Reserving %4s table memory at [mem 0x%llx-0x%llx]\n", > > + table_desc->signature.ascii, start, start + size - 1); > > + > > + memblock_reserve(start, size); > > + } > > +} > > + > > +void __init acpi_table_init_complete(void) > > +{ > > + acpi_table_initrd_scan(); > > check_multiple_madt(); > > +} > > + > > +int __init acpi_table_init(void) > > +{ > > + int ret; > > + > > + ret = acpi_locate_initial_tables(); > > + if (ret) > > + return ret; > > + > > + acpi_table_init_complete(); > > + > > return 0; > > } > > > > > > > > > > -- > Sincerely yours, > Mike.
On 3/24/2021 9:27 AM, Rafael J. Wysocki wrote: > On Wed, Mar 24, 2021 at 9:24 AM Mike Rapoport <rppt@linux.ibm.com> wrote: >> On Tue, Mar 23, 2021 at 08:26:52PM +0100, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> >>> The following problem has been reported by George Kennedy: >>> >>> Since commit 7fef431be9c9 ("mm/page_alloc: place pages to tail >>> in __free_pages_core()") the following use after free occurs >>> intermittently when ACPI tables are accessed. >>> >>> BUG: KASAN: use-after-free in ibft_init+0x134/0xc49 >>> Read of size 4 at addr ffff8880be453004 by task swapper/0/1 >>> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.12.0-rc1-7a7fd0d #1 >>> Call Trace: >>> dump_stack+0xf6/0x158 >>> print_address_description.constprop.9+0x41/0x60 >>> kasan_report.cold.14+0x7b/0xd4 >>> __asan_report_load_n_noabort+0xf/0x20 >>> ibft_init+0x134/0xc49 >>> do_one_initcall+0xc4/0x3e0 >>> kernel_init_freeable+0x5af/0x66b >>> kernel_init+0x16/0x1d0 >>> ret_from_fork+0x22/0x30 >>> >>> ACPI tables mapped via kmap() do not have their mapped pages >>> reserved and the pages can be "stolen" by the buddy allocator. >>> >>> Apparently, on the affected system, the ACPI table in question is >>> not located in "reserved" memory, like ACPI NVS or ACPI Data, that >>> will not be used by the buddy allocator, so the memory occupied by >>> that table has to be explicitly reserved to prevent the buddy >>> allocator from using it. >>> >>> In order to address this problem, rearrange the initialization of the >>> ACPI tables on x86 to locate the initial tables earlier and reserve >>> the memory occupied by them. >>> >>> The other architectures using ACPI should not be affected by this >>> change. >>> >>> Link: https://lore.kernel.org/linux-acpi/1614802160-29362-1-git-send-email-george.kennedy@oracle.com/ >>> Reported-by: George Kennedy <george.kennedy@oracle.com> >>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> FWIW: >> Reviewed-by: Mike Rapoport <rppt@linux.ibm.com> > Thank you! > > George, can you please try this patch on the affected system? Will do. George > >>> --- >>> arch/x86/kernel/acpi/boot.c | 25 ++++++++++++------------- >>> arch/x86/kernel/setup.c | 8 +++----- >>> drivers/acpi/tables.c | 42 +++++++++++++++++++++++++++++++++++++++--- >>> include/linux/acpi.h | 9 ++++++++- >>> 4 files changed, 62 insertions(+), 22 deletions(-) >>> >>> Index: linux-pm/arch/x86/kernel/acpi/boot.c >>> =================================================================== >>> --- linux-pm.orig/arch/x86/kernel/acpi/boot.c >>> +++ linux-pm/arch/x86/kernel/acpi/boot.c >>> @@ -1554,10 +1554,18 @@ void __init acpi_boot_table_init(void) >>> /* >>> * Initialize the ACPI boot-time table parser. >>> */ >>> - if (acpi_table_init()) { >>> + if (acpi_locate_initial_tables()) >>> disable_acpi(); >>> - return; >>> - } >>> + else >>> + acpi_reserve_initial_tables(); >>> +} >>> + >>> +int __init early_acpi_boot_init(void) >>> +{ >>> + if (acpi_disabled) >>> + return 1; >>> + >>> + acpi_table_init_complete(); >>> >>> acpi_table_parse(ACPI_SIG_BOOT, acpi_parse_sbf); >>> >>> @@ -1570,18 +1578,9 @@ void __init acpi_boot_table_init(void) >>> } else { >>> printk(KERN_WARNING PREFIX "Disabling ACPI support\n"); >>> disable_acpi(); >>> - return; >>> + return 1; >>> } >>> } >>> -} >>> - >>> -int __init early_acpi_boot_init(void) >>> -{ >>> - /* >>> - * If acpi_disabled, bail out >>> - */ >>> - if (acpi_disabled) >>> - return 1; >>> >>> /* >>> * Process the Multiple APIC Description Table (MADT), if present >>> Index: linux-pm/arch/x86/kernel/setup.c >>> =================================================================== >>> --- linux-pm.orig/arch/x86/kernel/setup.c >>> +++ linux-pm/arch/x86/kernel/setup.c >>> @@ -1045,6 +1045,9 @@ void __init setup_arch(char **cmdline_p) >>> >>> cleanup_highmap(); >>> >>> + /* Look for ACPI tables and reserve memory occupied by them. */ >>> + acpi_boot_table_init(); >>> + >>> memblock_set_current_limit(ISA_END_ADDRESS); >>> e820__memblock_setup(); >>> >>> @@ -1136,11 +1139,6 @@ void __init setup_arch(char **cmdline_p) >>> >>> early_platform_quirks(); >>> >>> - /* >>> - * Parse the ACPI tables for possible boot-time SMP configuration. >>> - */ >>> - acpi_boot_table_init(); >>> - >>> early_acpi_boot_init(); >>> >>> initmem_init(); >>> Index: linux-pm/include/linux/acpi.h >>> =================================================================== >>> --- linux-pm.orig/include/linux/acpi.h >>> +++ linux-pm/include/linux/acpi.h >>> @@ -222,10 +222,14 @@ void __iomem *__acpi_map_table(unsigned >>> void __acpi_unmap_table(void __iomem *map, unsigned long size); >>> int early_acpi_boot_init(void); >>> int acpi_boot_init (void); >>> +void acpi_boot_table_prepare (void); >>> void acpi_boot_table_init (void); >>> int acpi_mps_check (void); >>> int acpi_numa_init (void); >>> >>> +int acpi_locate_initial_tables (void); >>> +void acpi_reserve_initial_tables (void); >>> +void acpi_table_init_complete (void); >>> int acpi_table_init (void); >>> int acpi_table_parse(char *id, acpi_tbl_table_handler handler); >>> int __init acpi_table_parse_entries(char *id, unsigned long table_size, >>> @@ -814,9 +818,12 @@ static inline int acpi_boot_init(void) >>> return 0; >>> } >>> >>> +static inline void acpi_boot_table_prepare(void) >>> +{ >>> +} >>> + >>> static inline void acpi_boot_table_init(void) >>> { >>> - return; >>> } >>> >>> static inline int acpi_mps_check(void) >>> Index: linux-pm/drivers/acpi/tables.c >>> =================================================================== >>> --- linux-pm.orig/drivers/acpi/tables.c >>> +++ linux-pm/drivers/acpi/tables.c >>> @@ -780,7 +780,7 @@ acpi_status acpi_os_table_override(struc >>> } >>> >>> /* >>> - * acpi_table_init() >>> + * acpi_locate_initial_tables() >>> * >>> * find RSDP, find and checksum SDT/XSDT. >>> * checksum all tables, print SDT/XSDT >>> @@ -788,7 +788,7 @@ acpi_status acpi_os_table_override(struc >>> * result: sdt_entry[] is initialized >>> */ >>> >>> -int __init acpi_table_init(void) >>> +int __init acpi_locate_initial_tables(void) >>> { >>> acpi_status status; >>> >>> @@ -803,9 +803,45 @@ int __init acpi_table_init(void) >>> status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0); >>> if (ACPI_FAILURE(status)) >>> return -EINVAL; >>> - acpi_table_initrd_scan(); >>> >>> + return 0; >>> +} >>> + >>> +void __init acpi_reserve_initial_tables(void) >>> +{ >>> + int i; >>> + >>> + for (i = 0; i < ACPI_MAX_TABLES; i++) { >>> + struct acpi_table_desc *table_desc = &initial_tables[i]; >>> + u64 start = table_desc->address; >>> + u64 size = table_desc->length; >>> + >>> + if (!start || !size) >>> + break; >>> + >>> + pr_info("Reserving %4s table memory at [mem 0x%llx-0x%llx]\n", >>> + table_desc->signature.ascii, start, start + size - 1); >>> + >>> + memblock_reserve(start, size); >>> + } >>> +} >>> + >>> +void __init acpi_table_init_complete(void) >>> +{ >>> + acpi_table_initrd_scan(); >>> check_multiple_madt(); >>> +} >>> + >>> +int __init acpi_table_init(void) >>> +{ >>> + int ret; >>> + >>> + ret = acpi_locate_initial_tables(); >>> + if (ret) >>> + return ret; >>> + >>> + acpi_table_init_complete(); >>> + >>> return 0; >>> } >>> >>> >>> >>> >> -- >> Sincerely yours, >> Mike.
On 3/24/2021 9:27 AM, Rafael J. Wysocki wrote: > On Wed, Mar 24, 2021 at 9:24 AM Mike Rapoport <rppt@linux.ibm.com> wrote: >> On Tue, Mar 23, 2021 at 08:26:52PM +0100, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> >>> The following problem has been reported by George Kennedy: >>> >>> Since commit 7fef431be9c9 ("mm/page_alloc: place pages to tail >>> in __free_pages_core()") the following use after free occurs >>> intermittently when ACPI tables are accessed. >>> >>> BUG: KASAN: use-after-free in ibft_init+0x134/0xc49 >>> Read of size 4 at addr ffff8880be453004 by task swapper/0/1 >>> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.12.0-rc1-7a7fd0d #1 >>> Call Trace: >>> dump_stack+0xf6/0x158 >>> print_address_description.constprop.9+0x41/0x60 >>> kasan_report.cold.14+0x7b/0xd4 >>> __asan_report_load_n_noabort+0xf/0x20 >>> ibft_init+0x134/0xc49 >>> do_one_initcall+0xc4/0x3e0 >>> kernel_init_freeable+0x5af/0x66b >>> kernel_init+0x16/0x1d0 >>> ret_from_fork+0x22/0x30 >>> >>> ACPI tables mapped via kmap() do not have their mapped pages >>> reserved and the pages can be "stolen" by the buddy allocator. >>> >>> Apparently, on the affected system, the ACPI table in question is >>> not located in "reserved" memory, like ACPI NVS or ACPI Data, that >>> will not be used by the buddy allocator, so the memory occupied by >>> that table has to be explicitly reserved to prevent the buddy >>> allocator from using it. >>> >>> In order to address this problem, rearrange the initialization of the >>> ACPI tables on x86 to locate the initial tables earlier and reserve >>> the memory occupied by them. >>> >>> The other architectures using ACPI should not be affected by this >>> change. >>> >>> Link: https://lore.kernel.org/linux-acpi/1614802160-29362-1-git-send-email-george.kennedy@oracle.com/ >>> Reported-by: George Kennedy <george.kennedy@oracle.com> >>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> FWIW: >> Reviewed-by: Mike Rapoport <rppt@linux.ibm.com> > Thank you! > > George, can you please try this patch on the affected system? Rafael, 10 for 10 successful reboots with your patch. First, verified the failure is still there with latest 5.12.0-rc4. George P.S. Thanks Mike, Rafael & David > >>> --- >>> arch/x86/kernel/acpi/boot.c | 25 ++++++++++++------------- >>> arch/x86/kernel/setup.c | 8 +++----- >>> drivers/acpi/tables.c | 42 +++++++++++++++++++++++++++++++++++++++--- >>> include/linux/acpi.h | 9 ++++++++- >>> 4 files changed, 62 insertions(+), 22 deletions(-) >>> >>> Index: linux-pm/arch/x86/kernel/acpi/boot.c >>> =================================================================== >>> --- linux-pm.orig/arch/x86/kernel/acpi/boot.c >>> +++ linux-pm/arch/x86/kernel/acpi/boot.c >>> @@ -1554,10 +1554,18 @@ void __init acpi_boot_table_init(void) >>> /* >>> * Initialize the ACPI boot-time table parser. >>> */ >>> - if (acpi_table_init()) { >>> + if (acpi_locate_initial_tables()) >>> disable_acpi(); >>> - return; >>> - } >>> + else >>> + acpi_reserve_initial_tables(); >>> +} >>> + >>> +int __init early_acpi_boot_init(void) >>> +{ >>> + if (acpi_disabled) >>> + return 1; >>> + >>> + acpi_table_init_complete(); >>> >>> acpi_table_parse(ACPI_SIG_BOOT, acpi_parse_sbf); >>> >>> @@ -1570,18 +1578,9 @@ void __init acpi_boot_table_init(void) >>> } else { >>> printk(KERN_WARNING PREFIX "Disabling ACPI support\n"); >>> disable_acpi(); >>> - return; >>> + return 1; >>> } >>> } >>> -} >>> - >>> -int __init early_acpi_boot_init(void) >>> -{ >>> - /* >>> - * If acpi_disabled, bail out >>> - */ >>> - if (acpi_disabled) >>> - return 1; >>> >>> /* >>> * Process the Multiple APIC Description Table (MADT), if present >>> Index: linux-pm/arch/x86/kernel/setup.c >>> =================================================================== >>> --- linux-pm.orig/arch/x86/kernel/setup.c >>> +++ linux-pm/arch/x86/kernel/setup.c >>> @@ -1045,6 +1045,9 @@ void __init setup_arch(char **cmdline_p) >>> >>> cleanup_highmap(); >>> >>> + /* Look for ACPI tables and reserve memory occupied by them. */ >>> + acpi_boot_table_init(); >>> + >>> memblock_set_current_limit(ISA_END_ADDRESS); >>> e820__memblock_setup(); >>> >>> @@ -1136,11 +1139,6 @@ void __init setup_arch(char **cmdline_p) >>> >>> early_platform_quirks(); >>> >>> - /* >>> - * Parse the ACPI tables for possible boot-time SMP configuration. >>> - */ >>> - acpi_boot_table_init(); >>> - >>> early_acpi_boot_init(); >>> >>> initmem_init(); >>> Index: linux-pm/include/linux/acpi.h >>> =================================================================== >>> --- linux-pm.orig/include/linux/acpi.h >>> +++ linux-pm/include/linux/acpi.h >>> @@ -222,10 +222,14 @@ void __iomem *__acpi_map_table(unsigned >>> void __acpi_unmap_table(void __iomem *map, unsigned long size); >>> int early_acpi_boot_init(void); >>> int acpi_boot_init (void); >>> +void acpi_boot_table_prepare (void); >>> void acpi_boot_table_init (void); >>> int acpi_mps_check (void); >>> int acpi_numa_init (void); >>> >>> +int acpi_locate_initial_tables (void); >>> +void acpi_reserve_initial_tables (void); >>> +void acpi_table_init_complete (void); >>> int acpi_table_init (void); >>> int acpi_table_parse(char *id, acpi_tbl_table_handler handler); >>> int __init acpi_table_parse_entries(char *id, unsigned long table_size, >>> @@ -814,9 +818,12 @@ static inline int acpi_boot_init(void) >>> return 0; >>> } >>> >>> +static inline void acpi_boot_table_prepare(void) >>> +{ >>> +} >>> + >>> static inline void acpi_boot_table_init(void) >>> { >>> - return; >>> } >>> >>> static inline int acpi_mps_check(void) >>> Index: linux-pm/drivers/acpi/tables.c >>> =================================================================== >>> --- linux-pm.orig/drivers/acpi/tables.c >>> +++ linux-pm/drivers/acpi/tables.c >>> @@ -780,7 +780,7 @@ acpi_status acpi_os_table_override(struc >>> } >>> >>> /* >>> - * acpi_table_init() >>> + * acpi_locate_initial_tables() >>> * >>> * find RSDP, find and checksum SDT/XSDT. >>> * checksum all tables, print SDT/XSDT >>> @@ -788,7 +788,7 @@ acpi_status acpi_os_table_override(struc >>> * result: sdt_entry[] is initialized >>> */ >>> >>> -int __init acpi_table_init(void) >>> +int __init acpi_locate_initial_tables(void) >>> { >>> acpi_status status; >>> >>> @@ -803,9 +803,45 @@ int __init acpi_table_init(void) >>> status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0); >>> if (ACPI_FAILURE(status)) >>> return -EINVAL; >>> - acpi_table_initrd_scan(); >>> >>> + return 0; >>> +} >>> + >>> +void __init acpi_reserve_initial_tables(void) >>> +{ >>> + int i; >>> + >>> + for (i = 0; i < ACPI_MAX_TABLES; i++) { >>> + struct acpi_table_desc *table_desc = &initial_tables[i]; >>> + u64 start = table_desc->address; >>> + u64 size = table_desc->length; >>> + >>> + if (!start || !size) >>> + break; >>> + >>> + pr_info("Reserving %4s table memory at [mem 0x%llx-0x%llx]\n", >>> + table_desc->signature.ascii, start, start + size - 1); >>> + >>> + memblock_reserve(start, size); >>> + } >>> +} >>> + >>> +void __init acpi_table_init_complete(void) >>> +{ >>> + acpi_table_initrd_scan(); >>> check_multiple_madt(); >>> +} >>> + >>> +int __init acpi_table_init(void) >>> +{ >>> + int ret; >>> + >>> + ret = acpi_locate_initial_tables(); >>> + if (ret) >>> + return ret; >>> + >>> + acpi_table_init_complete(); >>> + >>> return 0; >>> } >>> >>> >>> >>> >> -- >> Sincerely yours, >> Mike.
On Wed, Mar 24, 2021 at 4:42 PM George Kennedy <george.kennedy@oracle.com> wrote: > > > > On 3/24/2021 9:27 AM, Rafael J. Wysocki wrote: > > On Wed, Mar 24, 2021 at 9:24 AM Mike Rapoport <rppt@linux.ibm.com> wrote: > >> On Tue, Mar 23, 2021 at 08:26:52PM +0100, Rafael J. Wysocki wrote: > >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > >>> > >>> The following problem has been reported by George Kennedy: > >>> > >>> Since commit 7fef431be9c9 ("mm/page_alloc: place pages to tail > >>> in __free_pages_core()") the following use after free occurs > >>> intermittently when ACPI tables are accessed. > >>> > >>> BUG: KASAN: use-after-free in ibft_init+0x134/0xc49 > >>> Read of size 4 at addr ffff8880be453004 by task swapper/0/1 > >>> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.12.0-rc1-7a7fd0d #1 > >>> Call Trace: > >>> dump_stack+0xf6/0x158 > >>> print_address_description.constprop.9+0x41/0x60 > >>> kasan_report.cold.14+0x7b/0xd4 > >>> __asan_report_load_n_noabort+0xf/0x20 > >>> ibft_init+0x134/0xc49 > >>> do_one_initcall+0xc4/0x3e0 > >>> kernel_init_freeable+0x5af/0x66b > >>> kernel_init+0x16/0x1d0 > >>> ret_from_fork+0x22/0x30 > >>> > >>> ACPI tables mapped via kmap() do not have their mapped pages > >>> reserved and the pages can be "stolen" by the buddy allocator. > >>> > >>> Apparently, on the affected system, the ACPI table in question is > >>> not located in "reserved" memory, like ACPI NVS or ACPI Data, that > >>> will not be used by the buddy allocator, so the memory occupied by > >>> that table has to be explicitly reserved to prevent the buddy > >>> allocator from using it. > >>> > >>> In order to address this problem, rearrange the initialization of the > >>> ACPI tables on x86 to locate the initial tables earlier and reserve > >>> the memory occupied by them. > >>> > >>> The other architectures using ACPI should not be affected by this > >>> change. > >>> > >>> Link: https://lore.kernel.org/linux-acpi/1614802160-29362-1-git-send-email-george.kennedy@oracle.com/ > >>> Reported-by: George Kennedy <george.kennedy@oracle.com> > >>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > >> FWIW: > >> Reviewed-by: Mike Rapoport <rppt@linux.ibm.com> > > Thank you! > > > > George, can you please try this patch on the affected system? > > Rafael, > > 10 for 10 successful reboots with your patch. > > First, verified the failure is still there with latest 5.12.0-rc4. Thank you! I'll add a Tested-by from you to it, then.
Index: linux-pm/arch/x86/kernel/acpi/boot.c =================================================================== --- linux-pm.orig/arch/x86/kernel/acpi/boot.c +++ linux-pm/arch/x86/kernel/acpi/boot.c @@ -1554,10 +1554,18 @@ void __init acpi_boot_table_init(void) /* * Initialize the ACPI boot-time table parser. */ - if (acpi_table_init()) { + if (acpi_locate_initial_tables()) disable_acpi(); - return; - } + else + acpi_reserve_initial_tables(); +} + +int __init early_acpi_boot_init(void) +{ + if (acpi_disabled) + return 1; + + acpi_table_init_complete(); acpi_table_parse(ACPI_SIG_BOOT, acpi_parse_sbf); @@ -1570,18 +1578,9 @@ void __init acpi_boot_table_init(void) } else { printk(KERN_WARNING PREFIX "Disabling ACPI support\n"); disable_acpi(); - return; + return 1; } } -} - -int __init early_acpi_boot_init(void) -{ - /* - * If acpi_disabled, bail out - */ - if (acpi_disabled) - return 1; /* * Process the Multiple APIC Description Table (MADT), if present Index: linux-pm/arch/x86/kernel/setup.c =================================================================== --- linux-pm.orig/arch/x86/kernel/setup.c +++ linux-pm/arch/x86/kernel/setup.c @@ -1045,6 +1045,9 @@ void __init setup_arch(char **cmdline_p) cleanup_highmap(); + /* Look for ACPI tables and reserve memory occupied by them. */ + acpi_boot_table_init(); + memblock_set_current_limit(ISA_END_ADDRESS); e820__memblock_setup(); @@ -1136,11 +1139,6 @@ void __init setup_arch(char **cmdline_p) early_platform_quirks(); - /* - * Parse the ACPI tables for possible boot-time SMP configuration. - */ - acpi_boot_table_init(); - early_acpi_boot_init(); initmem_init(); Index: linux-pm/include/linux/acpi.h =================================================================== --- linux-pm.orig/include/linux/acpi.h +++ linux-pm/include/linux/acpi.h @@ -222,10 +222,14 @@ void __iomem *__acpi_map_table(unsigned void __acpi_unmap_table(void __iomem *map, unsigned long size); int early_acpi_boot_init(void); int acpi_boot_init (void); +void acpi_boot_table_prepare (void); void acpi_boot_table_init (void); int acpi_mps_check (void); int acpi_numa_init (void); +int acpi_locate_initial_tables (void); +void acpi_reserve_initial_tables (void); +void acpi_table_init_complete (void); int acpi_table_init (void); int acpi_table_parse(char *id, acpi_tbl_table_handler handler); int __init acpi_table_parse_entries(char *id, unsigned long table_size, @@ -814,9 +818,12 @@ static inline int acpi_boot_init(void) return 0; } +static inline void acpi_boot_table_prepare(void) +{ +} + static inline void acpi_boot_table_init(void) { - return; } static inline int acpi_mps_check(void) Index: linux-pm/drivers/acpi/tables.c =================================================================== --- linux-pm.orig/drivers/acpi/tables.c +++ linux-pm/drivers/acpi/tables.c @@ -780,7 +780,7 @@ acpi_status acpi_os_table_override(struc } /* - * acpi_table_init() + * acpi_locate_initial_tables() * * find RSDP, find and checksum SDT/XSDT. * checksum all tables, print SDT/XSDT @@ -788,7 +788,7 @@ acpi_status acpi_os_table_override(struc * result: sdt_entry[] is initialized */ -int __init acpi_table_init(void) +int __init acpi_locate_initial_tables(void) { acpi_status status; @@ -803,9 +803,45 @@ int __init acpi_table_init(void) status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0); if (ACPI_FAILURE(status)) return -EINVAL; - acpi_table_initrd_scan(); + return 0; +} + +void __init acpi_reserve_initial_tables(void) +{ + int i; + + for (i = 0; i < ACPI_MAX_TABLES; i++) { + struct acpi_table_desc *table_desc = &initial_tables[i]; + u64 start = table_desc->address; + u64 size = table_desc->length; + + if (!start || !size) + break; + + pr_info("Reserving %4s table memory at [mem 0x%llx-0x%llx]\n", + table_desc->signature.ascii, start, start + size - 1); + + memblock_reserve(start, size); + } +} + +void __init acpi_table_init_complete(void) +{ + acpi_table_initrd_scan(); check_multiple_madt(); +} + +int __init acpi_table_init(void) +{ + int ret; + + ret = acpi_locate_initial_tables(); + if (ret) + return ret; + + acpi_table_init_complete(); + return 0; }