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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B7EA8C83F01 for ; Wed, 30 Aug 2023 15:48:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=WzHXdTK5wWmPJ7VRWOKTlSwZTLtrl1poE5WDxmtxqE0=; b=UG1QCLGjRKi0YH 5VyLX5OR0lWw7ttd0TJCPxUBeDfkvtS8L/BsNolgYm/ZTAnxfQyseDZgXzd9PF7PxSe7atT4FZISG keUDRERJ0Pf2aHHkyzrcQLefUqHscmIWEXFHh9LSZ1dYmJq0F55ju/27LrT8jmdU1DcOAbKdaQ8ci zyWoYJY+V04VHGDilifH/L509VA4AmW9nnKqh9nhNBT2TLkReK2XWY4JW0vN8EqCFYZiLfS7UT8uP wkHsBrGUTm5Ca3zV+fVRHwTWoXW8Iui1f7SfH2oMU7G5W1dUGzLybnD5K20yYdnrz4UnzK1jRa+Q4 oI65z/HTnAoavl2vu83w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qbNQT-00DnEk-2d; Wed, 30 Aug 2023 15:48:13 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qbNQQ-00DnE5-0b for linux-arm-kernel@lists.infradead.org; Wed, 30 Aug 2023 15:48:11 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1B3462F4; Wed, 30 Aug 2023 08:48:47 -0700 (PDT) Received: from bogus (unknown [10.57.36.157]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 472D53F64C; Wed, 30 Aug 2023 08:48:04 -0700 (PDT) Date: Wed, 30 Aug 2023 16:47:07 +0100 From: Sudeep Holla To: Radu Rendec Cc: Ricardo Neri , x86@kernel.org, Andreas Herrmann , Sudeep Holla , Catalin Marinas , Chen Yu , Len Brown , Pierre Gondois , Pu Wen , "Rafael J. Wysocki" , Srinivas Pandruvada , Will Deacon , Zhang Rui , stable@vger.kernel.org, Ricardo Neri , "Ravi V. Shankar" , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 1/3] cacheinfo: Allocate memory for memory if not done from the primary CPU Message-ID: <20230830154707.dyeihenolc5nwmi2@bogus> References: <20230805012421.7002-1-ricardo.neri-calderon@linux.intel.com> <20230805012421.7002-2-ricardo.neri-calderon@linux.intel.com> <20230830114918.be4mvwfogdqmsxk6@bogus> <23a1677c3df233c220df68ea429a2d0fec52e1d4.camel@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <23a1677c3df233c220df68ea429a2d0fec52e1d4.camel@redhat.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230830_084810_339506_85588CE9 X-CRM114-Status: GOOD ( 37.26 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Aug 30, 2023 at 08:13:09AM -0400, Radu Rendec wrote: > On Wed, 2023-08-30 at 12:49 +0100, Sudeep Holla wrote: > > On Fri, Aug 04, 2023 at 06:24:19PM -0700, Ricardo Neri wrote: > > > Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU= ") > > > adds functionality that architectures can use to optionally allocate = and > > > build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: A= dd > > > arch specific early level initializer") lets secondary CPUs correct (= and > > > reallocate memory) cacheinfo data if needed. > > > = > > > If the early build functionality is not used and cacheinfo does not n= eed > > > correction, memory for cacheinfo is never allocated. x86 does not use= the > > > early build functionality. Consequently, during the cacheinfo CPU hot= plug > > > callback, last_level_cache_is_valid() attempts to dereference a NULL > > > pointer: > > > = > > > =A0=A0=A0=A0 BUG: kernel NULL pointer dereference, address: 000000000= 0000100 > > > =A0=A0=A0=A0 #PF: supervisor read access in kernel mode > > > =A0=A0=A0=A0 #PF: error_code(0x0000) - not present page > > > =A0=A0=A0=A0 PGD 0 P4D 0 > > > =A0=A0=A0=A0 Oops: 0000 [#1] PREEPMT SMP NOPTI > > > =A0=A0=A0=A0 CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1 > > > =A0=A0=A0=A0 RIP: 0010: last_level_cache_is_valid+0x95/0xe0a > > > = > > > Allocate memory for cacheinfo during the cacheinfo CPU hotplug callba= ck if > > > not done earlier. > > > = > > > Cc: Andreas Herrmann > > > Cc: Catalin Marinas > > > Cc: Chen Yu > > > Cc: Len Brown > > > Cc: Radu Rendec > > > Cc: Pierre Gondois > > > Cc: Pu Wen > > > Cc: "Rafael J. Wysocki" > > > Cc: Sudeep Holla > > > Cc: Srinivas Pandruvada > > > Cc: Will Deacon > > > Cc: Zhang Rui > > > Cc: linux-arm-kernel@lists.infradead.org > > > Cc: stable@vger.kernel.org > > > Acked-by: Len Brown > > > Fixes: 6539cffa9495 ("cacheinfo: Add arch specific early level initia= lizer") > > = > > Not sure if we strictly need this(details below), but I am fine either = way. > > = > > > Signed-off-by: Ricardo Neri > > > --- > > > The motivation for commit 5944ce092b97 was to prevent a BUG splat in > > > PREEMPT_RT kernels during memory allocation. This splat is not observ= ed on > > > x86 because the memory allocation for cacheinfo happens in > > > detect_cache_attributes() from the cacheinfo CPU hotplug callback. > > > = > > > The dereference of a NULL pointer is not observed today because > > > cache_leaves(cpu) is zero until after init_cache_level() is called (a= lso > > > during the CPU hotplug callback). Patch2 will set it earlier and the = NULL- > > > pointer dereference will be observed. > > = > > Right, this is the information I have been asking in the previous versi= ons. > > This clarifies a lot. The trigger is in the patch 2/3 which is why it d= idn't > > make complete sense to me without it when you posted this patch indepen= dently. > > Thanks for posting it together and sorry for the delay(both reviewing t= his > > and in understanding the issue). > > = > > Given the trigger for NULL pointer dereference is in 2/3, I am not sure > > if it is really worth applying this to all the stable kernels with the > > commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU"). > > That is the reason why I asked to drop fixes tag if you agree with me. > > It is simple fix, so I am OK if you prefer to see that in the stable ke= rnels > > as well. > = > Thanks for reviewing, Sudeep. Since my previous commit 6539cffa9495 > ("cacheinfo: Add arch specific early level initializer") opens a door > for the NULL pointer dereference, I would sleep better at night if the > fix was included in the stable kernels :) But seriously, I am concerned > that with the fix applied in mainline and not in stable, something else > could be backported to the stable in the future, that could trigger the > NULL pointer dereference there. Ricardo's patch 2/3 is one way to > trigger it, but you never know what other patch lands in mainline in > the future that assumes it's safe to set the cache leaves earlier. > = Fair enough. I agree with you, so please retain the fixes tag as is. Please work with x86 maintainers to get it merged along with other patches. Let me know if you have other plans. -- = Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel