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 7E92EECE57A for ; Mon, 9 Sep 2024 10:41:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DEDB06B015B; Mon, 9 Sep 2024 06:41:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DC5DB6B015C; Mon, 9 Sep 2024 06:41:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C8C276B015D; Mon, 9 Sep 2024 06:41:11 -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 ABBE76B015B for ; Mon, 9 Sep 2024 06:41:11 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 5BCE81A0F7F for ; Mon, 9 Sep 2024 10:41:11 +0000 (UTC) X-FDA: 82544857542.05.1659ACA Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) by imf30.hostedemail.com (Postfix) with ESMTP id 8C63380019 for ; Mon, 9 Sep 2024 10:41:09 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=ChekUJhm; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf30.hostedemail.com: domain of aha310510@gmail.com designates 209.85.216.44 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=1725878356; 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=Lg+a9nkxAendxU2JTg4zlQAG8wk22rpVaVjtWTZo5WI=; b=M7x2Rz/lTR+4HUx1Z+BSpdTbBV6wSnOGqZsB8yyebbfpyjWXHLHUFn7kZLSL83iqT6EvU5 wMxX3bT2rhxZyqrPMTmRfvke/XDQGran86AxiBkkfPoUXUT4LZsCBsxZ7D5/39DLClpP2i 3RrAcSeDs3lgHaYbRcJpK5priLaz7zs= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725878356; a=rsa-sha256; cv=none; b=aQvCPRnSIQZxKOd2YIobu6/UN+3kyyWkJmWH7uyMb0MdcwU8O+FCtco+hPYiXU3tLOaQZc mze+g3gpDcavQwFOUdVUqrMiO2IgtK4sXPg+mMAbSPY/K8QlUgbhfOFmONQsQm32xl6STO t0StCgn/6k1kABiq++SsXj5+AUaR8Ko= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=ChekUJhm; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf30.hostedemail.com: domain of aha310510@gmail.com designates 209.85.216.44 as permitted sender) smtp.mailfrom=aha310510@gmail.com Received: by mail-pj1-f44.google.com with SMTP id 98e67ed59e1d1-2d8b679d7f2so3276169a91.1 for ; Mon, 09 Sep 2024 03:41:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725878468; x=1726483268; 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=Lg+a9nkxAendxU2JTg4zlQAG8wk22rpVaVjtWTZo5WI=; b=ChekUJhmuvXSm0yROrJwqAgqBEGrE7BCI6W90VWl2wbnOSA4+8cgGQLT38WeumaDn9 /J+SQtVJ104bSuXxQZ4DTeF3WzbIEHsbGUZt6HUgE+REvKo1RToYfvLb1MD1uXjHHf8K E9ZTiPFpeY70Xh8OrQQdQV+Qn4nXLAATDeyGf8romJIGQ3Fz9qvoAHZAcqOvKBJRgNkr b7OFAtWEIzCOGru8Lh+axFb7LwhaFjGGRiIa5wfir0DxJA9Z7G56KOaQLTP1Cd/xUQKS gVD5VDlD0H/52f+Ynk0/A/OWkL1VZyWcPIhaOJoTd0+yim98P9kd/sOJEipJTqonZyMn qOpA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725878468; x=1726483268; 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=Lg+a9nkxAendxU2JTg4zlQAG8wk22rpVaVjtWTZo5WI=; b=dVyoFHcVndPNMIeZILlNMt6vvfOZRdn0UYaBIQV4d6whibjH4AV77k3gIgdISmWFB/ CMaDvSCWWIDzgl7T7F1uOmVfkC2L2pZSCLCoFh+Dnzw4MHXN86yuNrC7li28ywKo/sJr QO4ABwcL4hHoFT6TY4XSayPVAm/ZhvKhYkSXoV0xBGkLJRF3Rl2Rg105419kSwWGJFU/ zM+4ewPWTSu9xuE+TdtEacPzV+HZpme8fJZUkzJENJNSSzumx5of5ZfuBdxl/ItUSf2M JPNRy1hgJ3cTejjzuoJuNAeaTg1Z9nfOwJTUV/b4O0etFotWFIBLaFI8bKZuv05iLN7/ noNA== X-Forwarded-Encrypted: i=1; AJvYcCVjZ6bweeyYi3uUILCJaNb9wg7WFFXWVy/CJTFsjOq6qFEZCJpkhczH2yMcKL4M40Ot87XaRiUkMA==@kvack.org X-Gm-Message-State: AOJu0YyiZ+cZGKxGCcoR5FJv4wkEQP1E1M7e44TF/6VFJZOMU5jRqtO/ 1Aso196NsdHf1rzYL1hlvCD9X3eweR05EwlSqL+APuyUbX3FAWrfCddOM+GvRQJFD2A4C6q2V9C 6I/MKyEa88K5VuRGpUnXnRvRjVRMlPJef X-Google-Smtp-Source: AGHT+IG0QUD6m+AiJizTzfk7sjDUU6+Uj47dDxnNESvgtQR7HJ29HcD8i2Nk3eMqKuLCMZV3QNCkIKa3wkqzmCrt0SM= X-Received: by 2002:a17:90b:4b89:b0:2da:61e1:1597 with SMTP id 98e67ed59e1d1-2dad50f9f56mr12721352a91.36.1725878468069; Mon, 09 Sep 2024 03:41:08 -0700 (PDT) MIME-Version: 1.0 References: <20240908135209.15159-1-aha310510@gmail.com> In-Reply-To: From: Jeongjun Park Date: Mon, 9 Sep 2024 19:40:54 +0900 Message-ID: Subject: Re: [PATCH] percpu: fix data race in pcpu_alloc_noprof() and extend spinlock protection area To: Dennis Zhou Cc: tj@kernel.org, cl@linux.com, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, syzbot Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 8C63380019 X-Stat-Signature: onacgu7tr3niju7u6fsyfuax46iucka6 X-Rspam-User: X-HE-Tag: 1725878469-181896 X-HE-Meta: U2FsdGVkX1/H0qGsXVBHaQwqLpF40B6DGEkSGD6lu3P275kImdGsbxetKTnSUtPuoEZZ/07ppjYyGoUlffWUUmhVs072QGYsoTdeUAfL7i5RytkD340SELzHH3Qz6WPbCgtU6KUqmj0oQ8O86RjqHtajw7xhdU8psmYKjRth9ZNAq+gm//xGUINWaeXxX4zKov1CPPfPGkSJG+c1ZmMlGNEScj15YckVij/H/wv7pbFhuEQ52pJulsAa5y2+HZjEZnZjrU9yA/kJNBsnOJgYAc/pDrbSmDfco7iLZWxEHudS/Mk7+yuhKAQCVdmGyxvES1fHoCGuiPzda+W/RfNFM+gl09nPSYsUTKJf/eH57muBTcRIzCk5lLzajgIgKyYGrqs2yOkvd1yM557iAcQUDmqLftaxpmI569yaUBGFVNJs4+t8cEuJ8FMifTW+jjBCmwxI3cORh2q2JWjpbHCtMQo+uOx99pEIjsYynCbSeUb8h3W6FpYNE9emOmmj7P3G6CsHdm4DjI/YT391dWWQ5RCVevygxN7pHyJv5qlQr3gwNL1xIZUaNonhOc21getcELZRJdc3PePMjGQ+oqaJic5J3zyjo/CLujYftLIF56dJmAtewBjOwHhPdRlNoemMQTaZGnSSBj45SZ7xZ1sU+NpVNMz/Xgx+E1TxBXF3OK5aw1b4kjef12y46SWy+Wsp+hGeaOE4/ps4zbu/eQkWCM41uYl82/XtlRATQqMa8xpFS8lxzBLn5v0ko+25OykqzQ1pWy7+k2aWwmqMXHtN0kOVJWyFOQr1IUhu0fZ5lekGhE2+rF3tUZn3wWbEeCtihtqjisLeaUsFI0mbid8uf0RNzAMOxlosF0l/gibb1H+rKndAmu+DBVi7vi4om7FOoHPSCW9jshM97QOZ5W3modEn2puRS1gg90pMfKIudpa/tF2uDw67LYdormRVjMfG5XgArDeyxcaQkqPSLnD A73N7xk2 FS68etL0tfhhWx0Jzz4uAGPnBNEFt8bxcBln/Q/I1ipYhYXvxjk7V4N+iReukJvwHRhH+17LtJF7BTJn+IYtnPZeWc0mbbyHtobWowV7vUjqtrS8cBI0f7hqhTvS1GtgX9rTJ+GtHAVyIsq/m//WJzDqCg84LY60lOhAN2TrSJtHCxWSdsWjVqnuow62Gu0hvCEBrAXkZb0MitYRi1Sm5xAew++BZ6UJ9IKWWCc13Q5hVMfH0NuOTZH6PaBmIhJjC5PVixxbg1C+K7jKrLR5CWs+5DyqZTGHHwFicFSdqSowRe93P/MeRt0l2Mvo3unQ7dpTetzgrlolZ8uo8zN6gtrITzoZD+tbZ/Tsord1Yfi/FWdyL4WvgsdkKdoObtmcVPTppqFc3gWAKw232cxT4uCYSMEntRBw/c6DEBXDQT+bSEU9tb+aPuWC+a+kAPi6FJ8CNG0iQTV9gtU+mGzkCOy8JYQ== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: 2024=EB=85=84 9=EC=9B=94 9=EC=9D=BC (=EC=9B=94) =EC=98=A4=EC=A0=84 3:53, De= nnis Zhou =EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1: > > Hello, > > On Sun, Sep 08, 2024 at 10:52:10PM +0900, Jeongjun Park wrote: > > I got the following KCSAN report during syzbot testing: > > > > =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 pcpu_alloc_noprof / pcpu_free_area > > > > read-write to 0xffffffff883f872c of 4 bytes by task 3378 on cpu 0: > > pcpu_update_empty_pages mm/percpu.c:602 [inline] > > pcpu_block_update_hint_free mm/percpu.c:1044 [inline] > > pcpu_free_area+0x4dc/0x570 mm/percpu.c:1302 > > free_percpu+0x1c6/0xb30 mm/percpu.c:2277 > > xt_percpu_counter_free+0x63/0x80 net/netfilter/x_tables.c:1951 > > cleanup_entry+0x195/0x1c0 net/ipv6/netfilter/ip6_tables.c:671 > > __do_replace+0x470/0x580 net/ipv6/netfilter/ip6_tables.c:1099 > > do_replace net/ipv6/netfilter/ip6_tables.c:1158 [inline] > > do_ip6t_set_ctl+0x820/0x8c0 net/ipv6/netfilter/ip6_tables.c:1644 > > nf_setsockopt+0x195/0x1b0 net/netfilter/nf_sockopt.c:101 > > ipv6_setsockopt+0x126/0x140 net/ipv6/ipv6_sockglue.c:998 > > tcp_setsockopt+0x93/0xb0 net/ipv4/tcp.c:3768 > > sock_common_setsockopt+0x64/0x80 net/core/sock.c:3735 > > do_sock_setsockopt net/socket.c:2324 [inline] > > __sys_setsockopt+0x1d8/0x250 net/socket.c:2347 > > __do_sys_setsockopt net/socket.c:2356 [inline] > > __se_sys_setsockopt net/socket.c:2353 [inline] > > __x64_sys_setsockopt+0x66/0x80 net/socket.c:2353 > > x64_sys_call+0x278d/0x2d60 arch/x86/include/generated/asm/syscalls_64.= h:55 > > do_syscall_x64 arch/x86/entry/common.c:52 [inline] > > do_syscall_64+0x54/0x120 arch/x86/entry/common.c:83 > > entry_SYSCALL_64_after_hwframe+0x76/0x7e > > > > read to 0xffffffff883f872c of 4 bytes by task 3374 on cpu 1: > > pcpu_alloc_noprof+0x9a5/0x10c0 mm/percpu.c:1894 > > xt_percpu_counter_alloc+0x79/0x110 net/netfilter/x_tables.c:1931 > > find_check_entry net/ipv4/netfilter/ip_tables.c:526 [inline] > > translate_table+0x921/0xf70 net/ipv4/netfilter/ip_tables.c:716 > > do_replace net/ipv4/netfilter/ip_tables.c:1137 [inline] > > do_ipt_set_ctl+0x7bd/0x8b0 net/ipv4/netfilter/ip_tables.c:1635 > > nf_setsockopt+0x195/0x1b0 net/netfilter/nf_sockopt.c:101 > > ip_setsockopt+0xea/0x100 net/ipv4/ip_sockglue.c:1424 > > tcp_setsockopt+0x93/0xb0 net/ipv4/tcp.c:3768 > > sock_common_setsockopt+0x64/0x80 net/core/sock.c:3735 > > do_sock_setsockopt net/socket.c:2324 [inline] > > __sys_setsockopt+0x1d8/0x250 net/socket.c:2347 > > __do_sys_setsockopt net/socket.c:2356 [inline] > > __se_sys_setsockopt net/socket.c:2353 [inline] > > __x64_sys_setsockopt+0x66/0x80 net/socket.c:2353 > > x64_sys_call+0x278d/0x2d60 arch/x86/include/generated/asm/syscalls_64.= h:55 > > do_syscall_x64 arch/x86/entry/common.c:52 [inline] > > do_syscall_64+0x54/0x120 arch/x86/entry/common.c:83 > > entry_SYSCALL_64_after_hwframe+0x76/0x7e > > > > value changed: 0x00000005 -> 0x00000006 > > > > Reported by Kernel Concurrency Sanitizer on: > > CPU: 1 UID: 0 PID: 3374 Comm: syz-executor.3 Not tainted 6.11.0-rc6-syz= kaller-00326-gd1f2d51b711a-dirty #0 > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS= Google 08/06/2024 > > =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 > > > > The global variable pcpu_nr_empty_pop_pages can be protected by pcpu_lo= ck, > > but since pcpu_alloc_noprof reads outside the spinlock protection secti= on, > > a data race may occur and the branch of the conditional statement may c= hange. > > Therefore, the reading of pcpu_nr_empty_pop_pages should be modified to= be > > performed within the spinlock protection section. > > > > However, the for_each_clear_bitrange_from loop requires and uses a spin= lock, > > but it repeatedly locks and unlocks the spinlock unnecessarily. > > > > Therefore, I think it is appropriate to remove the repeated spin_lock a= nd > > spin_unlock in for_each_clear_bitrange_from and perform the operation o= f > > reading pcpu_nr_empty_pop_pages and then perform spin_unlock to postpon= e > > the point in time when the spin_unlock is performed. > > > > Reported-by: syzbot > > Fixes: e04d320838f5 ("percpu: indent the population block in pcpu_alloc= ()") > > Signed-off-by: Jeongjun Park > > --- > > mm/percpu.c | 5 ++--- > > 1 files changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/mm/percpu.c b/mm/percpu.c > > index 20d91af8c033..5c958a54da51 100644 > > --- a/mm/percpu.c > > +++ b/mm/percpu.c > > @@ -1864,7 +1864,6 @@ void __percpu *pcpu_alloc_noprof(size_t size, siz= e_t align, bool reserved, > > > > area_found: > > pcpu_stats_area_alloc(chunk, size); > > - spin_unlock_irqrestore(&pcpu_lock, flags); > > > > /* populate if not all pages are already there */ > > if (!is_atomic) { > > @@ -1878,14 +1877,12 @@ void __percpu *pcpu_alloc_noprof(size_t size, s= ize_t align, bool reserved, > > > > ret =3D pcpu_populate_chunk(chunk, rs, re, pcpu_g= fp); > > > > - spin_lock_irqsave(&pcpu_lock, flags); > > if (ret) { > > pcpu_free_area(chunk, off); > > err =3D "failed to populate"; > > goto fail_unlock; > > } > > pcpu_chunk_populated(chunk, rs, re); > > - spin_unlock_irqrestore(&pcpu_lock, flags); > > } > > We don't want to do this because pcpu_populate_chunk() calls > alloc_pages_node() which can block. > > > > mutex_unlock(&pcpu_alloc_mutex); > > @@ -1894,6 +1891,8 @@ void __percpu *pcpu_alloc_noprof(size_t size, siz= e_t align, bool reserved, > > if (pcpu_nr_empty_pop_pages < PCPU_EMPTY_POP_PAGES_LOW) > > pcpu_schedule_balance_work(); > > > > + spin_unlock_irqrestore(&pcpu_lock, flags); > > + > > /* clear the areas and return address relative to base address */ > > for_each_possible_cpu(cpu) > > memset((void *)pcpu_chunk_addr(chunk, cpu, 0) + off, 0, s= ize); > > -- > > I sent out [1] which is a more appropriate fix. I'll merge it later > today. > > Thanks, > Dennis > > [1] https://lore.kernel.org/lkml/20240906031151.80719-1-dennis@kernel.org= / > Oh, you already patched that bug a few days ago. Sorry for taking up your time. Regards, Jeongjun Park