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 7A960C77B7C for ; Tue, 24 Jun 2025 14:57:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 16E126B009F; Tue, 24 Jun 2025 10:57:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1469D6B00A1; Tue, 24 Jun 2025 10:57:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 05C4D6B00B2; Tue, 24 Jun 2025 10:57:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id E878E6B00A7 for ; Tue, 24 Jun 2025 10:57:54 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 942711D762F for ; Tue, 24 Jun 2025 14:57:54 +0000 (UTC) X-FDA: 83590598868.23.1750363 Received: from mail-qt1-f173.google.com (mail-qt1-f173.google.com [209.85.160.173]) by imf29.hostedemail.com (Postfix) with ESMTP id A81D912000B for ; Tue, 24 Jun 2025 14:57:52 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=BrGN3P+9; spf=pass (imf29.hostedemail.com: domain of surenb@google.com designates 209.85.160.173 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=1750777072; 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=e3PeI/4aDsKXp+ZVEGu/5dLCW8wfvxsS+RZNNnOopr8=; b=vkW9M9jXBFvRNnJaT+JzixqCS2oW3gLKsrM6l+kwyNTcDdc+7ysMWEtQaIkjgC8WwIVPrS i2tOHj4jKWeBNZUn6XuG+5+lsJu17gTX38jNBfUrd2/wRX5JEMnAu9fil06020ZTDNqCnD B72d0T9AN+NNrzJNDkWJn/K1Uq3FD1o= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1750777072; a=rsa-sha256; cv=none; b=4dOQdt7qNuO+bROzwP9SqYWj59J1eRfORHGAXX7s63W1mubc+JaCFgUBENX5slUj5OJZsE eZ1kwHKmv5QYtJeDM55E767g+qxuZDteg+XdKrgqcylxh+GH35EOPaGg9fJkO11cMkcMXi kJmz74/4H+DXZF5hWXD3DfNTHEjTfos= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=BrGN3P+9; spf=pass (imf29.hostedemail.com: domain of surenb@google.com designates 209.85.160.173 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-qt1-f173.google.com with SMTP id d75a77b69052e-4a5ac8fae12so492311cf.0 for ; Tue, 24 Jun 2025 07:57:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1750777072; x=1751381872; 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=e3PeI/4aDsKXp+ZVEGu/5dLCW8wfvxsS+RZNNnOopr8=; b=BrGN3P+9cLuztDhPP8nGIoUz7GpplZVHjjnFkIj18RLp7quhUpng5C2/4bFW9ID1qG zUlk+srQy3ZFmVJsab2hA6ALCkyngz9WdRMWMux+86xZurH9yJNyIzl98K9LezosSzaI SW0EAthTNuJnc06fyDV9SFaEmZFu8yNb3lPoaQua9f0LFdSNeo76dEmWCDCkvc6rVFgg 51NHnQD6+nMHipxp7EfgQE3lYepPIGc2rSaH1QtTS7Rzdy1TXi/Qsem+C899hFJjKx9+ D1zgEeC/X6KC37y1aLZnJDfhXpspJOeOs1CzCnxuZbFCqnm5TrrKlgn/6ZZm6lOFpFRC aSdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1750777072; x=1751381872; 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=e3PeI/4aDsKXp+ZVEGu/5dLCW8wfvxsS+RZNNnOopr8=; b=T/8QV3sz47fy+nQjddPoFuTn3eildoPwLTTJmNF4yFHrF8Aw8zMKy506JtO0f8c7T7 jZ7R4sMdH/kW+1VAT6R/ngocK87jZ+cRvnzgQr4de9SPKVThaKqLs6uOvWQiX3ujwFdw Q3logjB8jV/Gik3gv5MyVJrcmpDMB9mMpIVEjUSxnOtJavimvz80VQvjSAYlGOvGcJ7o lLkULkLSCsvQ1LzwPZLetqW+XSHmpZbU55LCY7G+Qz5uX501yuoDWVpj3lNvOjcx8s6b 3jGhH5Dk6JQtQ7sy/lSpWJu7ek5SQVZ/sd9uKTalJ5obuDIA5qQUpQ4lxACuO+Kaw/QF Q+Iw== X-Forwarded-Encrypted: i=1; AJvYcCWh4ycpBsTAhJ6ngA4aSJXoFUwFLsRbMzWOSzPMs6Srelv730w593UcBtctjMwEa+ByeEvJpYcQTA==@kvack.org X-Gm-Message-State: AOJu0YxJxiVTKseepcSlkiFiwH77oMClDxBWwl+27cvvGprEeZp0K1T5 Cnnpw4xyqnBlcHXbKpjprbSjQHzxXgbv+ceeZhmcPeJtSfFAnDgzlZyWYEIxlgwux9ZpZf3e4i6 Bl0RLg4o++hE3T8KpiKKUMuH2x3eT0jt4gWcioaZa X-Gm-Gg: ASbGncubj02Ay0W92FGU1Pp511zAUMAZieNmjlTABEFLaEzP9GhXazBMMBxJOlcIa4j obVqVjdzAwqrtl8+XGwLHny4jV1CBTFN6iVVmcz0KkjVziSXPjlxf/MiEewKQz3antA/FePQjnd jquYLbyUqFGVDjsEqMngFYZIdVlIcihm2t26EQY7hiFsiR8Vwog9qar45ans/PdID/mre6Vfjkv w== X-Google-Smtp-Source: AGHT+IE/8EKQLg3rDLzXEyGk8O9aooPX+CVFWWHBn/u+kyuJsVd/1VbvNQvmvqIwO+mdR6WZAO0VlZLUuWDIC1e0wOw= X-Received: by 2002:ac8:7dcd:0:b0:4a6:fc57:b85a with SMTP id d75a77b69052e-4a7af56fef0mr4289511cf.14.1750777071405; Tue, 24 Jun 2025 07:57:51 -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: <3942323b.b31d.197a2572832.Coremail.00107082@163.com> From: Suren Baghdasaryan Date: Tue, 24 Jun 2025 07:57:40 -0700 X-Gm-Features: Ac12FXy_yceQLqTPpBUjK-UH9noWkyxq_rmHu2wNxXxhQRvpuqB3eo42d_2gwEw Message-ID: Subject: Re: [PATCH v3] lib/alloc_tag: do not acquire non-existent lock in alloc_tag_top_users() To: David Wang <00107082@163.com> Cc: Harry Yoo , 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: A81D912000B X-Rspam-User: X-Rspamd-Server: rspam06 X-Stat-Signature: o8b8y4oawpjd3rfuobzrfebgxr4o9skk X-HE-Tag: 1750777072-500604 X-HE-Meta: U2FsdGVkX199IxSlOqKAz3rH6DhBR1Cn4tJtqB0pLYzXS82VuNnovrv2bnzACUkxoWh50Tvhtt+6TL+G/KXmj39pxcAuk394Mwz6wNyolBE4PxZCRtD8A3U5S8swD+kvbhyH4h3UQtmW7fj7Y/CoLBdBfHsc7EcANW1zyQ9/8B369aMwg1KUqTPxEmdxcoQ1jTP1w2O3Xhnrb7M5oJBvGxTOliXQWjO9ZU94hY+EapzxT4Hq+NGTCkVGuK5m7QOc/D6lEvF1TKBpph4Ss6X1GJmWKqzeHbcsFnx/gW1GRtoKxx3oVRBIA5up5mpYtaQUmhB1raCfi4nv3FChddvsp0hbZmQV1IAVwPPGSh+3fsB3mVb/i7CoGvu1hjVKCjAnYoMYZB6As0f8ajfcjXB0Vo13So/0khfpI9duXF/fjU61n40H35S4++y/kp8ESeXtdl18JvYIYNqep+vRQ8eqAf0AwvjIH4R0pzAr1mKRpYmcv0haA957vxzmbu0llxX5TnV0JptcMvgcU+a3AzAuJV8VM4KPEF7crkbt5QRISfi2jdaKDlFg9/MWcu4Jph7eWBaMsHVgeWmDFG4dK0QC0o6GOJJCTzWx5oJze+ok5i5qnatAQxs1O1qS6cG3cyQl/pr4xG5qBg3PexkqFVgzoOmAJtUu7oLcvEgMUxntJZYeZ4zLVgNRH1aZtmMF1qNzrqgrnnBZq10kp5dqzbCuLYHfvdaue3dnpb5UwPLYPsrr1hDmDrOK72NAeC5i6hPi58NQTyj7nqy/jHd5p7OVdEyYsbo0D4YApTOP+0RiB4XV4EUk3xL1faQdj5Bu7Wj4pdZ7PMVWKsRD3UW6igueGvuUcSPI8rrR9Fd+ZFj/BHQo08eP9xiHNnttbPtSfgheKEnK3ZwDS1FTDfY9xeYNzw+AvhQpda22BtmTouCR+aW0KIgrgo+T658U4PPHFCxhouq+5z4lU+MV1/eY+G4 tb/ZxmoF 3Wb4RMbKj7kpRilXTGYqD4g1N8PMQkZS0/yKBcItRuztHhUUiCg8Jnb4l3F3AyJHCY7sb3siQDYLGcmYmVlWtaOp4MIUKRyBzwIY6TEn0HytN3FWi0j0kFQJfwBCXSLOqQ+I52L37E4N5zn/5IHO95CP1/SNIp7mh4fVhtG9GOytZTKct+NNwO761lay3mjTD8ynrSAqADKq4tTDIkRrkK1M7gLHlCsKoVto/UgCWoxcPW1Rnss3jcLNxLD+1bQg06Dlrgzs8ISS/oYJRZHn80kGxDHiowgbQrpIJHprNEDINfzk5JOO/f9AJ8x8a2uGAok2AX/FB3nO6NyHM+iN8gBe2WMcwUx9douPWlYhu1v3ym0rXkujnk9bVtYuot/7PYVuDfd23YWvecOh+eWOmzUS7lDdmW4x/FYAsrMBuKBOBh8eDPj+IDkYli+4bOyzQptSa0rfjwB5uEWQ5tQOV6Osll0DTS+6aDWID 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 7:28=E2=80=AFAM David Wang <00107082@163.com> wrote= : > > > 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" wrote: > >> >> >alloc_tag_top_users() attempts to lock alloc_tag_cttype->mod_lock > >> >> >even when the alloc_tag_cttype is not allocated because: > >> >> > > >> >> > 1) alloc tagging is disabled because mem profiling is disabled > >> >> > (!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 semaphore. > >> >> > > >> >> >This leads to a crash on memory allocation failure by attempting t= o > >> >> >acquire a non-existent semaphore: > >> >> > > >> >> > Oops: general protection fault, probably for non-canonical addre= ss 0xdffffc000000001b: 0000 [#3] SMP KASAN NOPTI > >> >> > KASAN: null-ptr-deref in range [0x00000000000000d8-0x00000000000= 000df] > >> >> > 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 7= 5 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: 000000000000000= 0 > >> >> > RDX: 000000000000001b RSI: 000000000000000a RDI: 000000000000007= 0 > >> >> > RBP: 00000000000000d8 R08: 0000000000000001 R09: ffffed107dde49d= 1 > >> >> > R10: ffff8883eef24e8b R11: ffff8881002cec20 R12: 1ffff11020059d3= 7 > >> >> > R13: 00000000003fff7b R14: ffff8881002cec20 R15: dffffc000000000= 0 > >> >> > FS: 00007f963f21d940(0000) GS:ffff888458ca6000(0000) knlGS:0000= 000000000000 > >> >> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > >> >> > CR2: 00007f963f5edf71 CR3: 000000010672c000 CR4: 0000000000350ef= 0 > >> >> > 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 afte= r commit > >> >> >780138b12381 ("alloc_tag: check mem_profiling_support in alloc_tag= _init"). > >> >> > > >> >> >Before the commit, the issue occurred only when it failed to alloc= ate > >> >> >and initialize alloc_tag_cttype or if a memory allocation fails be= fore > >> >> >alloc_tag_init() is called. After the commit, it can be easily tri= ggered > >> >> >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 NULL or= an error > >> >> >value, it has not been properly initialized. In such a case, just = 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!MADvGKtvTvlLXNxlrJ= 4BdOSnbsJlyrSroPUGJ3JQHs_IF-gxxqfQ89OTZ21aN96DbmjG9qH3Wi1MlgtiSA$ > >> >> >Closes: https://urldefense.com/v3/__https://lore.kernel.org/oe-lkp= /202506131711.5b41931c-lkp@intel.com__;!!ACWV5N9M2RV99hQ!MADvGKtvTvlLXNxlrJ= 4BdOSnbsJlyrSroPUGJ3JQHs_IF-gxxqfQ89OTZ21aN96DbmjG9qH3Wi0o2OoynA$ > >> >> >Fixes: 780138b12381 ("alloc_tag: check mem_profiling_support in al= loc_tag_init") > >> >> >Fixes: 1438d349d16b ("lib: add memory allocations report in show_m= em()") > >> >> >Cc: stable@vger.kernel.org > >> >> >Signed-off-by: Harry Yoo > >> >> >--- > >> >> > > >> >> >@Suren: I did not add another pr_warn() because every error path i= n > >> >> >alloc_tag_init() already has pr_err(). > >> >> > > >> >> >v2 -> v3: > >> >> >- Added another Closes: tag (David) > >> >> >- Moved the condition into a standalone if block for better readab= ility > >> >> > (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_byte= s *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 case...= .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 mem_p= rofiling_support and set it to 0, after alloc_tag_init with mem_profiling_s= upport=3D1 Wait, /proc/sys/vm/mem_profiling is changing mem_alloc_profiling_key, not mem_profiling_support. Am I missing something? > > > >Ok. Maybe it shouldn't report memory allocation information that is > >collected before mem profiling was disabled. (I'm not sure why it disabl= ing > >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 *tag= s, 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; > + > + 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; > + } > + > if (can_sleep) > > > > David