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 78635D3E195 for ; Sat, 19 Oct 2024 00:56:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C5FE06B00AC; Fri, 18 Oct 2024 20:56:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C0F4A6B00AD; Fri, 18 Oct 2024 20:56:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AD72F6B00AE; Fri, 18 Oct 2024 20:56:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 8EC466B00AC for ; Fri, 18 Oct 2024 20:56:52 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 15122AB357 for ; Sat, 19 Oct 2024 00:56:27 +0000 (UTC) X-FDA: 82688536686.30.E073ED8 Received: from mail-ua1-f42.google.com (mail-ua1-f42.google.com [209.85.222.42]) by imf03.hostedemail.com (Postfix) with ESMTP id 8D26520008 for ; Sat, 19 Oct 2024 00:56:44 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=UkIIfMZK; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf03.hostedemail.com: domain of yuzhao@google.com designates 209.85.222.42 as permitted sender) smtp.mailfrom=yuzhao@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729299290; a=rsa-sha256; cv=none; b=W46cO9aKhE6rvurSCAM5y4X2E4wJqlJtQCYxcjSYVr41O70Ig4hjT7S48wKmThWjbQYLsE UceFJtAeoRZJvqZb3cDCWXbO6oR7uLrNu8d803bUnMdb3jnm0H803KEDpU5xsTLQH07gBM j6OTHl9pdUHQUiUJ6YJyjDTDtLsGnZ4= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=UkIIfMZK; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf03.hostedemail.com: domain of yuzhao@google.com designates 209.85.222.42 as permitted sender) smtp.mailfrom=yuzhao@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729299290; 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=+ogPnR+iPnz306L/2kyuRy57MK9w8E4o4gDlqm43OcY=; b=KanZ2ZD4n9/6Hnyd3xWeftsHs5YS2aSP2Kt8HPRqsTEsrfcN4jT9Ku/SyJKcNaj676hp5g T7ErUmoERh2QfP7HxQ2yAt8iacldj3Jp3b2Y232/fJn9w4exc/sEgQPrMmC+tJ9UVsYUqJ 3Nyw/UVbL6nNyU3yScdgh+KnIOVSLXY= Received: by mail-ua1-f42.google.com with SMTP id a1e0cc1a2514c-84fc818f518so762476241.2 for ; Fri, 18 Oct 2024 17:56:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1729299409; x=1729904209; 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=+ogPnR+iPnz306L/2kyuRy57MK9w8E4o4gDlqm43OcY=; b=UkIIfMZKMg1A84pn+iEnU7apTMWOkhz8Sd1Gn6MGvWSsMauAwF15ttpl//qQfVdGOO FvXLz8KJrTpez0AHfy/n87clklCIHUYR+6dBQR/XOAS7nlEwQAj/JEsdH5jtT9+/Qoi4 ZbOVhCMVu5nhcEg0M/iJ/choY0puVMOswNuraQLEFn+kOe4TcW27e5dm0D5XcrvGEcIu dkwqj+HB6GMcNtkY7GX5IQKwr1aSo/7WYDr4d9cMi7QqNAxdsYw1WI4FmCz09PsJ21uT H7ws/wlxL8Z2Fh6k9UCr8/wXO2ui23BIFPejFUlH12fg1U+T62EkAu7qYYQcvQrzyt/r Z3FA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729299409; x=1729904209; 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=+ogPnR+iPnz306L/2kyuRy57MK9w8E4o4gDlqm43OcY=; b=QsNz07/iU82EIHxGwZKN9XiJWYTynoQtj7dgZ6FW4Fa59bmrq/BqgTGoopd0OJX39j BpQZBz97XscDGWrFUqyIoQJ9mdPeNnWm50cZyqBXds0pidbmuEz2KqRtBcVoso2i4ozs PZQ+rPG/TWYGo8Yqv98jgZZ18o8t6pg2jeWWbqWDZx37xhHkG2HQrhqdATRSrCU7og1v EK2gW7K5qlmFdwwh1iKbJAdKPti3RV1kgpYkUD3WrOVMToX+8N9svXohS02yctT9U/r7 zcaHevdtHz8/7tvSISp/FDLM4/NylVxFJftqjXqNkCZ3GHSRwODkThGWlh9Dlu3jQgu0 QnLg== X-Forwarded-Encrypted: i=1; AJvYcCWKa/ux30aTvIkx6eTumcPS6CMErYymuKyyYqmgGWs99Z59rJEvvtmrIxUKNCBXD+QpS/SxubokMw==@kvack.org X-Gm-Message-State: AOJu0YwpDRNnwuEb+R3kcO8nUdgHpZBrjlIkch79NS3uIOjtVRVqHjY1 TPh4gkVHIU65JsJ9Fo3adIX4CgZKS+PoLAODH87aJfMDSATFjsQ3xhw8hR/wipXmXKouOnwG5K5 yK/ztn/eCrAD4997ZbXgxHdwSeTI7sMr67Ylf X-Google-Smtp-Source: AGHT+IGGzCk8DuoFf2e4+BWdxxCuoVDC+WR0hRVS416Ii7TDJ1/ieZz3jF/qq9mm8/zGhxUaVBeGFKI2RnL7Q5T02YE= X-Received: by 2002:a05:6102:c04:b0:4a3:cb2b:9745 with SMTP id ada2fe7eead31-4a5d6bb5f1emr5104987137.22.1729299408864; Fri, 18 Oct 2024 17:56:48 -0700 (PDT) MIME-Version: 1.0 References: <20241018152925.138341-1-hao.ge@linux.dev> <20241018162559.143548-1-hao.ge@linux.dev> In-Reply-To: From: Yu Zhao Date: Fri, 18 Oct 2024 18:56:08 -0600 Message-ID: Subject: Re: [PATCH v2] mm/codetag: move ref and tag null pointer check to alloc_tag_add To: Suren Baghdasaryan , Hao Ge Cc: kent.overstreet@linux.dev, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Hao Ge Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 8D26520008 X-Stat-Signature: oe6pp9ompnhn3gywg1zaddc7b47abciw X-Rspam-User: X-HE-Tag: 1729299404-78739 X-HE-Meta: U2FsdGVkX1+jAhlYaOXTjb1O5E065BoWsHC6x4SFToxJ8gH9BwIpvDU3mgzTFCxlNEv52s+AYk5GxgIXF3AsgmqYPV4aUoXzYC6t5ZAEZBqm0wdyT9pnX7h/RSRCm6Yp/5s+B1rP98F2JzLyk5mRMGIT4X4FS8frCp1R3Njgb0MegoeSqJx4FQkvsCnDbog+HDQ1BDmHS+dusf9yCqbkbbspNpdIk/xtgePlMPUWR3e3a8bh+cWCv+JDA7+4N0PYiLkYv42QNYjnW5Ab824bpIyXpqIwZGp4C1NMVZtkEFFcDXxZ+R7aIITTFAA5/6q+uDyLVU0bembLXCAbJxHv24S/o4J1+d/MXciUB4djGqe/0o2/S2ZyFpTrUgfZWaFJ947S7/pMVnjRKnKDf/LdkafSNuJDHxGxzxNWaj+qUJgwNESxZsPGmI1mQnbbloXI4pynqEV2I8nGy0b2x83hBaTiQJQiZ/LfBKeQqnaOTUZSEr9onSNBdscqC0QV8paq/i7GRXemzB7DBM1ISZMXWN9XXO0pMeAvxTjnvDl9y5yj6r5HZLj82rCzJLlX3LKRUalknIeALlY7dzqlfJ4ng4J0qaIZfWnlWWUsXSADWHbuQlvwJWn++zmF8PvJLWdJt3uNgKZD16XILLF0l64FEiQgbaV61M3Oydp6R776Aj8VOqSFsNU5XuRDsLusKvtXM5AKoStzyALG96aR1znNiGj4NzzKCglMfRRqmAr7+v5X3LfSWW+IQ1+fAkApCwSouP4nz7kSXtwi7yxWpB6W6jTwZ/xXU2O3bGtUiI9vaYgfELSDxTL49gWWClbh5uOayWc7pIq1kVPnVSnv6mntUVnDEQi2rKJGzTxYj48V9bpZw2XqiFMB1z30TqX+K5Y+sD25aa0DSi9/r9sKDH/kzyqOs4TebkAM9Hn9GREQggXV5N1OMLFkrS9uoqBaOJl8h6UpDa2GeAq12nopS13 mK7mdH/L guSdmqnykMRCZ/1+BM5wXlws/2bq4PSWuYkBhTDy5lXH9TxONNxekb3CMhCo5xRN8jdY9gyD2Gc6BUonxbABQ6Br/k4hpvlS59PqfuoZzN9E+ca03JWiFm5baItbfJb1bbERPWnGDW8hXT5Nvh0r8Pu+doh0gHDoj3x+V0/soADOlayWS/RVRZZZ5TjqPrc/CeClrBDvXLhrmJ8Vo6+RMjrYKl8hnBcXGEbfZuXYo1n60uf4ywNhFJy7kOSJEu+KrBGd1nxzhUZYKR3A= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000005, 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 Fri, Oct 18, 2024 at 11:30=E2=80=AFAM Suren Baghdasaryan wrote: > > On Fri, Oct 18, 2024 at 9:26=E2=80=AFAM Hao Ge wrote: > > > > From: Hao Ge > > > > When we compile and load lib/slub_kunit.c,it will cause a panic. > > > > The root cause is that __kmalloc_cache_noprof was directly called > > instead of kmem_cache_alloc,which resulted in no alloc_tag being > > allocated.This caused current->alloc_tag to be null,leading to a > > null pointer dereference in alloc_tag_ref_set. > > > > Despite the fact that my colleague Pei Xiao will later fix the code > > in slub_kunit.c,we still need to move the null pointer check for ref > > and tag to alloc_tag_add here. > > It is sufficient for us to issue a warning to the user; > > It should not lead to a panic. > > > > Here is the log for the panic: > > > > [ 74.779373][ T2158] Unable to handle kernel NULL pointer dereference= at virtual address 0000000000000020 > > [ 74.780130][ T2158] Mem abort info: > > [ 74.780406][ T2158] ESR =3D 0x0000000096000004 > > [ 74.780756][ T2158] EC =3D 0x25: DABT (current EL), IL =3D 32 bits > > [ 74.781225][ T2158] SET =3D 0, FnV =3D 0 > > [ 74.781529][ T2158] EA =3D 0, S1PTW =3D 0 > > [ 74.781836][ T2158] FSC =3D 0x04: level 0 translation fault > > [ 74.782288][ T2158] Data abort info: > > [ 74.782577][ T2158] ISV =3D 0, ISS =3D 0x00000004, ISS2 =3D 0x0000= 0000 > > [ 74.783068][ T2158] CM =3D 0, WnR =3D 0, TnD =3D 0, TagAccess =3D = 0 > > [ 74.783533][ T2158] GCS =3D 0, Overlay =3D 0, DirtyBit =3D 0, Xs = =3D 0 > > [ 74.784010][ T2158] user pgtable: 4k pages, 48-bit VAs, pgdp=3D00000= 00105f34000 > > [ 74.784586][ T2158] [0000000000000020] pgd=3D0000000000000000, p4d= =3D0000000000000000 > > [ 74.785293][ T2158] Internal error: Oops: 0000000096000004 [#1] SMP > > [ 74.785805][ T2158] Modules linked in: slub_kunit kunit ip6t_rpfilte= r ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack ebtable= _nat ebtable_broute ip6table_nat ip6table_mangle 4 > > [ 74.790661][ T2158] CPU: 0 UID: 0 PID: 2158 Comm: kunit_try_catch Kd= ump: loaded Tainted: G W N 6.12.0-rc3+ #2 > > [ 74.791535][ T2158] Tainted: [W]=3DWARN, [N]=3DTEST > > [ 74.791889][ T2158] Hardware name: QEMU KVM Virtual Machine, BIOS 0.= 0.0 02/06/2015 > > [ 74.792479][ T2158] pstate: 40400005 (nZcv daif +PAN -UAO -TCO -DIT = -SSBS BTYPE=3D--) > > [ 74.793101][ T2158] pc : alloc_tagging_slab_alloc_hook+0x120/0x270 > > [ 74.793607][ T2158] lr : alloc_tagging_slab_alloc_hook+0x120/0x270 > > [ 74.794095][ T2158] sp : ffff800084d33cd0 > > [ 74.794418][ T2158] x29: ffff800084d33cd0 x28: 0000000000000000 x27:= 0000000000000000 > > [ 74.795095][ T2158] x26: 0000000000000000 x25: 0000000000000012 x24:= ffff80007b30e314 > > [ 74.795822][ T2158] x23: ffff000390ff6f10 x22: 0000000000000000 x21:= 0000000000000088 > > [ 74.796555][ T2158] x20: ffff000390285840 x19: fffffd7fc3ef7830 x18:= ffffffffffffffff > > [ 74.797283][ T2158] x17: ffff8000800e63b4 x16: ffff80007b33afc4 x15:= ffff800081654c00 > > [ 74.798011][ T2158] x14: 0000000000000000 x13: 205d383531325420 x12:= 5b5d383734363537 > > [ 74.798744][ T2158] x11: ffff800084d337e0 x10: 000000000000005d x9 := 00000000ffffffd0 > > [ 74.799476][ T2158] x8 : 7f7f7f7f7f7f7f7f x7 : ffff80008219d188 x6 := c0000000ffff7fff > > [ 74.800206][ T2158] x5 : ffff0003fdbc9208 x4 : ffff800081edd188 x3 := 0000000000000001 > > [ 74.800932][ T2158] x2 : 0beaa6dee1ac5a00 x1 : 0beaa6dee1ac5a00 x0 := ffff80037c2cb000 > > [ 74.801656][ T2158] Call trace: > > [ 74.801954][ T2158] alloc_tagging_slab_alloc_hook+0x120/0x270 > > [ 74.802494][ T2158] __kmalloc_cache_noprof+0x148/0x33c > > [ 74.802976][ T2158] test_kmalloc_redzone_access+0x4c/0x104 [slub_ku= nit] > > [ 74.803607][ T2158] kunit_try_run_case+0x70/0x17c [kunit] > > [ 74.804124][ T2158] kunit_generic_run_threadfn_adapter+0x2c/0x4c [k= unit] > > [ 74.804768][ T2158] kthread+0x10c/0x118 > > [ 74.805141][ T2158] ret_from_fork+0x10/0x20 > > [ 74.805540][ T2158] Code: b9400a80 11000400 b9000a80 97ffd858 (f9401= 2d3) > > [ 74.806176][ T2158] SMP: stopping secondary CPUs > > [ 74.808130][ T2158] Starting crashdump kernel... Thank for catching this! > > Fixes: e0a955bf7f61 ("mm/codetag: add pgalloc_tag_copy()") > > Signed-off-by: Hao Ge > > --- > > v2: Modify the errors in the title and commit message. > > Remove the empty lines that were mistakenly added in version v1. > > --- > > include/linux/alloc_tag.h | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h > > index 1f0a9ff23a2c..8603e3a9df10 100644 > > --- a/include/linux/alloc_tag.h > > +++ b/include/linux/alloc_tag.h > > @@ -137,10 +137,6 @@ static inline void alloc_tag_sub_check(union codet= ag_ref *ref) {} > > /* Caller should verify both ref and tag to be valid */ > > static inline void __alloc_tag_ref_set(union codetag_ref *ref, struct = alloc_tag *tag) > > { > > - alloc_tag_add_check(ref, tag); > > - if (!ref || !tag) > > - return; > > - > > Unfortunately this change will result in __alloc_tag_ref_set() and > alloc_tag_ref_set() missing the following important check Correct, and therefore this patch is a good fix. > from alloc_tag_sub_check(): > > WARN_ONCE(ref && ref->ct, > "alloc_tag was not cleared (got tag for %s:%u)\n", > ref->ct->filename, ref->ct->lineno); > > I think the change below would fix this issue without the above > mentioned side-effect: I had this in my mind too when I first saw this patch earlier today. > -static inline void __alloc_tag_ref_set(union codetag_ref *ref, struct > alloc_tag *tag) > +static inline bool __alloc_tag_ref_set(union codetag_ref *ref, struct > alloc_tag *tag) > { > alloc_tag_add_check(ref, tag); > if (!ref || !tag) > - return; > + return false; > > ref->ct =3D &tag->ct; > + return true; > } > > -static inline void alloc_tag_ref_set(union codetag_ref *ref, struct > alloc_tag *tag) > +static inline bool alloc_tag_ref_set(union codetag_ref *ref, struct > alloc_tag *tag) > { > - __alloc_tag_ref_set(ref, tag); > + if (unlikely(!__alloc_tag_ref_set(ref, tag))) > + return false; > + > /* > * We need in increment the call counter every time we have a new > * allocation or when we split a large allocation into smaller on= es. > * Each new reference for every sub-allocation needs to increment= call > * counter because when we free each part the counter will be > decremented. > */ > this_cpu_inc(tag->counters->calls); > + return true; > } > > static inline void alloc_tag_add(union codetag_ref *ref, struct > alloc_tag *tag, size_t bytes) > { > - alloc_tag_ref_set(ref, tag); > - this_cpu_add(tag->counters->bytes, bytes); > + if (likely(alloc_tag_ref_set(ref, tag))) > + this_cpu_add(tag->counters->bytes, bytes); > } > > Could you please confirm this fix? After that, please feel free to add Acked-by: Yu Zhao (Of course with Suggested-by: Suren). > Thanks, > Suren. > > > ref->ct =3D &tag->ct; > > } > > > > @@ -158,6 +154,10 @@ static inline void alloc_tag_ref_set(union codetag= _ref *ref, struct alloc_tag *t > > > > static inline void alloc_tag_add(union codetag_ref *ref, struct alloc_= tag *tag, size_t bytes) > > { > > + alloc_tag_add_check(ref, tag); > > + if (!ref || !tag) > > + return; > > + > > alloc_tag_ref_set(ref, tag); > > this_cpu_add(tag->counters->bytes, bytes); > > } > > -- > > 2.25.1 > >