On Fri, Mar 5, 2021 at 12:14 AM George Kennedy
On 3/4/2021 7:14 AM, Rafael J. Wysocki wrote:
> On Thu, Mar 4, 2021 at 2:22 AM George Kennedy <george.kennedy(a)oracle.com>
>> 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:
>> ACPI tables mapped via kmap() do not have their mapped pages
>> reserved and the pages can be "stolen" by the buddy allocator.
> What do you mean by this?
The ibft table, for example, is mapped in via acpi_map() and kmap(). The
page for the ibft table is not reserved, so it can end up on the freelist.
You appear to be saying that it is not sufficient to kmap() a page in
order to use it safely. It is also necessary to reserve it upfront,
for example with the help of memblock_reserve(). Is that correct? If
so, is there an alternative way to reserve a page frame?
>> Use memblock_reserve() to reserve all the ACPI table pages.
> How is this going to help?
If the ibft table page is not reserved, it will end up on the freelist
and potentially be allocated before ibft_init() is called.
I believe this is the call that causes the ibft table page (in this case
pfn=0xbe453) to end up on the freelist:
memmap_init_range: size=bd49b, nid=0, zone=1, start_pfn=1000,
David, is commit 7fef431be9c9 related to this and if so, then how?
[ 0.477319] memmap_init_range+0x33b/0x4e2
[ 0.479053] memmap_init_zone+0x1e0/0x243
[ 0.485276] free_area_init_node+0xa4e/0xac5
[ 0.498242] free_area_init+0xf4a/0x107a
[ 0.509958] zone_sizes_init+0xd9/0x111
[ 0.511731] paging_init+0x4a/0x4c
[ 0.512417] setup_arch+0x14f8/0x1758
[ 0.519193] start_kernel+0x6c/0x46f
[ 0.519921] x86_64_start_reservations+0x37/0x39
[ 0.520847] x86_64_start_kernel+0x7b/0x7e
[ 0.521666] secondary_startup_64_no_verify+0xb0/0xbb
>> Signed-off-by: George Kennedy <george.kennedy(a)oracle.com>
>> arch/x86/kernel/setup.c | 3 +--
>> drivers/acpi/acpica/tbinstal.c | 4 ++++
>> 2 files changed, 5 insertions(+), 2 deletions(-)
>> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
>> index d883176..97deea3 100644
>> --- a/arch/x86/kernel/setup.c
>> +++ b/arch/x86/kernel/setup.c
>> @@ -1046,6 +1046,7 @@ void __init setup_arch(char **cmdline_p)
>> + acpi_boot_table_init();
> This cannot be moved before the acpi_table_upgrade() invocation AFAICS.
> Why exactly do you want to move it?
Want to make sure there are slots for memblock_reserve() to be able to
reserve the page.
Well, that may not require reordering the initialization this way.
>> @@ -1139,8 +1140,6 @@ void __init setup_arch(char **cmdline_p)
>> * Parse the ACPI tables for possible boot-time SMP configuration.
>> - acpi_boot_table_init();
>> diff --git a/drivers/acpi/acpica/tbinstal.c b/drivers/acpi/acpica/tbinstal.c
>> index 8d1e5b5..4e32b22 100644
>> --- a/drivers/acpi/acpica/tbinstal.c
>> +++ b/drivers/acpi/acpica/tbinstal.c
>> @@ -8,6 +8,7 @@
>> #include <acpi/acpi.h>
>> +#include <linux/memblock.h>
>> #include "accommon.h"
>> #include "actables.h"
>> @@ -58,6 +59,9 @@
>> + memblock_reserve(new_table_desc->address,
>> + PAGE_ALIGN(new_table_desc->pointer->length));
> Why do you want to do this here in the first place?
If there is a better place to do it, I can move the memblock_reserve()
there. The memblock_reserve() cannot be done from the ibft code - it's
too late - the ibft table page has already ended up on the freelist by
the time ibft_init() is called.
> Things like that cannot be done in the ACPICA code in general.
Can you recommend a better place to do the memblock_reserve() from?
Maybe. I need to understand the problem better, though.