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 14F5FC77B7C for ; Tue, 24 Jun 2025 15:38:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8BFAB6B00C0; Tue, 24 Jun 2025 11:38:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 896AC6B00C1; Tue, 24 Jun 2025 11:38:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7D4316B00C3; Tue, 24 Jun 2025 11:38:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 6E33C6B00C0 for ; Tue, 24 Jun 2025 11:38:21 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 05B71C0B15 for ; Tue, 24 Jun 2025 15:38:21 +0000 (UTC) X-FDA: 83590700802.16.EE36CE9 Received: from mail-qt1-f176.google.com (mail-qt1-f176.google.com [209.85.160.176]) by imf30.hostedemail.com (Postfix) with ESMTP id 19BC680017 for ; Tue, 24 Jun 2025 15:38:18 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=b1rQi9Ni; spf=pass (imf30.hostedemail.com: domain of surenb@google.com designates 209.85.160.176 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1750779499; 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=Cq//EZtJtX/3kG7Q6lYMli8iIFmWk2rlPi0wEzAHGhI=; b=gbXcBF0Jzx+k46QpYH+1rho8lAVrx4RvISfunTDnORLF9sHcwZgaTioqi7JABKFIU+TpsK BS9pUoUBOQC7LBhLb3fN3td+E5oUPalg4qqTRRndRtPZxMaMl7a9jzzaUI2ucKHqjPjYBc Ax2x2/+YyeNmSd/5C29KmuCEBh+P5qc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1750779499; a=rsa-sha256; cv=none; b=rGbb4Y05yqU16DNDhoBhLg4NIQrrv8/wIpvVwX9lZC0YVTUzhrZmDvZ7SG6avtX9+21Zpm Uqnn1UVfnAZoD7ZxGhn1KdILveLS1/6atujxluBji60MgfDi9eQVvXwsLg2mFmMZMmCSte 91F8JVfsYtUYVyds+RdVv5wnqPrc2+E= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=b1rQi9Ni; spf=pass (imf30.hostedemail.com: domain of surenb@google.com designates 209.85.160.176 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-qt1-f176.google.com with SMTP id d75a77b69052e-4a5ac8fae12so519711cf.0 for ; Tue, 24 Jun 2025 08:38:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1750779498; x=1751384298; 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=Cq//EZtJtX/3kG7Q6lYMli8iIFmWk2rlPi0wEzAHGhI=; b=b1rQi9NixNoq1+lzW8tPbbLJ/qjlowlCZ4CQsL/FI0WGLiMMVW2XdtW29EF2t1g3Ux O2nD/wFWmQoJg9ke8SoyXSydo/7qOqk1tMfCv0shayiVQA1H1APvBRlBFTV6Pj6ljDct A7ZL0n/at9uyTp71/f12ucC7/Zgyv0rg7vBExYedE8VpHLR0MCg5tOFS31CsSMQYRBJL 2MiUh1qNnn4yBnkHUzJ7JqYgUcZ6AD/4kka39srsb6M7JgqHGWxWBUdqGacgSQJYbpM7 RfSNQo/RsIgDhF11UF1HgEsrfG5F6h4N7r/xSrZdyC/+UnOP38uts+GbcBlle1K8xRAG zT6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1750779498; x=1751384298; 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=Cq//EZtJtX/3kG7Q6lYMli8iIFmWk2rlPi0wEzAHGhI=; b=sPK0O0nPdHIwSo8fxXGIONP6rSjH+1slB/wLZ5krH851lnM3JfttQgCMbrqVQGsOW+ PKhEehv9/DTkhkU4kEFTT72xhbg54ciKpCYBg1oEnW92GXu53UqLS9HPvTH4gcNPqttj XlkaZ38DitvEzm5XifrTiuOpfCd7BAKUvR1pFdPbpb/S+zQSLX3UkkN3UIeHkLeoOf7n KSQum0c6AMdjO1gcctaEFb7zLd3QKZxZ/BXPbdiTXg95dqFAMneWh9qg5Txl907BKHwK Lpnnl2aLBAE/hH1lCwwIne/+nYy0B3kiRzR0nICq5DjzqZIMTvlquRBpCPzdWUjCfk2X IaJw== X-Forwarded-Encrypted: i=1; AJvYcCX8XTcsOJ8pE2quChR0WpQd9amf0J8jFiyoLw9iBK7A5mWrDiJmYVo5hg0JC6F8Olp7z3oTwTBeIA==@kvack.org X-Gm-Message-State: AOJu0Yxv0/Z+PsPqx7awmWaHoSRcRFODtQt4aIiHSlK4mr5GCHGGxFV8 i/hIKHSjdIKNDI8pBN3uslcUBtrJThm6eGZc68iyMMKDDqAzmPfqCbiJ8G3mE96spfw0Asop1en 68iVzPw9pyYM9FM+1TIz8CVNV++qqqQqlqTunPj5k X-Gm-Gg: ASbGncumxOX50CE96zIUaq2Fhi+wCjYwP0lIDgTNrR8/orfUtdCLTb/47/X7adnxXnN ++9AwpymJ0lxGgWiFWGg9oaoS6Ecy33mee96EpB6Bdfjb86DuvzhCR8472piJKd9bUSF60hk7/y V5THt/YO41idg0CSTuk467YuaFIMCwKrvctL4fLlPbc+k6eye2ZP/BXUQQhPuTrremG/T1GjLCp g== X-Google-Smtp-Source: AGHT+IHqGEoyCIb1kL6lCMHniFW2jNWsQ0MDjwjsbUEUDfZx7ba1RHnCsgxlTPT2lF8ldtmvuCceUKiyaAMBnv0QXWg= X-Received: by 2002:a05:622a:7a8e:b0:498:e884:7ca9 with SMTP id d75a77b69052e-4a7b16ec589mr3514991cf.13.1750779497561; Tue, 24 Jun 2025 08:38:17 -0700 (PDT) MIME-Version: 1.0 References: <20250624072513.84219-1-harry.yoo@oracle.com> <7f2f180f.a643.197a21de68c.Coremail.00107082@163.com> <2dba37c6.b15a.197a23dcce2.Coremail.00107082@163.com> <3942323b.b31d.197a2572832.Coremail.00107082@163.com> In-Reply-To: From: Suren Baghdasaryan Date: Tue, 24 Jun 2025 08:38:05 -0700 X-Gm-Features: Ac12FXzTGdsWvqL0LrY5iFd5379W7uqXNVh49LPmxI4wKUIDJuIADKSM0EDsg-w Message-ID: Subject: Re: [PATCH v3] lib/alloc_tag: do not acquire non-existent lock in alloc_tag_top_users() To: Harry Yoo Cc: David Wang <00107082@163.com>, akpm@linux-foundation.org, kent.overstreet@linux.dev, oliver.sang@intel.com, cachen@purestorage.com, linux-mm@kvack.org, oe-lkp@lists.linux.dev, stable@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 19BC680017 X-Rspam-User: X-Rspamd-Server: rspam06 X-Stat-Signature: nge9eq7hfzp1wyw43o3884fqoosf7w3o X-HE-Tag: 1750779498-992402 X-HE-Meta: U2FsdGVkX185+v4mUERWX9GfT6tJYU7FRL24edIfBWg7s0NgwOuWAIOGEhk4D5XQw3z1unlwxE3oOq3Vm/JkuFh5uE9pFSHcD9FvaaIo6gCwBj3hbG/f9PB5uPtgInjxcOoBGGbAafuUuWWbHhydzgDtLvubQXR7iewjK+tOQyeVVWciI89rF+7kpCz0vQPbDUyNcMPQVYnOTfQBG+gaI293dDp35EtcTRgbdUhCeV9fM9f3V5cCH1Bjr4L7CjbnKvZrmz8YK0yjkQdW3qHDSldhMdy9XZYyBzshEK679621yEvqqfTmo++4b+cm+H8qU7b8epQqEJY6E/sn4hujuJC81jNkBCqwzK3Sk1hEMu6cWx4vmWv/7tL7gjhS6wTLleaK/n6GWON01JfLy8axXm4CckcEYjx4b+4Zs2gwtA+UMjzcZnq53AYrxazw4hULKZwrf2Z4KBjbeLfCcl9CW7F84+ZXfUOsN15JrTeuUXJeyrkKSig0WDvIS7ckijCimSQQ2gT6TBBGIoGUGbRTSG/rkonB1KHNoMifFS2E0royzaO1VhtsGlHE9MYNMGhuOmGkp7OCDtIAam4rqU4moyXjNTTIwB4H/xGwsNU0aWV1mMm0ZGnt6ObGRibMlfkMqgK/UeDZ3Bpa7hHWZ93p//aZvb0CoJmN/QDXbX2VsNLV4efpidQB4SWvVIrbARsnivBWt4fG5cad/kV2HcQIVsI1aDcrPeiKkU8oyN/MAvDR9/lPF1qRmW2WKb+OnlWlJ15LGpM9ykS/AiWpWzi4Vp05By64UIW4cYB2nTK1R2tXmkU1b2F0X/yc2HfBA7kDcelWRRVYCXTYVjgvrru9qz6B6x2P9LGvhWGi6YeUbf9hepFrk0+dGNcc2nXYDnV4rkm4JH0l+QOIq8YxBDwGuFjAy1RQtOVwh7NFG3YCUzUFS+qgc5d9KktCjEi0/JfxM7pvSKO3gO/4ocG2h2N YSsl6z+L nMR8R5cUHco+8QxEtndjvpTS8dW/KVMEKN+tXP/WibXSjKJnGKowoC4JpSd1r8bZBxflL/0ZZcbgwas9O3kemi5zrovygTgwm+/i7PI4trSF1UgBnLvIQH2eGMKqUk3ryVAmYbH3JCTL5cFz/FnstuNLQYied/Uw8ubmGVrXIKEcKLQgeb3kBDxMspXo2UEZg+k6h9rnG9Dk9ArQFENomgYM++dfym6250uLRVrP64Kl/60tICfh3hev8Ikur05F7RmC1aBHwS/GxI93wZgF/2tFJwCslPQly9Um5HdCb7LEnZCN1h58dJJvrkyeE7jmLJA79ivcvYRxtZBUHLnfSSzGJs7wTYI9YgwbZc1aM8tnwyACI9k5EhCcpfdNDMoNebEYyUygsdtbJSfgEMyEfIk5oUweM3fkXj5cou2LoiBh7FAmrp1/3vaL4MyIs67/2TaiZHcD25JIdbeNsKkceNi0iB9Tv/mununV6 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: On Tue, Jun 24, 2025 at 8:14=E2=80=AFAM Harry Yoo wr= ote: > > On Tue, Jun 24, 2025 at 07:57:40AM -0700, Suren Baghdasaryan wrote: > > On Tue, Jun 24, 2025 at 7:28=E2=80=AFAM David Wang <00107082@163.com> w= rote: > > > > > > > > > At 2025-06-24 22:13:49, "Harry Yoo" wrote: > > > >On Tue, Jun 24, 2025 at 10:00:48PM +0800, David Wang wrote: > > > >> > > > >> At 2025-06-24 21:50:02, "Harry Yoo" wrote: > > > >> >On Tue, Jun 24, 2025 at 09:25:58PM +0800, David Wang wrote: > > > >> >> > > > >> >> At 2025-06-24 15:25:13, "Harry Yoo" wrot= e: > > > >> >> >alloc_tag_top_users() attempts to lock alloc_tag_cttype->mod_l= ock > > > >> >> >even when the alloc_tag_cttype is not allocated because: > > > >> >> > > > > >> >> > 1) alloc tagging is disabled because mem profiling is disabl= ed > > > >> >> > (!alloc_tag_cttype) > > > >> >> > 2) alloc tagging is enabled, but not yet initialized (!alloc= _tag_cttype) > > > >> >> > 3) alloc tagging is enabled, but failed initialization > > > >> >> > (!alloc_tag_cttype or IS_ERR(alloc_tag_cttype)) > > > >> >> > > > > >> >> >In all cases, alloc_tag_cttype is not allocated, and therefore > > > >> >> >alloc_tag_top_users() should not attempt to acquire the semaph= ore. > > > >> >> > > > > >> >> >This leads to a crash on memory allocation failure by attempti= ng to > > > >> >> >acquire a non-existent semaphore: > > > >> >> > > > > >> >> > Oops: general protection fault, probably for non-canonical a= ddress 0xdffffc000000001b: 0000 [#3] SMP KASAN NOPTI > > > >> >> > KASAN: null-ptr-deref in range [0x00000000000000d8-0x0000000= 0000000df] > > > >> >> > CPU: 2 UID: 0 PID: 1 Comm: systemd Tainted: G D = 6.16.0-rc2 #1 VOLUNTARY > > > >> >> > Tainted: [D]=3DDIE > > > >> >> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS = 1.16.2-debian-1.16.2-1 04/01/2014 > > > >> >> > RIP: 0010:down_read_trylock+0xaa/0x3b0 > > > >> >> > Code: d0 7c 08 84 d2 0f 85 a0 02 00 00 8b 0d df 31 dd 04 85 = c9 75 29 48 b8 00 00 00 00 00 fc ff df 48 8d 6b 68 48 89 ea 48 c1 ea 03 <80= > 3c 02 00 0f 85 88 02 00 00 48 3b 5b 68 0f 85 53 01 00 00 65 ff > > > >> >> > RSP: 0000:ffff8881002ce9b8 EFLAGS: 00010016 > > > >> >> > RAX: dffffc0000000000 RBX: 0000000000000070 RCX: 00000000000= 00000 > > > >> >> > RDX: 000000000000001b RSI: 000000000000000a RDI: 00000000000= 00070 > > > >> >> > RBP: 00000000000000d8 R08: 0000000000000001 R09: ffffed107dd= e49d1 > > > >> >> > R10: ffff8883eef24e8b R11: ffff8881002cec20 R12: 1ffff110200= 59d37 > > > >> >> > R13: 00000000003fff7b R14: ffff8881002cec20 R15: dffffc00000= 00000 > > > >> >> > FS: 00007f963f21d940(0000) GS:ffff888458ca6000(0000) knlGS:= 0000000000000000 > > > >> >> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > >> >> > CR2: 00007f963f5edf71 CR3: 000000010672c000 CR4: 00000000003= 50ef0 > > > >> >> > Call Trace: > > > >> >> > > > > >> >> > codetag_trylock_module_list+0xd/0x20 > > > >> >> > alloc_tag_top_users+0x369/0x4b0 > > > >> >> > __show_mem+0x1cd/0x6e0 > > > >> >> > warn_alloc+0x2b1/0x390 > > > >> >> > __alloc_frozen_pages_noprof+0x12b9/0x21a0 > > > >> >> > alloc_pages_mpol+0x135/0x3e0 > > > >> >> > alloc_slab_page+0x82/0xe0 > > > >> >> > new_slab+0x212/0x240 > > > >> >> > ___slab_alloc+0x82a/0xe00 > > > >> >> > > > > >> >> > > > > >> >> >As David Wang points out, this issue became easier to trigger = after commit > > > >> >> >780138b12381 ("alloc_tag: check mem_profiling_support in alloc= _tag_init"). > > > >> >> > > > > >> >> >Before the commit, the issue occurred only when it failed to a= llocate > > > >> >> >and initialize alloc_tag_cttype or if a memory allocation fail= s before > > > >> >> >alloc_tag_init() is called. After the commit, it can be easily= triggered > > > >> >> >when memory profiling is compiled but disabled at boot. > > > >> >> > > > > >> >> >To properly determine whether alloc_tag_init() has been called= and > > > >> >> >its data structures initialized, verify that alloc_tag_cttype = is a valid > > > >> >> >pointer before acquiring the semaphore. If the variable is NUL= L or an error > > > >> >> >value, it has not been properly initialized. In such a case, j= ust skip > > > >> >> >and do not attempt to acquire the semaphore. > > > >> >> > > > > >> >> >Reported-by: kernel test robot > > > >> >> >Closes: https://urldefense.com/v3/__https://lore.kernel.org/oe= -lkp/202506181351.bba867dd-lkp@intel.com__;!!ACWV5N9M2RV99hQ!MADvGKtvTvlLXN= xlrJ4BdOSnbsJlyrSroPUGJ3JQHs_IF-gxxqfQ89OTZ21aN96DbmjG9qH3Wi1MlgtiSA$ > > > >> >> >Closes: https://urldefense.com/v3/__https://lore.kernel.org/oe= -lkp/202506131711.5b41931c-lkp@intel.com__;!!ACWV5N9M2RV99hQ!MADvGKtvTvlLXN= xlrJ4BdOSnbsJlyrSroPUGJ3JQHs_IF-gxxqfQ89OTZ21aN96DbmjG9qH3Wi0o2OoynA$ > > > >> >> >Fixes: 780138b12381 ("alloc_tag: check mem_profiling_support i= n alloc_tag_init") > > > >> >> >Fixes: 1438d349d16b ("lib: add memory allocations report in sh= ow_mem()") > > > >> >> >Cc: stable@vger.kernel.org > > > >> >> >Signed-off-by: Harry Yoo > > > >> >> >--- > > > >> >> > > > > >> >> >@Suren: I did not add another pr_warn() because every error pa= th in > > > >> >> >alloc_tag_init() already has pr_err(). > > > >> >> > > > > >> >> >v2 -> v3: > > > >> >> >- Added another Closes: tag (David) > > > >> >> >- Moved the condition into a standalone if block for better re= adability > > > >> >> > (Suren) > > > >> >> >- Typo fix (Suren) > > > >> >> > > > > >> >> > lib/alloc_tag.c | 3 +++ > > > >> >> > 1 file changed, 3 insertions(+) > > > >> >> > > > > >> >> >diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c > > > >> >> >index 41ccfb035b7b..e9b33848700a 100644 > > > >> >> >--- a/lib/alloc_tag.c > > > >> >> >+++ b/lib/alloc_tag.c > > > >> >> >@@ -127,6 +127,9 @@ size_t alloc_tag_top_users(struct codetag_= bytes *tags, size_t count, bool can_sl > > > >> >> > struct codetag_bytes n; > > > >> >> > unsigned int i, nr =3D 0; > > > >> >> > > > > >> >> >+ if (IS_ERR_OR_NULL(alloc_tag_cttype)) > > > >> >> >+ return 0; > > > >> >> > > > >> >> What about mem_profiling_support set to 0 after alloc_tag_init,= in this case: > > > >> >> alloc_tag_cttype !=3D NULL && mem_profiling_support=3D=3D0 > > > >> >> > > > >> >> I kind of think alloc_tag_top_users should return 0 in this cas= e....and both mem_profiling_support and alloc_tag_cttype should be checked= .... > > > >> > > > > >> >After commit 780138b12381, alloc_tag_cttype is not allocated if > > > >> >!mem_profiling_support. (And that's why this bug showed up) > > > >> > > > >> There is a sysctl(/proc/sys/vm/mem_profiling) which can override m= em_profiling_support and set it to 0, after alloc_tag_init with mem_profili= ng_support=3D1 > > > > Wait, /proc/sys/vm/mem_profiling is changing mem_alloc_profiling_key, > > not mem_profiling_support. Am I missing something? > > Feels like it should call shutdown_mem_profiling() instead of > proc_do_static_key() (and also remove /proc/allocinfo)? No, we should be able to re-enable it later on. You can't do that if you call shutdown_mem_profiling(). mem_profiling_support is very different from mem_alloc_profiling_key. mem_profiling_support means memory profiling is not supported while mem_alloc_profiling_key means it's enabled or disabled and can be changed later. > > > > > > > > >Ok. Maybe it shouldn't report memory allocation information that is > > > >collected before mem profiling was disabled. (I'm not sure why it di= sabling > > > >at runtime is allowed, though) > > > > > > > >That's a good thing to have, but I think that's a behavioral change = in > > > >mem profiling, irrelevant to this bug and not a -stable thing. > > > > > > > >Maybe as a follow-up patch? > > > > > > Only a little more changes needed, I was suggesting: > > > > > > @@ -134,6 +122,14 @@ size_t alloc_tag_top_users(struct codetag_bytes = *tags, size_t count, bool can_sl > > > struct codetag_bytes n; > > > unsigned int i, nr =3D 0; > > > > > > + if (!mem_profiling_support) > > > + return 0; > > > > David is right that with /proc/sys/vm/mem_profiling memory profiling > > can be turned off at runtime but the above condition should be: > > if (!mem_alloc_profiling_enabled()) > > return 0; > > I agree that this change is a useful addition, but adding it to the patch > doesn't look right. It's doing two different things. You might be right, calling alloc_tag_top_users() while !mem_alloc_profiling_enabled() will print older data but it won't lead to UAF. > > > > + > > > + if (IS_ERR_OR_NULL(alloc_tag_cttype)) { > > > + pr_warn("alloctag module is not ready yet.\n"); > > > > I don't think spitting out this warning on every show_mem() is useful. > > If alloc_tag_cttype is invalid because codetag_register_type() failed > > then we already print an error here: > > https://elixir.bootlin.com/linux/v6.16-rc3/source/lib/alloc_tag.c#L829, > > so user has the logs to track this down. > > If show_mem() is called so early that alloc_tag_init() hasn't been > > called yet then missing allocation tag information would not be > > surprising I think, considering it's early boot. I don't think it's > > worth detecting and reporting such a state. > > > > > + return 0; > > > + } > > > + > > -- > Cheers, > Harry / Hyeonggon