Message-ID: <7v47bmgtm5jyvbxzgr2465kkho2bcziiilz7fw736fghto32cd@nmrwxwbtdptl>
Date: Fri, 21 Mar 2025 11:40:04 +0100
From: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
To: Andrey Ryabinin <ryabinin.a.a@...il.com>
CC: <kees@...nel.org>, <julian.stecklina@...erus-technology.de>,
	<kevinloughlin@...gle.com>, <peterz@...radead.org>, <tglx@...utronix.de>,
	<justinstitt@...gle.com>, <catalin.marinas@....com>,
	<wangkefeng.wang@...wei.com>, <bhe@...hat.com>,
	<kirill.shutemov@...ux.intel.com>, <will@...nel.org>, <ardb@...nel.org>,
	<jason.andryuk@....com>, <dave.hansen@...ux.intel.com>,
	<pasha.tatashin@...een.com>, <ndesaulniers@...gle.com>,
	<guoweikang.kernel@...il.com>, <dwmw@...zon.co.uk>, <mark.rutland@....com>,
	<broonie@...nel.org>, <apopple@...dia.com>, <bp@...en8.de>,
	<rppt@...nel.org>, <kaleshsingh@...gle.com>, <richard.weiyang@...il.com>,
	<luto@...nel.org>, <glider@...gle.com>, <pankaj.gupta@....com>,
	<andreyknvl@...il.com>, <pawan.kumar.gupta@...ux.intel.com>,
	<kuan-ying.lee@...onical.com>, <tony.luck@...el.com>, <tj@...nel.org>,
	<jgross@...e.com>, <dvyukov@...gle.com>, <baohua@...nel.org>,
	<samuel.holland@...ive.com>, <dennis@...nel.org>,
	<akpm@...ux-foundation.org>, <thomas.weissschuh@...utronix.de>,
	<surenb@...gle.com>, <kbingham@...nel.org>, <ankita@...dia.com>,
	<nathan@...nel.org>, <ziy@...dia.com>, <xin@...or.com>,
	<rafael.j.wysocki@...el.com>, <andriy.shevchenko@...ux.intel.com>,
	<cl@...ux.com>, <jhubbard@...dia.com>, <hpa@...or.com>,
	<scott@...amperecomputing.com>, <david@...hat.com>, <jan.kiszka@...mens.com>,
	<vincenzo.frascino@....com>, <corbet@....net>, <maz@...nel.org>,
	<mingo@...hat.com>, <arnd@...db.de>, <ytcoode@...il.com>, <xur@...gle.com>,
	<morbo@...gle.com>, <thiago.bauermann@...aro.org>,
	<linux-doc@...r.kernel.org>, <kasan-dev@...glegroups.com>,
	<linux-kernel@...r.kernel.org>, <llvm@...ts.linux.dev>, <linux-mm@...ck.org>,
	<linux-arm-kernel@...ts.infradead.org>, <x86@...nel.org>
Subject: Re: [PATCH v2 09/14] mm: Pcpu chunk address tag reset

On 2025-03-20 at 18:39:35 +0100, Andrey Ryabinin wrote:
>On Tue, Feb 18, 2025 at 9:19 AM Maciej Wieczor-Retman
><maciej.wieczor-retman@...el.com> wrote:
>>
>> The problem presented here is related to NUMA systems and tag-based
>> KASAN mode. Getting to it can be explained in the following points:
>>
>>         1. A new chunk is created with pcpu_create_chunk() and
>>            vm_structs are allocated. On systems with one NUMA node only
>>            one is allocated, but with more NUMA nodes at least a second
>>            one will be allocated too.
>>
>>         2. chunk->base_addr is assigned the modified value of
>>            vms[0]->addr and thus inherits the tag of this allocated
>>            structure.
>>
>>         3. In pcpu_alloc() for each possible cpu pcpu_chunk_addr() is
>>            executed which calculates per cpu pointers that correspond to
>>            the vms structure addresses. The calculations are based on
>>            adding an offset from a table to chunk->base_addr.
>>
>> Here the problem presents itself since for addresses based on vms[1] and
>> up, the tag will be different than the ones based on vms[0] (base_addr).
>> The tag mismatch happens and an error is reported.
>>
>> Reset the base_addr tag, since it will disable tag checks for pointers
>> derived arithmetically from base_addr that would inherit its tag.
>>
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
>> ---
>>  mm/percpu-vm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/percpu-vm.c b/mm/percpu-vm.c
>> index cd69caf6aa8d..e13750d804f7 100644
>> --- a/mm/percpu-vm.c
>> +++ b/mm/percpu-vm.c
>> @@ -347,7 +347,7 @@ static struct pcpu_chunk *pcpu_create_chunk(gfp_t gfp)
>>         }
>>
>>         chunk->data = vms;
>> -       chunk->base_addr = vms[0]->addr - pcpu_group_offsets[0];
>> +       chunk->base_addr = kasan_reset_tag(vms[0]->addr) - pcpu_group_offsets[0];
>
>This looks like a generic tags mode bug. I mean that arm64 is also
>affected by this.
>I assume it just wasn't noticed before because arm64 with multiple
>NUMAs are much less common.

That was my assumption as well

>
>With this change tag-mode KASAN won't be able to catch bugus accesses
>to pcpu areas.
>I'm thinking it would be better to fix this on the pcpu_get_vm_areas()
>area side by replacing
>this
>    for (area = 0; area < nr_vms; area++)
>        vms[area]->addr = kasan_unpoison_vmalloc(vms[area]->addr,
>                                             vms[area]->size,
>KASAN_VMALLOC_PROT_NORMAL);
>
>with something like
>    kasan_unpoison_vmap_areas(vms, nr_vms);
>which will unpoison all areas using the same tag.
>
>Thoughts?

I was looking for a solution that would preserve the individual tags for each
area. But so far I didn't come up with anything that would work. I first assumed
that some per-cpu pointers would always be derived from from vms[0]->addr, and
some from vms[1]->addr. For example first half of the cpus would be tied to
vms[0]->addr, second half to vms[1]->addr (since areas are allocated based on
the NUMA layout as far as I know from the docs). But that didn't work and
pointers popped up that were from the second half of the cores but had tags from
the vms[0]->addr.

I think unpoisoning all the areas with the same tag would work, but could it
create issues for out-of-bounds accesses? From my testing the areas are nowhere
near each other but I don't know if that's a given or just by accident?

TLDR: can the areas be adjecent and therefor break OOB checking?

-- 
Kind regards
Maciej Wieczór-Retman

Powered by blists - more mailing lists