From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 932A0E7716D for ; Thu, 5 Dec 2024 16:26:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0D0896B019B; Thu, 5 Dec 2024 11:26:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 07F656B019C; Thu, 5 Dec 2024 11:26:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E62496B019D; Thu, 5 Dec 2024 11:25:59 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id C4E876B019B for ; Thu, 5 Dec 2024 11:25:59 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 7D6C414161E for ; Thu, 5 Dec 2024 16:25:59 +0000 (UTC) X-FDA: 82861431324.03.90EAC49 Received: from out-175.mta1.migadu.com (out-175.mta1.migadu.com [95.215.58.175]) by imf17.hostedemail.com (Postfix) with ESMTP id F1DA440002 for ; Thu, 5 Dec 2024 16:25:44 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=tLirK+ug; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf17.hostedemail.com: domain of hao.ge@linux.dev designates 95.215.58.175 as permitted sender) smtp.mailfrom=hao.ge@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733415942; a=rsa-sha256; cv=none; b=TihvJr0gkUJx50RLFAPrIHlFpbKRK6OLmNkJ6twFsYPJ5/BO0f47tifrQdCn4kCkkpDDrd h405yzJajE7ypecNP1qAFu518iIH0l0dD0Q0LLHViZUwYsvzWU/RN7grmB9/dZj69rm1sC ETFgFsPshe7ZquEyCbVr6PTvcrgZK2Y= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=tLirK+ug; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf17.hostedemail.com: domain of hao.ge@linux.dev designates 95.215.58.175 as permitted sender) smtp.mailfrom=hao.ge@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733415942; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=HG62bXMxKf5BwGoqprydcjPnr4H0yaXBkFv7zkXw7Zw=; b=pdZfEgkfZhl9gOHvhapeCRuKWKjVFL9larVxpfucgLRiS3iAD/RMM4sBcdrhAhiFh1Hrne zHZsrD+ZOAU54y/dgG5AABmZ0GLm4qCE7IGSiAGwIj9w6bbNgFyBYnnWmOhuIrAxbXejrp 2kPPuCo6chdzA4A1o/mfcBfj79OJHZY= Message-ID: <43bf216e-18f7-680c-ae75-773b03c8dc00@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1733415954; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=HG62bXMxKf5BwGoqprydcjPnr4H0yaXBkFv7zkXw7Zw=; b=tLirK+ugRY/kWRApc7yYbIpCiS3E2V3yylfhfLN5VLJPLW5QP3/ilYCyfzx6sITM2bXNXf fhYQtMc/ZvQhTN/NXmBA6kTpctHjJVcDo52SJULij79OHwhxNCAb6S3DkhNdth+918apbI DmEsnPKnKQVPp/+ymeESOJ7PquselJg= Date: Fri, 6 Dec 2024 00:25:47 +0800 MIME-Version: 1.0 Subject: Re: [PATCH v2] mm/alloc_tag: fix vm_module_tags_populate's KASAN poisoning logic X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Hao Ge To: Suren Baghdasaryan Cc: kent.overstreet@linux.dev, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Hao Ge References: <20241204075248.384215-1-hao.ge@linux.dev> <20241204083448.387862-1-hao.ge@linux.dev> <6dab626e-acee-9f4e-c97b-7a225882edff@linux.dev> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Stat-Signature: ouxw41wuxp9cwweae1a6ikj4w43ptk91 X-Rspam-User: X-Rspamd-Queue-Id: F1DA440002 X-Rspamd-Server: rspam08 X-HE-Tag: 1733415944-609474 X-HE-Meta: U2FsdGVkX180+PSfCZd4bZ7QU9YCe1n+Mm+e9FyS3oRNA+8JNyNqeQrmQMz1cd+e+I1DP/QeOsTuqhy3WoqRmuLNyEeWNp678P8OOSqSMnP8UFTt2+rioN+xPnzZ0GMUpVxizTnH/MxHuqZDud98oG1MWePwicGMhD0vAGn0KXP5vPuTwbkSRItVpI2/STC2cISm02zAl/vYkaGOUU/2oQvZRctMMmH0nyByn7r0Hzq5dqK5c/snsMkwJlogupGA4eXIUVccmjxHuHD64HxGqVIo9WwRoqUbLmw1hY2o50ng+evYpewUEli2Y/CgMWvBTBGRk1pfnVDy/JnUxvOhFKw1qUjkWI+RJBsnwXUGKG1TIfuB4SH3CLi9X2NfFMdr1pRwQlG6prJ9ROtWobA0XJDXppdIgPdXKCeBnBhNkmJ2ePtsrVDn3G/KMMOvoSYx2plzHAS14RCcDl1Ky0Cm3Rf2EdXWUcBowHGioaba5qu+WVdrGbn8JjKe3WTTV5SD1l7a5O8uv8rMnI3X82EZTTygJeVMKpbzGNU3dePeMeAtoXFZl+a/i65uEfNflyvsDJKfr17OkKdWep2bQ3WhgIadOXnA2oHCEe46JMqRWoHxiFIgQHrPA5MZmLL5Zwy/e2EJ+2K3l+uNxuoXhRZNX0eZhVK1iuojrTV6t5kfzZG+XKdJhkFW6/SlE53S3ZhunO2bzej5RR+tGE2eOmcRqiQGD+FT/P+ndcZuNbN81Iv2TzSmlJd/CXRULFRxBjdnTv9rVg3VkXk1E1P0csEc9TqFFPkig3WMyzuQtPyodPbdeVi6b/CgBxjTYe2gXMzc8B2c+aJW5AsjLd+7eqRmrU1so/U0jeCVuXsX88xRG0jkPd/ug7/1GFNtXw2NL8rRQbYNyMhzUhYt8VFQS+Ek6J4qFg8mz4IN7bKo3XaygFfRti7y81VelU39VmTGH4V66Hst5oE1Nuu0iqtZmPQ m76jqjzU OSYUb7M/KYPWRHlhtpHnqxwDun6aYFMkyfWaiy6g6r7hQO5jFOo7zeVsTN8eT3/d9WHVhk91CQvm9yallOoQRyGILoKsKPLlp2VUMo8KbKixO7Iygcz6vJQdYBSXp4CytMu2pRk5ydtUfGUIPRZOfbyEicehCGL0V5Drek7RO217vBOrz+UzJlBnkby6AQLgtVMUQljs3n3auklb40MaLKFj7TUm6Umvpk/U8 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hi  Suren I think I understand what you mean now. You're right. I think I shouldn't have included the unpoisoning process within the condition check for|phys_end < new_end|. Give me a moment while I make the necessary modifications and verify the new version. Thanks Best Regards Hao On 12/5/24 23:34, Hao Ge wrote: > Hi Suren > > > On 12/5/24 22:48, Suren Baghdasaryan wrote: >> On Wed, Dec 4, 2024 at 7:20 PM Hao Ge 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 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 wrote: >>>>>>>> From: Hao Ge >>>>>>>> >>>>>>>> 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 >>>>>>> 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 >>>>>>>>