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 76F25E7716F for ; Thu, 5 Dec 2024 03:21:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BC5856B0082; Wed, 4 Dec 2024 22:21:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B760E6B0083; Wed, 4 Dec 2024 22:21:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A3D2D6B0085; Wed, 4 Dec 2024 22:21:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 855E16B0082 for ; Wed, 4 Dec 2024 22:21:00 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 3343BAD8AE for ; Thu, 5 Dec 2024 03:21:00 +0000 (UTC) X-FDA: 82859453586.04.DAF897C Received: from out-176.mta0.migadu.com (out-176.mta0.migadu.com [91.218.175.176]) by imf05.hostedemail.com (Postfix) with ESMTP id 5589B100011 for ; Thu, 5 Dec 2024 03:20:29 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=WWJsiaaA; spf=pass (imf05.hostedemail.com: domain of hao.ge@linux.dev designates 91.218.175.176 as permitted sender) smtp.mailfrom=hao.ge@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733368851; 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=Qr8GW4K1UBAkugl9lZa9Mb7MVz0SqMO1J50BVo3IvyQ=; b=WJRgcFSvA9PlI//ZviwgPewx6lT+cA9GGcAs9D34tQg59RSpQXIo6XlNgwwcwW7mFs/7So 80kgL4zrBI4AN7zf9K0WPHN9dcBQ9Qd/EgqXxEmsBhcf+t7ElTpotmgYYaD9vTHaPzrNSC B/668IxEXnELWYpgVDELJqyjnVhpiVs= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733368851; a=rsa-sha256; cv=none; b=RHCKV4HLb5dEo93a7RyebzcFGB7kmNwIpE2Wv7nTFpUMoIi86PVaoURRl0k2gCMuhTlO0j ihJLE3zP3PrGfNpGd1LNrtSaQxMYN2MTmqw5HbshgKXyX0csEDX4/YxHMM48Ie8xqE1EEH 5NU4RPmG8KxCPg1GuInseHoJ2Zp96vc= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=WWJsiaaA; spf=pass (imf05.hostedemail.com: domain of hao.ge@linux.dev designates 91.218.175.176 as permitted sender) smtp.mailfrom=hao.ge@linux.dev; dmarc=pass (policy=none) header.from=linux.dev Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1733368855; 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=Qr8GW4K1UBAkugl9lZa9Mb7MVz0SqMO1J50BVo3IvyQ=; b=WWJsiaaAFpnmoAwUvNReeccXaYx7s5xc0g1YbwGRCzpJBL48U5qttJsNwkdM8P6LlOxZj9 WLVurcZhOqFrAyFP+faIziVJN/Dhp+/dxVlYOHX9jsErEBRcHOztqW+axrWXRrZ+CmIyLF 4SzPOj7/r+XjABfHg2PpVkWTkuOL94w= Date: Thu, 5 Dec 2024 11:20:48 +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: <6dab626e-acee-9f4e-c97b-7a225882edff@linux.dev> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 5589B100011 X-Stat-Signature: meyrrqoee4rxfkq81ei3jzar8z5de95a X-Rspam-User: X-HE-Tag: 1733368829-775194 X-HE-Meta: U2FsdGVkX1+9zjbIaBbi5Y0yhBg+41wl8W4WvBtTWU67WwPUyeR26aCgB5xiiOYqDZ2ya9KdGF67Z93/Wul8OHIxlk8wfmdVi8wJ4QSr26JbsOc9nYT/RZpzDnr0pDu2qBElR6pQS84+rNaJHXah0sGhNkTyycGlNAh/T/2/aRqhXP3OCz/FErlrcAwzdBHjSXdd77GqF9J2ckhlR75/AWBCwQ5hXld2XdSF1JnNhyiLNFeg3YyvWl3I6NEcXWODBr23SQPwBGgpSSxJyWkDXyFcq7FCZntxHsL0z64BPT8E5KGtexsZ8fKCNCyyjAzzGgJG6xIGhggO+dzFccEHcUW8A+V8GvCm8qOAoU0IhHt5Iw73H6tLE6joV2v/BhLtJzIHY05zd5leOnkbesm1Bde6RNtOi2wdpf/ewpUUm5zS6uZlI//r/NhRLFRyIVgd8qd9sn73bdG8Dg+QLMzS0Z3OAi+j/hwrpCPccj9FiByNrJw0nKHzSTwz6Ks5xLLcNu3/2hB7rqE2S3qmR/aLpzgiVzM7C06PhV9SOVb4iDkgE49iXxaaJEVXDE000Kulz8n2vmufji9XMchgtRI1GMKCuGn+rzZyZevv6vq39HN0IZheZ/qMDV0gCVwTclhtLbclbJqJ0+56TJ1MhChiW0GCT6rFP9dStI+EPz3JTN5DiKD7sZpYn2b96isU2INaKXp1/ni2p75sBQ4oK3eqrVwXZvlpdLUvUZdiCG7UDUjsiXl1XXBROsJbpXO+v0VEsNIXkUr6eXWFsZigK9Sawnh+5ytFPugEKjGpeD0wuRPu4e8mx1R3b6BC+SS+66IdJq+2zzFaeNMf8Brf8Pu+Y64OJGkR9dShimYsFEk60ZW1D85mOIPMdZnbjQulH6DSvK51i5s3f7O+4hHh4ZibUr6mcPbQFpMaMNpGTWo/oIsDOO2a/d2HGo1ll5luR0RZaS9HQalpFx4SBXPFI6u ZutAkMzI CgD8Pq6bj597HnWyss8z/P4Q1IbyLPU7MRKrmNYGAQp0gY4FlD579l6gqj3tVE/eOeVyTiVPCmAFuPNA6mYWZ6kSDjPtJOAnlJSwc69zoXyv9puCJbwEY30F2L9FuwA6Ne/s8O1grUfZsW5EWxQVDPkFMgugFes0dgf+Hb+7HK04Mg0XvTXiLa6MMHbTbMO7hSxw53aXuMi9TdOnQlcsJFW3xdA3fBN9FgG6d 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 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. 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. Based on that, I would suggest sticking with the V2 version. Thanks Best Regards Hao > > > Thanks > > Best regards > > Hao > > >>> >>>>>           } >>>>> >>>>>           return 0; >>>>> -- >>>>> 2.25.1 >>>>>