| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <cd37d06b-2310-2cc8-c19f-c9eb50798a67@linux.dev>
Date: Thu, 5 Dec 2024 23:34:46 +0800
From: Hao Ge <hao.ge@...ux.dev>
To: Suren Baghdasaryan <surenb@...gle.com>
Cc: kent.overstreet@...ux.dev, akpm@...ux-foundation.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, Hao Ge <gehao@...inos.cn>
Subject: Re: [PATCH v2] mm/alloc_tag: fix vm_module_tags_populate's KASAN
poisoning logic
Hi Suren
On 12/5/24 22:48, Suren Baghdasaryan wrote:
> On Wed, Dec 4, 2024 at 7:20 PM Hao Ge <hao.ge@...ux.dev> wrote:
>> Hi Suren
>>
>>
>> On 12/5/24 10:14, Hao Ge wrote:
>>> Hi Suren
>>>
>>>
>>> On 12/5/24 03:33, Suren Baghdasaryan wrote:
>>>> On Wed, Dec 4, 2024 at 7:08 AM Hao Ge <hao.ge@...ux.dev> wrote:
>>>>> Hi Suren
>>>>>
>>>>>
>>>>> Thank you for your review.
>>>>>
>>>>>
>>>>> On 12/4/24 22:39, Suren Baghdasaryan wrote:
>>>>>> On Wed, Dec 4, 2024 at 12:35 AM Hao Ge <hao.ge@...ux.dev> wrote:
>>>>>>> From: Hao Ge <gehao@...inos.cn>
>>>>>>>
>>>>>>> After merge commit 233e89322cbe ("alloc_tag:
>>>>>>> fix module allocation tags populated area calculation"),
>>>>>>> We still encountered a KASAN bug.
>>>>>>>
>>>>>>> This is because we have only actually performed
>>>>>>> page allocation and address mapping here.
>>>>>>> we need to unpoisoned portions of underlying memory.
>>>>>>>
>>>>>>> Because we have a change in the size here,we need to
>>>>>>> re-annotate poisoned and unpoisoned portions of underlying memory
>>>>>>> according to the new size.
>>>>>>>
>>>>>>> Here is the log for KASAN:
>>>>>>>
>>>>>>> [ 5.041171][ T1]
>>>>>>> ==================================================================
>>>>>>> [ 5.042047][ T1] BUG: KASAN: vmalloc-out-of-bounds in
>>>>>>> move_module+0x2c0/0x708
>>>>>>> [ 5.042723][ T1] Write of size 240 at addr ffff80007e510000
>>>>>>> by task systemd/1
>>>>>>> [ 5.043412][ T1]
>>>>>>> [ 5.043523][ T72] input: QEMU QEMU USB Tablet as
>>>>>>> /devices/pci0000:00/0000:00:01.1/0000:02:001
>>>>>>> [ 5.043614][ T1] CPU: 0 UID: 0 PID: 1 Comm: systemd Not
>>>>>>> tainted 6.13.0-rc1+ #28
>>>>>>> [ 5.045560][ T1] Hardware name: QEMU KVM Virtual Machine,
>>>>>>> BIOS 0.0.0 02/06/2015
>>>>>>> [ 5.046328][ T1] Call trace:
>>>>>>> [ 5.046670][ T1] show_stack+0x20/0x38 (C)
>>>>>>> [ 5.047127][ T1] dump_stack_lvl+0x80/0xf8
>>>>>>> [ 5.047533][ T1]
>>>>>>> print_address_description.constprop.0+0x58/0x358
>>>>>>> [ 5.048092][ T72] hid-generic 0003:0627:0001.0001:
>>>>>>> input,hidraw0: USB HID v0.01 Mouse [QEMU 0
>>>>>>> [ 5.048126][ T1] print_report+0xb0/0x280
>>>>>>> [ 5.049682][ T1] kasan_report+0xb8/0x108
>>>>>>> [ 5.050170][ T1] kasan_check_range+0xe8/0x190
>>>>>>> [ 5.050685][ T1] memcpy+0x58/0xa0
>>>>>>> [ 5.051135][ T1] move_module+0x2c0/0x708
>>>>>>> [ 5.051586][ T1] layout_and_allocate.constprop.0+0x308/0x5b8
>>>>>>> [ 5.052219][ T1] load_module+0x134/0x16c8
>>>>>>> [ 5.052671][ T1] init_module_from_file+0xdc/0x138
>>>>>>> [ 5.053193][ T1] idempotent_init_module+0x344/0x600
>>>>>>> [ 5.053742][ T1] __arm64_sys_finit_module+0xbc/0x150
>>>>>>> [ 5.054289][ T1] invoke_syscall+0xd4/0x258
>>>>>>> [ 5.054749][ T1] el0_svc_common.constprop.0+0xb4/0x240
>>>>>>> [ 5.055319][ T1] do_el0_svc+0x48/0x68
>>>>>>> [ 5.055743][ T1] el0_svc+0x40/0xe0
>>>>>>> [ 5.056142][ T1] el0t_64_sync_handler+0x10c/0x138
>>>>>>> [ 5.056658][ T1] el0t_64_sync+0x1ac/0x1b0
>>>>>>>
>>>>>>> Fixes: 233e89322cbe ("alloc_tag: fix module allocation tags
>>>>>>> populated area calculation")
>>>>>>> Signed-off-by: Hao Ge <gehao@...inos.cn>
>>>>>> Thanks for the fix!
>>>>>>
>>>>>>> ---
>>>>>>> v2: Add comments to kasan_unpoison_vmalloc like other places.
>>>>>>>
>>>>>>> commit 233e89322cbe ("alloc_tag: fix module allocation
>>>>>>> tags populated area calculation") is currently in the
>>>>>>> mm-hotfixes-unstable branch, so this patch is
>>>>>>> developed based on the mm-hotfixes-unstable branch.
>>>>>>> ---
>>>>>>> lib/alloc_tag.c | 13 +++++++++++++
>>>>>>> 1 file changed, 13 insertions(+)
>>>>>>>
>>>>>>> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
>>>>>>> index 4ee6caa6d2da..f885b3f3af0e 100644
>>>>>>> --- a/lib/alloc_tag.c
>>>>>>> +++ b/lib/alloc_tag.c
>>>>>>> @@ -421,7 +421,20 @@ static int vm_module_tags_populate(void)
>>>>>>> __free_page(next_page[i]);
>>>>>>> return -ENOMEM;
>>>>>>> }
>>>>>>> +
>>>>>>> + kasan_poison_vmalloc((void *)module_tags.start_addr,
>>>>>>> + vm_module_tags->nr_pages << PAGE_SHIFT);
>>>>>>> +
>>>>>>> vm_module_tags->nr_pages += nr;
>>>>>>> +
>>>>>>> + /*
>>>>>>> + * Mark the pages as accessible, now that they are
>>>>>>> mapped.
>>>>>>> + * With hardware tag-based KASAN, marking is
>>>>>>> skipped for
>>>>>>> + * non-VM_ALLOC mappings, see
>>>>>>> __kasan_unpoison_vmalloc().
>>>>>>> + */
>>>>>>> + kasan_unpoison_vmalloc((void
>>>>>>> *)module_tags.start_addr,
>>>>>>> + vm_module_tags->nr_pages << PAGE_SHIFT,
>>>>>>> + KASAN_VMALLOC_PROT_NORMAL);
>>>>>> Instead of poisoning [module_tags.start_addr,
>>>>>> vm_module_tags->nr_pages], incrementing vm_module_tags->nr_pages and
>>>>>> the unpoisoning [module_tags.start_addr, vm_module_tags->nr_pages]
>>>>>> could we simply poisons the additional area like this:
>>>>>>
>>>>>> kasan_unpoison_vmalloc((void
>>>>>> *)module_tags.start_addr +
>>>>>> (vm_module_tags->nr_pages << PAGE_SHIFT),
>>>>>> nr << PAGE_SHIFT,
>>>>>> KASAN_VMALLOC_PROT_NORMAL);
>>>>>> vm_module_tags->nr_pages += nr;
>>>>>> ?
>>>>> I had considered making such modifications earlier.
>>>>>
>>>>> But considering the following situation,
>>>>>
>>>>> A module tags spans across the regions of [module_tags.start_addr,
>>>>> vm_module_tags->nr_pages] and [module_tags.start_addr +
>>>>> vm_module_tags->nr_pages, ...].
>>>>>
>>>>> It may result in false positives for out-of-bounds errors.
>>>> Sorry, maybe I'm missing something but I don't see why poisoning only
>>>> newly mapped area would lead to false positives. Could you please
>>>> clarify?
>>>
>>> Because KASAN may perceive the two as distinct address spaces, despite
>>> their addresses being contiguous.
>>>
>>> So, when a module tag spans across these two contiguous address
>>> spaces, KASAN may incorrectly consider it as an out-of-bounds access.
>>>
>>>
>>>> Also, if you do need to unpoison and then poison, using phys_end and
>>>> new_end would be better, like this:
>>>>
>>>> kasan_poison_vmalloc((void *)module_tags.start_addr,
>>>> phys_end -
>>>> module_tags.start_addr)
>>>>
>>>> kasan_unpoison_vmalloc((void *)module_tags.start_addr,
>>>> new_end -
>>>> module_tags.start_addr,
>>>> KASAN_VMALLOC_PROT_NORMAL);
>>> OK, the next version will include.
>> After verification and consideration, I have found that this
>> modification may still pose problems.
>>
>> Because we haven't ensured that new_end is page-aligned,
>>
>> So, we've only made the region from||module_tags.start_addr
>> tonew_endaccessible.
> Correct and the area [module_tags.start_addr, new_end] is the one that
> should be considered valid/accessible. We fault-in a physical page
> that includes new_end and might cover some area after that address but
> accessing the addresses above new_end is technically out-of-bounds
> (there are no valid codetags there).
>
>> Using this example, in reality,end equals 0xffff80007e5100f0:
>>
>> Write of size 240 at addr ffff80007e510000 by task systemd/1
>>
>> When we access other memory within the same page as0xffff80007e5100f0,
>> KASAN warnings will also be issued due to the lack of unpoisoned
>> portions in that memory.
> Will you get a KASAN warning if you access memory below new_end?
> Warnings above that address I think should be considered as expected
> (even though we have a valid physical page there).
> Does that make sense?
Is that really the case?
Here is the log that has been updated to include the calculation new_end
- module_tags.start_addr.
On my machine,module_tags.start_addr is equal to ffff80007e510000
and the size of the first module_tags is 240
So, because you only made the range|[module_tags.start_addr,
new_end]|accessible, the same issue will arise again later on.
[ 5.798918][ T258] BUG: KASAN: vmalloc-out-of-bounds in
move_module+0x2c0/0x708
[ 5.799622][ T258] Write of size 200 at addr ffff80007e5100f0 by
task systemd-modules/258
[ 5.800149][ T256] systemd-journald[256]: Collecting audit messages
is disabled.
[ 5.800296][ T258]
[ 5.800301][ T258] CPU: 2 UID: 0 PID: 258 Comm: systemd-modules Not
tainted 6.13.0-rc1+ #46
[ 5.801727][ T10] input: QEMU QEMU USB Mouse as
/devices/pci0000:00/0000:00:01.1/0000:02:00.3
[ 5.801905][ T258] Hardware name: QEMU KVM Virtual Machine, BIOS
0.0.0 02/06/2015
[ 5.801911][ T258] Call trace:
[ 5.804120][ T258] show_stack+0x20/0x38 (C)
[ 5.804512][ T258] dump_stack_lvl+0x80/0xf8
[ 5.804916][ T258] print_address_description.constprop.0+0x58/0x358
[ 5.805276][ T10] hid-generic 0003:0627:0001.0003: input,hidraw2:
USB HID v0.01 Mouse [QEMU 0
[ 5.805501][ T258] print_report+0xb0/0x280
[ 5.807031][ T258] kasan_report+0xb8/0x108
[ 5.807415][ T258] kasan_check_range+0xe8/0x190
[ 5.807714][ T124] pcieport 0000:00:02.3: pciehp: Slot(0-11): No
device found
[ 5.807921][ T258] memcpy+0x58/0xa0
[ 5.807927][ T258] move_module+0x2c0/0x708
[ 5.809346][ T258] layout_and_allocate.constprop.0+0x308/0x5b8
[ 5.809942][ T258] load_module+0x134/0x16c8
[ 5.810375][ T258] init_module_from_file+0xdc/0x138
[ 5.810870][ T258] idempotent_init_module+0x344/0x600
[ 5.811389][ T258] __arm64_sys_finit_module+0xbc/0x150
[ 5.811916][ T258] invoke_syscall+0xd4/0x258
[ 5.812362][ T258] el0_svc_common.constprop.0+0xb4/0x240
[ 5.812914][ T258] do_el0_svc+0x48/0x68
[ 5.813318][ T258] el0_svc+0x40/0xe0
[ 5.813698][ T258] el0t_64_sync_handler+0x10c/0x138
[ 5.814210][ T258] el0t_64_sync+0x1ac/0x1b0[ 5.798918][ T258]
BUG: KASAN: vmalloc-out-of-bounds in move_module+0x2c0/0x708
[ 5.799622][ T258] Write of size 200 at addr ffff80007e5100f0 by
task systemd-modules/258
[ 5.800149][ T256] systemd-journald[256]: Collecting audit messages
is disabled.
[ 5.800296][ T258]
[ 5.800301][ T258] CPU: 2 UID: 0 PID: 258 Comm: systemd-modules Not
tainted 6.13.0-rc1+ #46
[ 5.801727][ T10] input: QEMU QEMU USB Mouse as
/devices/pci0000:00/0000:00:01.1/0000:02:00.3
[ 5.801905][ T258] Hardware name: QEMU KVM Virtual Machine, BIOS
0.0.0 02/06/2015
[ 5.801911][ T258] Call trace:
[ 5.804120][ T258] show_stack+0x20/0x38 (C)
[ 5.804512][ T258] dump_stack_lvl+0x80/0xf8
[ 5.804916][ T258] print_address_description.constprop.0+0x58/0x358
[ 5.805276][ T10] hid-generic 0003:0627:0001.0003: input,hidraw2:
USB HID v0.01 Mouse [QEMU 0
[ 5.805501][ T258] print_report+0xb0/0x280
[ 5.807031][ T258] kasan_report+0xb8/0x108
[ 5.807415][ T258] kasan_check_range+0xe8/0x190
[ 5.807714][ T124] pcieport 0000:00:02.3: pciehp: Slot(0-11): No
device found
[ 5.807921][ T258] memcpy+0x58/0xa0
[ 5.807927][ T258] move_module+0x2c0/0x708
[ 5.809346][ T258] layout_and_allocate.constprop.0+0x308/0x5b8
[ 5.809942][ T258] load_module+0x134/0x16c8
[ 5.810375][ T258] init_module_from_file+0xdc/0x138
[ 5.810870][ T258] idempotent_init_module+0x344/0x600
[ 5.811389][ T258] __arm64_sys_finit_module+0xbc/0x150
[ 5.811916][ T258] invoke_syscall+0xd4/0x258
[ 5.812362][ T258] el0_svc_common.constprop.0+0xb4/0x240
[ 5.812914][ T258] do_el0_svc+0x48/0x68
[ 5.813318][ T258] el0_svc+0x40/0xe0
[ 5.813698][ T258] el0t_64_sync_handler+0x10c/0x138
[ 5.814210][ T258] el0t_64_sync+0x1ac/0x1b0
>> Based on that, I would suggest sticking with the V2 version.
>>
>>
>> Thanks
>>
>> Best Regards
>>
>> Hao
>>
>>>
>>> Thanks
>>>
>>> Best regards
>>>
>>> Hao
>>>
>>>
>>>>>>> }
>>>>>>>
>>>>>>> return 0;
>>>>>>> --
>>>>>>> 2.25.1
>>>>>>>
Powered by blists - more mailing lists