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 B2EACC3ABC0 for ; Thu, 8 May 2025 06:18:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0AC416B0085; Thu, 8 May 2025 02:18:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 05A9F6B0088; Thu, 8 May 2025 02:18:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E8EF06B0089; Thu, 8 May 2025 02:18:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id C994F6B0085 for ; Thu, 8 May 2025 02:18:54 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id A93E0CA9C1 for ; Thu, 8 May 2025 06:18:54 +0000 (UTC) X-FDA: 83418737388.26.A02D641 Received: from mail-pg1-f178.google.com (mail-pg1-f178.google.com [209.85.215.178]) by imf25.hostedemail.com (Postfix) with ESMTP id C52F1A0008 for ; Thu, 8 May 2025 06:18:52 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Ja1rMEGB; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf25.hostedemail.com: domain of aha310510@gmail.com designates 209.85.215.178 as permitted sender) smtp.mailfrom=aha310510@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1746685132; a=rsa-sha256; cv=none; b=tM/+J84D+qj7tbjiTFxiZdLvccv5PFTgUymoBTJgORgdGs/vQhjXqYOJ5Jcai+OwMCbv9g OUEqSpsQ6Z21wK+dlVzsY/iEiIeBvNsja4/HguGztANIv+F+6zTmX+Yy7cOwI7GnqQLW5y spbGswntQ47rE4xOiY5HpJ1F1VtQOXg= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Ja1rMEGB; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf25.hostedemail.com: domain of aha310510@gmail.com designates 209.85.215.178 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=1746685132; 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=gnXX9/gWqGflBTi47okDnJ7YfQszH1NtWXK/5xCPxaw=; b=zdXVF88C9t+d2ulX8p3D2dNr40lgg2z7Qfj0oMcNJkG3xqKQl8K1zxW9qqJi6GtqDTZBnU ohSeuNDalsId7iyumJgaNHoMyQuQWirqJxuf1L/s4v7ra88WAwmwX0hlPRn5bD+NBZFi60 fOZo/1cRv7mSOSgbBJa+WE3XniAa4dA= Received: by mail-pg1-f178.google.com with SMTP id 41be03b00d2f7-b0b2d0b2843so413030a12.2 for ; Wed, 07 May 2025 23:18:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1746685131; x=1747289931; 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=gnXX9/gWqGflBTi47okDnJ7YfQszH1NtWXK/5xCPxaw=; b=Ja1rMEGBGD29q6PieegnIXIHmFTjs8P4xw46VcyUX7iyG6wdWbQFovV1/oeptyPlKb X7CfTD9mG3rbDu88YBl3PZxtxyZ2ynhZsaEA4OHcbBP2WEDC7i8GymLZl/VbqPd31k4X 7k3uK7RM+nnNZOLfOE3356gVKHV/aLKbqN3kTiXjYvKMUvOa3UEgQ88D5IS7VpWO7A5M z+rgFYwA0O1ORH9ZBmsOyO8n9yBpqBOoVWzTUkFqHKz7Vji5BUV0LWELldEEV1JH047h XmlTuxSGVek5hhyo7IseXSvUzRfGQuSCYM4inR/UwFmErxCk1A+aMQXZd1bRTbOScwlJ +UUw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1746685131; x=1747289931; 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=gnXX9/gWqGflBTi47okDnJ7YfQszH1NtWXK/5xCPxaw=; b=jX4Tn6nHzjnO2nDB5V+TjJGLASG7JzvC4dh1pPaIH/UBVS17JjU9eJ8wlJTMYhSdWD uUL5xVPmSILunlwuR7JYlkW1A//oEZoPW5CbP77qUncnthQ6U4XliGhCJXamCdVTnV3e xwlguHDDGpXiOnGKVZdoJ3hdyW/xTulTVZQtO7Q7Clh0Khnadzn7l8ybkxUZo4l97rDt uLH+79+WoV4PdReU1oahNW+pi/iSr63Y+2MOkbi+EN4H9SWDgcVGrd6tNtez1958ko5G 4B6nYChSMB7U4/0AKV2HMYvUr8QdamrLW6sIE+da2pSV5UEPcbuAWwOhHcSCzhaN8L1C k0aA== X-Forwarded-Encrypted: i=1; AJvYcCXoG8f1iiKXlPABWSQTxEnnrKAc/MQm6asDOutWydNUBUNz1bKmBsIVdy07rqIk+cxvilJQF3gcIg==@kvack.org X-Gm-Message-State: AOJu0YzKX8x+8JwRctNul88g3eJ0RoM+JYpLEAdTJL+rZdwgrHStMIQX LOlOvAHgmlt1Oc/0gMdHabGez+9UdalFRJyjPCsDxbvE0M2sp2upznUmb8OxuBZ1ZK2f+HFLBoF 2ayLnYT4eAfNuojKhQJX5+/zG2Kw= X-Gm-Gg: ASbGncufbhmwED2g9fr/E02Au0GL/xpnLKBmF53tP+eh1dFfI5gfcV8w3U4tCYjYpr3 2AsNJ/JOgeJjRYEHgIDWYB94seq/VbWz2GzN6d06mDXlGyfnVElC7GIHcS9P2ViYMA6rtKhO8K4 Yx5AV9lPET4ppIy6I3fY1dzA== X-Google-Smtp-Source: AGHT+IERyV23+ZJV3y67CEcKtPEA6LouYSlmL2EklZcF0P5oY7V8rBAWnMiCp0MB9owXZb2Tgh6z6yWtuAcUX3v8KSM= X-Received: by 2002:a17:90b:3b85:b0:301:9f62:a944 with SMTP id 98e67ed59e1d1-30aac2b3d1amr10049219a91.33.1746685131531; Wed, 07 May 2025 23:18:51 -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: From: Jeongjun Park Date: Thu, 8 May 2025 15:18:39 +0900 X-Gm-Features: ATxdqUHvSlkXI4Pq_-2LiiwIfvxZUNRefF1nJdTqgZxULCl5pafx55pOUZ5retQ 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: rspam11 X-Rspamd-Queue-Id: C52F1A0008 X-Stat-Signature: d6s1qfnrg65hpit1zak4jzodr7r3wxwz X-Rspam-User: X-HE-Tag: 1746685132-988452 X-HE-Meta: U2FsdGVkX18SOENhfoHYGsobUCPGE+ORXFCL8f8fAJIhXj6wEkwYzdJ5SVRX/BP7awNwGYtnfyTx/Vh/K4lNC5Ht0TKucp2O45QRWMkJfBOrgdiDuCqZ1wvvFbLuSNvkP32uG8jKeRz1rNCfJJznGHVqEa2UGfKE0dE2McrVCH/yuJOmYxSv9SD/yTXn7yzctZEejYz1egCnhdXkaG8QPlX/HWHAQ0Dm26hKiWnCoDTCd3UrGrzZVeKywXe63RlOB7lbWp4Fcnrjp/21GMJ7r/Z3PV5rOzCZxTTAolAKOoZGOgZJ8INGedgkF6S91YFnBCYqfrBUoOXpW3nEr3pFIQ/yOOuEkoI8OYJevm6Sy2Lmvab6/Nr15ZJyk7V88evyVhizVY3oWu6pBu5tx8x2QbArqXNdtyFvkIPvhAz6wp/GS28BwHkpHqv3dIBF3bXuTg3SVVRwS3gGYwyzsHh6WY5FCjW722EQHVF7A1eGuWW7MBMpnzaB8vQqksgnLBjhnqRrxxCicsJ8dp6YFhRSNiGxsnNKvvB2IbMDDbvOw8WzJA0ToIRGLYWL00Y4HmA6G/QFCeFn4IBxs+xByS/g3mX5SQXSKruhG9c3c+Tug2fW0Jxg3OMdGVdI7n6YBEaI6/djRLIx00E7B2TWovwYp72LsakVKhger8U4FNehp44v6NKafRdieM5NBlPHXkjZ9+nDl+8XLlZWpa67em1isGGorH9aTsxMmQY8C2hI4XgvtDscNZPpTfSoCuHroCq3sRBdAqv5z037c95NkNWf4EmFiTuL9r1406iuFxwPeaIwTDz8fB8kqd4jKgXj3tJHxSuamzCFsYhljWeB3wd84aqxNu3MdT2fNVzvQ5fv3WIWdbwvBWQW6NmM7du3VJUlh9CuJeFvmC192DIVv2We+rEYfsTSxHvlM6cxlGvso1ENnLi9nQUM7ee6+wNaWQdpcYQVHGgitPMNyG+6eFz UeV4Bu0R A3T6ycrcAuh28cqlaV+bEZRixBYjndUHaVDPbNxT4KdF2UIUTKMys6tV+zbxSoWvivJoaCRHuY7wnZBDsQnyCY+aTTW9F5kjvI81KW7zBZwV5fi9SKfONYt5aLyvw80Xi3C/AvaQFBZw7gmgQcIDfiwfjTK6Ui1qmp5MvSgeyBZolb1YAkzLJ7em2+RjzTC0bfRaaU6xsng0BfhicDxdNqBPlT7nPzga8cCTYUnZ9xM80nZ4qxWzwMUxI1xp5j4T8cVL15EqUeUS7qqnOWEzK3zcQCx07DurBgznud4+/cCEIIY+H7EUDZZtiOyQ+bVVVH97tQ6qYzkNpCh5Q2YhjtMRDJGNDL6EsSlSba24G0vgWO9kdXgOPRhBHD5ikwcmWGRFtrfzD4dpXh8JZQRHMNcWD0eXNh3owozLdZV1jA4D/NcUjrzymFtSGKiILyVIkU8xFhItUPCG9cAVjoPaZvQHUZT8BpRVhIWMwGnBEs6vGO/M= 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: Jeongjun Park wrote: > > Ozgur Kara wrote: > > > > Jeongjun Park , 7 May 2025 =C3=87ar, 17:32 tarihin= de =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_6= 4.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_6= 4.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->= private > > > is accessible to multiple CPUs. To fix this, instead of allocating th= e 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 se= ction > > > of a spinlock, so it must be allocated on the heap with GFP_ATOMIC fl= ag. > > > > > > 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 before= hand > > > to avoid printing uninitialized members of vm_struct. > > > - Link to v2: https://lore.kernel.org/all/20250506082520.84153-1-aha3= 10510@gmail.com/ > > > v2: Refactoring some functions and fix patch as per Eric Dumazet sugg= estion > > > - Link to v1: https://lore.kernel.org/all/20250505171948.24410-1-aha3= 10510@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_A= TOMIC); > > > + 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 reflec= t > this in the v4 patch. > > Regards, > > Jeongjun Park > Oh, I misread the code. This function already checks if the va->vm value is null, so there's no need to do this duplicate check. 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, counte= rs[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 i= nt), GFP_KERNEL); > > > - > > > - proc_create_single_data("vmallocinfo", > > > - 0400, NULL, vmalloc_info_show, priv_data); > > > - > > > + proc_create_single("vmallocinfo", 0400, NULL, vmalloc_info_sh= ow); > > > > 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); > > > -- > > > > > > > > >