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 EB70EE7716D for ; Thu, 5 Dec 2024 02:15:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7268E6B0085; Wed, 4 Dec 2024 21:15:52 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6D60F6B0088; Wed, 4 Dec 2024 21:15:52 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 59E076B008A; Wed, 4 Dec 2024 21:15:52 -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 3F4BF6B0088 for ; Wed, 4 Dec 2024 21:15:52 -0500 (EST) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id B790F121270 for ; Thu, 5 Dec 2024 02:15:51 +0000 (UTC) X-FDA: 82859289030.19.476C603 Received: from out-186.mta0.migadu.com (out-186.mta0.migadu.com [91.218.175.186]) by imf10.hostedemail.com (Postfix) with ESMTP id 38F66C0002 for ; Thu, 5 Dec 2024 02:15:43 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=fBlGrEdS; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf10.hostedemail.com: domain of hao.ge@linux.dev designates 91.218.175.186 as permitted sender) smtp.mailfrom=hao.ge@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733364936; a=rsa-sha256; cv=none; b=QiD6vB8Xxx9hDFIzwPamgt53PKZ3eHUwh/Dqm0+UOWp/yXGCb52HcTBog6FDJBlc3Oiakd A5fJtzZWkOwTAU2MUljaAbVKNdY+Zv/BcGGAqKRYlti/GrnD9UsQMFXkRBY17VHRQCg1eg Xss+759Js8EQqyY/t+fIgDUmJY67FRQ= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=fBlGrEdS; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf10.hostedemail.com: domain of hao.ge@linux.dev designates 91.218.175.186 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=1733364936; 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=pRTPO6yPQbL/JsKyn6272w8a89k5009o0qPxqVnHQWI=; b=qWuSKSIMLezbxLE9bUirIVFJJtoKoQo6KAnMn2vJD9q2L8rgtQxAbpFaIRHxrJaeD0zqxv 5ZzKZXtCotWaJK2aD2MWbcAJ9sNakVJU2qlHoJZ4ATvm+cYyxeA/dcSV1BJ4Vjlv+rfRR/ Q4XFA19AmMTq1TVZfIZNbuTOz9/U2tI= Message-ID: <6dab626e-acee-9f4e-c97b-7a225882edff@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1733364947; 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=pRTPO6yPQbL/JsKyn6272w8a89k5009o0qPxqVnHQWI=; b=fBlGrEdS0bOFerVMMX/sNiJHYQf6S6xmGrK0f8XAF7HGv/9JWAzBZX8B/238JkqS+aBBXv zb8HT2Ho4b6XAOvtUxbqR8CZ4OTETsf0kq/sB/J+NKewNEkM6COGVJqTmIOZWgwmRL76+G jPFSaLD3GY+BZuWd8Fwb5wniRdM1HB4= Date: Thu, 5 Dec 2024 10:14:54 +0800 MIME-Version: 1.0 Subject: Re: [PATCH v2] mm/alloc_tag: fix vm_module_tags_populate's KASAN poisoning logic 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> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Hao Ge In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 38F66C0002 X-Stat-Signature: tbpe3en47a4ujb54cmoc5w63bw641mpc X-Rspam-User: X-HE-Tag: 1733364943-21328 X-HE-Meta: U2FsdGVkX1951aq/5uubIeAPUrTA9cVSSitmZQ+BrEA+0q2IsGMDzECQNcNaApWRxKa92nPdO67c3jdRmbves2tqnxEywaN6uarP1L5aJ2r+UruUkMX9Si2YVNxKL3AmufdvtchFHopSd4KyKJjrtOmZn3COfIx9ynm1x8DKspIHCLSTOZbze+kViWHoub67gYyr5hXZ48bqGH77EqkbnYAbOhPoKdHy11RWTvKhsrlykygNGvMG017QEyNk5IGJnrr9I/p6Hu7kK+I8CNcw6gYh683FMozjIoHHZyVJJps+GuGOhP+weLUi14X2XKEapmuVKbfFjUn+WWr+2DQeEgipLe4qxHAZgIOgF8Y6e4Y6aYbH4WEBqnyUgn2NlYdmY60DRC18F+66AN5prvmPDM1KEISR9Mp+pn17spuTYgu6Ww05va6/hhqTxdchUxbUzAJy6pt1R5gcdtX6Xv8J1YjYaNuziA1NYqRr38/6S1waA4e3vOHWvp2TjVzi3vHKjq5O9TVKjYvxKIW1cRmoDHkIlYO19sjCao0Rl5/xjQvz9ymoWq4Ns7sclBmcmAItZxztfK4Y3lUKtUxMZZ2f3ulCCbSQ0+wJc0CQ96f3heRvnQ3kZsOP+0RMfkWxyTosTvzuAMYt/02jYGBxJTzxILvfzGXHaRbHclT69J+9+LwqLaIapNTi2847MJob5KBcAqFQ1E8/aWlMW1LnZML2YKaQem3WkNeuq/SAhkR39WyudZe9yz+nu+iwK84i5qLVYv37ZbTyzcJUkxbI9h1eRo641gOS6IgwS5GY2BHHM0AqrH4n0JxvVE5n03yEA+6/iaVrNZ0yl4st1h7E9j7PHjHjBJWRVccCgzWw1Po3Phz/+CxUNXn5GPSprBGLc4e1VH3UI3zMS2B65+jBBJd/fQmjjInOdiHC8U6jPvoz0wVJhjvXZctUMJ5UXDbBGxJok3C7g0x3MkZo3+PQ1pF EzvNW//Y fzKFK5bYjfRr+B0P50mpRwgS7Nd5hEBOSHzm+Jqfb+H0ea0bbIdTnAouIUD6dogkjMQvHLAmEGNv7RtzdkjS+ymyRDp0rzToioFt5Og5qJcZDpYLhPqn1lHC0uUeT8yVAFXqyqYFHdFxuP/IG2xJU9e/T3z8MG5qw7/9NzS+DbuUzN9mN+n/RXPIz2+3wQyhn5SGi//JWIURQSZoCbSqUkKIztY0Yid0/ePXh 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 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. Thanks Best regards Hao >> >>>> } >>>> >>>> return 0; >>>> -- >>>> 2.25.1 >>>>