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 E2ED4C3ABBE for ; Thu, 8 May 2025 05:04:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8EF866B0085; Thu, 8 May 2025 01:04:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8C5646B0088; Thu, 8 May 2025 01:04:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 78E426B0089; Thu, 8 May 2025 01:04:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 5ABE76B0085 for ; Thu, 8 May 2025 01:04:26 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id A231E1407B3 for ; Thu, 8 May 2025 05:04:27 +0000 (UTC) X-FDA: 83418549774.15.A4A7838 Received: from mail-pj1-f50.google.com (mail-pj1-f50.google.com [209.85.216.50]) by imf20.hostedemail.com (Postfix) with ESMTP id 9AABF1C0002 for ; Thu, 8 May 2025 05:04:25 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=awvTLnIa; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf20.hostedemail.com: domain of aha310510@gmail.com designates 209.85.216.50 as permitted sender) smtp.mailfrom=aha310510@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1746680665; 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=RQh0QX+GxBXDNWFhTBGL8N6/5cYPcAZibMDGc6Bv1HI=; b=4vqHkmpWGk8uSEOkJOJFcOUV+nUXyLcvqwweodQfCWE3bukMTypuxtImGxyqcc6wAgc/ii 6V8G7TpVXjpPq/lF+GldKafhyoGwyqws7Be9LT5aU+DEyGJHXfCoyhVn1fzSTFNpn06XwF 8xm2L2Og1YBVeHFjtR0J5T96m2vkqFA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1746680665; a=rsa-sha256; cv=none; b=isXpJVPCtdxn6gV6Ev3udhNC5OqXXgZMdXo0kRTtzW7mw5xjDQv5agTdzFpYvEOy4wDYmA NvunDVPllMl4Xen9FKuLH9f7RHRl82ApjTvVyPRjcg6nHvDcQAbn49KXWlp9tQYenMt9+W vgxRIH5+2nkmsQW5khyg08vIHOf7ahE= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=awvTLnIa; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf20.hostedemail.com: domain of aha310510@gmail.com designates 209.85.216.50 as permitted sender) smtp.mailfrom=aha310510@gmail.com Received: by mail-pj1-f50.google.com with SMTP id 98e67ed59e1d1-309d2e8c20cso820228a91.0 for ; Wed, 07 May 2025 22:04:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1746680664; x=1747285464; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=RQh0QX+GxBXDNWFhTBGL8N6/5cYPcAZibMDGc6Bv1HI=; b=awvTLnIaLZCW6fAxsGoAqWA7K6DLyUA97sZrSuKh5+634XuESTJbrqualRUrBPf/Rs FAuAfudgfQzdXCaAvNTN76SQyLS6YOjGpHRYNeR66BUMOnfKYQpOFtDajsi60OTfDJD8 iPQeZ7/Ww1lofMCIiyxwxL9viU07TX6pZMJA6k1ACQRj3+PmHoRMRkVpCBtMlXAdqRaR uCRpLaAB4DziA7p6iQ9vtFld/X+tuqMWYgsPRKiZP2IL9VeXwbk1oPO2M0sn+yZk9mq6 MLkqiaUgtqx/jL4w4/GZ+Ucra9q3+LkrH7K1zISw1XXs24artuBAq/vVNGTqnAIOgxGd 280A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1746680664; x=1747285464; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=RQh0QX+GxBXDNWFhTBGL8N6/5cYPcAZibMDGc6Bv1HI=; b=ty4VBk3+Gs3qF3GmYFTWeFhIjGzdj9PQl2tPrrpR3NAsnCmnGHIPRXbnLiDu1LPHrO P8AgaXqlpU3//GZu90rL/GS+PYQNgUHvlcN9fGpNSvWrfRLdf8Z2m9UbbvVVxUADY5dY yQXBfVHnbpt2u8ccLDwD4tGYGSkgzSZD9/P6VaiIHQz+8ntNwjvExpCnCw4aN9wdrSMb HGWW6eJpHbS6IT7TDQhDHtsGakTj0F7HKxEZVutw6l8P+YWF5RbOFe0bJdI+mNvp/jes rxsRV6VU1PQEOr2GSx7GCnBh8fCkqEfENtXcg6ifpzOPDMwIsYWpkSaPAamGQR4R3W1w tNdQ== X-Forwarded-Encrypted: i=1; AJvYcCVK+dzQAYhLyORoOloniYEwSudEs9l6/7yftwDi+qt9eqjMebXQAzr8Q0apMDFbGxXI+eNWRrLJ0A==@kvack.org X-Gm-Message-State: AOJu0YyMYpHm37+W/V1hwlPZK7IpWzMLxpvBBYq+7vFqff2xBcWJ8T83 CZDR8uo7Q1tUt4IhNKuNexkz6UtrzRuJnx/8OE+OeBqBqNzkSg7G5vTnLkmVBNBs+s3jdA7huA1 jVjYoEgqbHkFeVvoglNX8eAB0hJE= X-Gm-Gg: ASbGncuFV/53/26LzQ/KsxpW0IwR48iLtbIZ1pSc3ZDvqfKTf0wwsDY2pNeLuAqEqrE CePMprXmhztTkpx7c2Zg4z2mxfObtxRjqZUPtEbkvvzSQSzMJtTGjR4A6/50HetiH2SzNzcnNSz xmVrR118/4q25rtAYU6yNmBw== X-Google-Smtp-Source: AGHT+IGl/HETb93TXqKDxSrQg5qUxDwQHMS3IdObPyIVFD2nSrAXGYTD3dByv+nexkRsxIUlLHFg5oOtOmC2pd0UnJU= X-Received: by 2002:a17:90b:1d09:b0:2ff:7c2d:6ff3 with SMTP id 98e67ed59e1d1-30aac257b1dmr9558368a91.35.1746680664352; Wed, 07 May 2025 22:04:24 -0700 (PDT) MIME-Version: 1.0 References: <20250507142552.9446-1-aha310510@gmail.com> <01100196acf1ede5-ae116361-04f2-4e8f-b7a4-7079d6158ffb-000000@eu-north-1.amazonses.com> In-Reply-To: <01100196acf1ede5-ae116361-04f2-4e8f-b7a4-7079d6158ffb-000000@eu-north-1.amazonses.com> From: Jeongjun Park Date: Thu, 8 May 2025 14:04:13 +0900 X-Gm-Features: ATxdqUHe5Xv7cyJUHGbeF5sFahI0R_3r6XfksT6zDge-vRMlk7yINrLjLUUdoCg Message-ID: Subject: Re: [PATCH v3] mm/vmalloc: fix data race in show_numa_info() To: Ozgur Kara Cc: akpm@linux-foundation.org, urezki@gmail.com, edumazet@google.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 9AABF1C0002 X-Rspam-User: X-Stat-Signature: kowjcpwdmhc59c7g9bc1s31xhwp1gxw3 X-HE-Tag: 1746680665-574357 X-HE-Meta: U2FsdGVkX1+9HgkW7MGhomsWXPSwNdRI2oHTUpeKJXmF1knoY9vPqYeiNMiFDFsOmDf0/juFUxNf4IIwjaJHcMkK8ifez2xyE6ek/vo+Y9upWlAN63Mb5gEyhI6GbMDA3ilmf7IsyKoF90FgFCoyhhG4CAXLlCntJuhciU2GVYYLrTsGVrUrNiKDFvEz/6Y+154+ouVXr/hehunERg9Rbpje9onB7/7hSoP7Y2Z9l+YmhcsgywhCX5As8908LRcT4WejVcNGiwOXB3EDp8/8SzJwteAqG2waaqO9PIetqrEBbwIoh9QLMYl2zID4x08pMTyMeTimBObMGDKJaxFeqHYT/9nkAY+wxyh2OB4TTEP/ABjklwDj5lnARwS3FNOLgIagtpViWlxm3ghrG32UaJcdmGUpq16tLmOp+1jreq7JKGO3tsCRGd2pZqF2xewYKqb7BLlNkMuOvlriS6e01gPnBr1Loi3c3X7B/n1M+dQMCgOQBwDTPrApk2WXt8nYIpAJjt32fI9DhOubxCTpEBgD6/myiNARTP/k3oNGrsy7OcDm0a5cTReT/Ry7jQ77/Y+xAkkt7F+Qj5SgsdC5vtEuV2cZa9hPbuFshikxeNhVns+AnMUYfDE0IJDbtNAIeDQsZuzyBxCnwglg52BFI2cJUXBmG4EKUAc/jQ6ICybr0UDP97HxNDcTdGJ/QiH5FZA7dAMMIdIZccdcQ1JyxNlI7CT6axiWN02Tko3LYC1G1qI+FF5lnuBUMSfbNau53nJm/D08WlZVrI7Q7nlHJVS87k4sU7ttyolZsYhQCrANRC5Shc94kabfS6Y0RQo12pICGEEuulxgcs+kUbQ7v0T6SDttjk5nLs4PnqQrz2dRP6OeMyDyWKedk7bRWlwaBefWqVMDyWmdaH4kKcT/+BAD35GO6na73TnmEoCHbhYklWbteuXdA74WJZOoklEsSKsGcfjWzYC6NOeHmJQ ZJnBglRL mky4DRmxdK10lJ+ecoMo7BGctP1+Pxa+c96dPCBK/4fmS52JRyw7QyVSwvyyAQKWT57nOKncEwpnuS5IvcVp6ieOgcO5XW4gG/GYxY+zNs/JBcMe1xMmaJc84b1c6TnHgGImDLOwWDEE3/COH9L54fgjMSwNSFUks70/xDlbENgurNlN9MBvf9iL6er4nC/uCY7qsEurwifWkS0fFIgz+n9sy9Yu2cNUfyDYlkIiwoUn2joBZg1SvN7eVH7F9DUHvVmE61Sf0eb/bYxbKBYgIiY+oDZItJYTwC9Or40vs7bTLgqGTcAu+u5NJpjRzlc6r9swH+x6vHjFDtMtT5Z8lRKczd3cfUD/Nlc349f3tZF2in9HyVKtH6C5w3i88ajGB/aMQ7h+eiiZMVW6uI024lMIs21soTG3P486iEfvQTtHN3Dgz96XED1ZAm1J9a/PphrdJCP1ieyfuuvCkHvN2fWViZhpiRR86/nRop7xKL0Q+icE= 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: Ozgur Kara wrote: > > Jeongjun Park , 7 May 2025 =C3=87ar, 17:32 tarihinde= =C5=9Funu yazd=C4=B1: > > > > The following data-race was found in show_numa_info(): > > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > BUG: KCSAN: data-race in vmalloc_info_show / vmalloc_info_show > > > > read to 0xffff88800971fe30 of 4 bytes by task 8289 on cpu 0: > > show_numa_info mm/vmalloc.c:4936 [inline] > > vmalloc_info_show+0x5a8/0x7e0 mm/vmalloc.c:5016 > > seq_read_iter+0x373/0xb40 fs/seq_file.c:230 > > proc_reg_read_iter+0x11e/0x170 fs/proc/inode.c:299 > > new_sync_read fs/read_write.c:489 [inline] > > vfs_read+0x5b4/0x740 fs/read_write.c:570 > > ksys_read+0xbe/0x190 fs/read_write.c:713 > > __do_sys_read fs/read_write.c:722 [inline] > > __se_sys_read fs/read_write.c:720 [inline] > > __x64_sys_read+0x41/0x50 fs/read_write.c:720 > > x64_sys_call+0x1729/0x1fd0 arch/x86/include/generated/asm/syscalls_64.= h:1 > > do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline] > > do_syscall_64+0xa6/0x1b0 arch/x86/entry/syscall_64.c:94 > > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > > > write to 0xffff88800971fe30 of 4 bytes by task 8287 on cpu 1: > > show_numa_info mm/vmalloc.c:4934 [inline] > > vmalloc_info_show+0x38f/0x7e0 mm/vmalloc.c:5016 > > seq_read_iter+0x373/0xb40 fs/seq_file.c:230 > > proc_reg_read_iter+0x11e/0x170 fs/proc/inode.c:299 > > new_sync_read fs/read_write.c:489 [inline] > > vfs_read+0x5b4/0x740 fs/read_write.c:570 > > ksys_read+0xbe/0x190 fs/read_write.c:713 > > __do_sys_read fs/read_write.c:722 [inline] > > __se_sys_read fs/read_write.c:720 [inline] > > __x64_sys_read+0x41/0x50 fs/read_write.c:720 > > x64_sys_call+0x1729/0x1fd0 arch/x86/include/generated/asm/syscalls_64.= h:1 > > do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline] > > do_syscall_64+0xa6/0x1b0 arch/x86/entry/syscall_64.c:94 > > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > > > value changed: 0x0000008f -> 0x00000000 > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > > According to this report, there is a read/write data-race because m->pr= ivate > > is accessible to multiple CPUs. To fix this, instead of allocating the = heap > > in proc_vmalloc_init() and passing the heap address to m->private, > > show_numa_info() should allocate the heap. > > > > One thing to note is that show_numa_info() is called in a critical sect= ion > > of a spinlock, so it must be allocated on the heap with GFP_ATOMIC flag= . > > > > Fixes: a47a126ad5ea ("vmallocinfo: add NUMA information") > > Suggested-by: Eric Dumazet > > Suggested-by: Uladzislau Rezki (Sony) > > Signed-off-by: Jeongjun Park > > --- > > v3: Following Uladzislau Rezki's suggestion, we check v->flags beforeha= nd > > to avoid printing uninitialized members of vm_struct. > > - Link to v2: https://lore.kernel.org/all/20250506082520.84153-1-aha310= 510@gmail.com/ > > v2: Refactoring some functions and fix patch as per Eric Dumazet sugges= tion > > - Link to v1: https://lore.kernel.org/all/20250505171948.24410-1-aha310= 510@gmail.com/ > > --- > > mm/vmalloc.c | 51 ++++++++++++++++++++++++++------------------------- > > 1 file changed, 26 insertions(+), 25 deletions(-) > > > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > > index 3ed720a787ec..9139025e20e5 100644 > > --- a/mm/vmalloc.c > > +++ b/mm/vmalloc.c > > @@ -4914,28 +4914,32 @@ bool vmalloc_dump_obj(void *object) > > #endif > > > > #ifdef CONFIG_PROC_FS > > + > > +/* > > + * Print number of pages allocated on each memory node. > > + * > > + * This function can only be called if CONFIG_NUMA is enabled > > + * and VM_UNINITIALIZED bit in v->flags is disabled. > > + */ > > static void show_numa_info(struct seq_file *m, struct vm_struct *v) > > { > > - if (IS_ENABLED(CONFIG_NUMA)) { > > - unsigned int nr, *counters =3D m->private; > > - unsigned int step =3D 1U << vm_area_page_order(v); > > + unsigned int nr, *counters; > > + unsigned int step =3D 1U << vm_area_page_order(v); > > > > - if (!counters) > > - return; > > + counters =3D kcalloc(nr_node_ids, sizeof(unsigned int), GFP_ATO= MIC); > > + if (!counters) > > + return; > > > > - if (v->flags & VM_UNINITIALIZED) > > Hello, > > although skipping memory blocks with VM_UNINITIALIZED flag seems like > a good idea maybe it might be a good idea to check correctness of > memory areas. > > if (v && (v->flags & VM_UNINITIALIZED)) { > continue; > } > Thanks for the suggestion! Not related to data-race, but it seems like a good idea to add some check code in case null-deref occurs. I'll reflect this in the v4 patch. Regards, Jeongjun Park > > - return; > > - /* Pair with smp_wmb() in clear_vm_uninitialized_flag()= */ > > - smp_rmb(); > > + /* Pair with smp_wmb() in clear_vm_uninitialized_flag() */ > > + smp_rmb(); > > > > - memset(counters, 0, nr_node_ids * sizeof(unsigned int))= ; > > + for (nr =3D 0; nr < v->nr_pages; nr +=3D step) > > + counters[page_to_nid(v->pages[nr])] +=3D step; > > + for_each_node_state(nr, N_HIGH_MEMORY) > > + if (counters[nr]) > > + seq_printf(m, " N%u=3D%u", nr, counters[nr]); > > > > - for (nr =3D 0; nr < v->nr_pages; nr +=3D step) > > - counters[page_to_nid(v->pages[nr])] +=3D step; > > - for_each_node_state(nr, N_HIGH_MEMORY) > > - if (counters[nr]) > > - seq_printf(m, " N%u=3D%u", nr, counters= [nr]); > > - } > > + kfree(counters); > > } > > > > static void show_purge_info(struct seq_file *m) > > @@ -4979,6 +4983,8 @@ static int vmalloc_info_show(struct seq_file *m, = void *p) > > } > > > > v =3D va->vm; > > + if (v->flags & VM_UNINITIALIZED) > > + continue; > > > > seq_printf(m, "0x%pK-0x%pK %7ld", > > v->addr, v->addr + v->size, v->size); > > @@ -5013,7 +5019,9 @@ static int vmalloc_info_show(struct seq_file *m, = void *p) > > if (is_vmalloc_addr(v->pages)) > > seq_puts(m, " vpages"); > > > > - show_numa_info(m, v); > > + if (IS_ENABLED(CONFIG_NUMA)) > > + show_numa_info(m, v); > > + > > seq_putc(m, '\n'); > > } > > spin_unlock(&vn->busy.lock); > > @@ -5028,14 +5036,7 @@ static int vmalloc_info_show(struct seq_file *m,= void *p) > > > > static int __init proc_vmalloc_init(void) > > { > > - void *priv_data =3D NULL; > > - > > - if (IS_ENABLED(CONFIG_NUMA)) > > - priv_data =3D kmalloc(nr_node_ids * sizeof(unsigned int= ), GFP_KERNEL); > > - > > - proc_create_single_data("vmallocinfo", > > - 0400, NULL, vmalloc_info_show, priv_data); > > - > > + proc_create_single("vmallocinfo", 0400, NULL, vmalloc_info_show= ); > > proc_create_single function clean but it no longer receives data like > priv_data right? so if priv_data is needed again code will not work. > if use priv_data becomes necessary, a suitable memory allocation and > release mechanism should be added for this. > otherwise a memory leak could occur and perhaps the use of kfree > instead of kmalloc could also be added. > > proc_create_single_data("vmallocinfo", 0400, NULL, vmalloc_info_show, > priv_data); > > // use kfree and free priv_data > kfree(priv_data); > > Regards > > Ozgur > > > return 0; > > } > > module_init(proc_vmalloc_init); > > -- > > > > > >