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 F1475C4345F for ; Thu, 18 Apr 2024 15:56:08 +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=3vNiUwuIfHR4ZkiwW7N3m6QeENRqErl2RLaoEudUIbU=; b=3tyfrhnapRM9qc TRDHn4aT7izD/5eiCy9TmQkJFdczXMjl2Ro55ovAD4aQv2Tuhw791RtIL+vQf4ynceCneJuDbYdy2 rfY0CeFaFn0H67fg5lSZrnZHJhXt0We4+QXZ9DQXrSjfvsdGwviXq5Vg3ULPEWG79Z66fEzGD+ZVg JiZLzQs3Y9RUeTMcgdl4t/SHGd/FUnStrlB2dE2ynDe9Uf7BkGIWS1dN+CV0UmwavoCw5f8yyrFml CnN8dY2Qx6h5KPBnk+x00WogtOGw4AZbRz9Y4E9hZm/6j6d3BFdDseuB5iEWh76uSl/x0EeLhP42g 0FwP8S4CSsJ8dZMoTsZw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rxU7D-00000002tyL-088H; Thu, 18 Apr 2024 15:55:59 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rxU7A-00000002txW-1NEa for linux-arm-kernel@lists.infradead.org; Thu, 18 Apr 2024 15:55:57 +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 E935F2F; Thu, 18 Apr 2024 08:56:20 -0700 (PDT) Received: from arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C8A5D3F64C; Thu, 18 Apr 2024 08:55:49 -0700 (PDT) Date: Thu, 18 Apr 2024 17:55:43 +0200 From: Beata Michalska To: Sudeep Holla Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, ionela.voinescu@arm.com, vanshikonda@os.amperecomputing.com, will@kernel.org, catalin.marinas@arm.com, vincent.guittot@linaro.org, sumitg@nvidia.com, yang@os.amperecomputing.com, lihuisong@huawei.com, viresh.kumar@linaro.org Subject: Re: [PATCH v5 2/5] arm64: amu: Rule out potential use after free Message-ID: References: <20240417093848.1555462-1-beata.michalska@arm.com> <20240417093848.1555462-3-beata.michalska@arm.com> <20240418105052.zvkomz5yeayie4ph@bogus> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240418105052.zvkomz5yeayie4ph@bogus> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240418_085556_440663_039A8855 X-CRM114-Status: GOOD ( 29.87 ) 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="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Apr 18, 2024 at 11:50:52AM +0100, Sudeep Holla wrote: > On Wed, Apr 17, 2024 at 10:38:45AM +0100, Beata Michalska wrote: > > For the time being, the amu_fie_cpus cpumask is being exclusively used > > by the AMU-related internals of FIE support and is guaranteed to be > > valid on every access currently made. Still the mask is not being > > invalidated on one of the error handling code paths, which leaves > > a soft spot with potential risk of uaf for CPUMASK_OFFSTACK cases. > > To make things sound, set the cpumaks pointer explicitly to NULL upon > > failing to register the cpufreq notifier. > > Note that, due to the quirks of CPUMASK_OFFSTACK, this change needs to > > be wrapped with grim ifdefing (it would be better served by > > incorporating this into free_cpumask_var ...) > > > > Yes it doesn't look neat. > > > Signed-off-by: Beata Michalska > > --- > > arch/arm64/kernel/topology.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c > > index 1a2c72f3e7f8..3c814a278534 100644 > > --- a/arch/arm64/kernel/topology.c > > +++ b/arch/arm64/kernel/topology.c > > @@ -244,8 +244,12 @@ static int __init init_amu_fie(void) > > > > ret = cpufreq_register_notifier(&init_amu_fie_notifier, > > CPUFREQ_POLICY_NOTIFIER); > > - if (ret) > > + if (ret) { > > free_cpumask_var(amu_fie_cpus); > > +#ifdef CONFIG_CPUMASK_OFFSTACK > > + amu_fie_cpus = NULL; > > +#endif > > + } > > Instead of this #ifdeffery, I was wondering if we can actually do the > allocation in init_amu_fie_callback() the first time it gets called > checking if amu_fie_cpus is NULL. init_amu_fie_callback() must get called > only if the cpufreq_register_notifier() succeeds right ? > Delayed allocation ... I guess this will do the trick. > Also I don't see anyone calling amu_fie_setup(), so where do you think > the possible use after free could occur for amu_fie_cpus. Just thinking > out loud to check if I missed anything. > You haven't missed anything. Currently the uaf is purely theoretical as the code that relies on that mask will only be executed if we have succeeded to register the amu fie support: so far so good. This change is required for following patches, where that mask is being used to determine if given CPU has been setup with AMU counters, and as such there needs to be a safe way to validate it (at any time) by arch_freq_get_on_cpu and arch_cpu_idle_enter (patches 3/5 & 4/5): both will be called from outside the FIE. Without this change, if cpufreq_register_notifier fails in init_amu_fie, we will have amu_fie_cpus pointing to released memory. Hope that makes things more clear. --- BR Beata > -- > Regards, > Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel