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 EDFEEC3601E for ; Sun, 13 Apr 2025 10:04:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=2Q0cjlliZOTAGxUbWhU7qhDv7ejKQFR1jbFLZJIPKK4=; b=mPVbq0RAfzEKxDLz38Dr0gAs6l XLe+N62dp3OlNo15f3yMT315dMd3EZRbtmEHEp3vhlV7Rc6EEZKAS6RUStAW9g+2lowKxwJlQdbgC 8mUheRQlUfhSArQX5YDxNWyxscbWWnnW+9RsfJCsN8KGou0IyMuedSXlwwBYNww8llAjHPsiKHCGv BZ1YEAWUTRVq3mzozr6X0sClPZuBYQygyzrakc3fAjkLivQTlnqSsb59nsxlg7TCT6NbhwaEoL8DD UbDMjsstok4YfSITRyyL3j6hNNy5tHo+a/tJl7BR9eJ53N9zb4mebytbEDPLdZ3zHyiFZ7P/MsKcy Mh+s1CJQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1u3uCR-0000000H45w-28Fk; Sun, 13 Apr 2025 10:04:27 +0000 Received: from nyc.source.kernel.org ([2604:1380:45d1:ec00::3]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1u3uAb-0000000H40S-0t39 for linux-arm-kernel@lists.infradead.org; Sun, 13 Apr 2025 10:02:34 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 61863A414A8; Sun, 13 Apr 2025 09:57:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 76DFCC4CEDD; Sun, 13 Apr 2025 10:02:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1744538551; bh=GLcXtzkyEcqkVe79D0YOof25hce64lkHGEFRIYwzgqs=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=AH2Oizqp3xiY82mebN3ewYsQ2Ma+gdSQ9TXAaP9GiHtkiOlYYoe9ZXrSVIeCeOIuY yHhkIlbFUMnm57QONmK7hKwE8pKY53OGhaF6BI73DgJGXESalh4FN7bPFP6yOaqa1S tESWCq4wrdcWoptf/w4phsX04z6o0oWmdHzdnw5PS6TM0W/SjwsQOJp8HsO/AvunnM 2Jhd8afBcnSBsQs+sPTYoLsH9m8zllRJ9wZb2USYPfjHCa4P3kfM0b97VB+QBGlKJy HIEbr+N+RQ114exZRon0Hxon0YWEzak+VVGP/B6TZJRc2G6oAwkevNdhIYWwDrYfjd TnItTsDcyzCAQ== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1u3uAW-004w7g-UD; Sun, 13 Apr 2025 11:02:29 +0100 Date: Sun, 13 Apr 2025 11:02:27 +0100 Message-ID: <86bjt0l6q4.wl-maz@kernel.org> From: Marc Zyngier To: Chenyuan Yang Cc: sven@svenpeter.dev, j@jannau.net, alyssa@rosenzweig.io, neal@gompa.dev, rafael@kernel.org, viresh.kumar@linaro.org, marcan@marcan.st, asahi@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] cpufreq: apple-soc: Fix possible null pointer dereference In-Reply-To: <20250412160518.1824538-1-chenyuan0y@gmail.com> References: <20250412160518.1824538-1-chenyuan0y@gmail.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/29.4 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: chenyuan0y@gmail.com, sven@svenpeter.dev, j@jannau.net, alyssa@rosenzweig.io, neal@gompa.dev, rafael@kernel.org, viresh.kumar@linaro.org, marcan@marcan.st, asahi@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250413_030233_378204_3895DFE6 X-CRM114-Status: GOOD ( 28.35 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sat, 12 Apr 2025 17:05:18 +0100, Chenyuan Yang wrote: > > Check if policy is NULL before dereferencing it. > > This is similar to the commit cf7de25878a1 > ("cppc_cpufreq: Fix possible null pointer dereference"). > No, it's not similar. The patch you refer to actually introduces bugs by returning -ENODEV in functions that have an unsigned return type. > This is found by our static analysis tool KNighter. I'm surprised that your tool hasn't found the above, because it should be a pretty easy thing to do. Irrespective of this, it would be good to describe under which circumstances this can occur, because I can't see *how* this can trigger. The policy is directly provided by the core code and provide its association with a cpu, and is never NULL at the point of init. And if it can trigger, why only fix this one particular case? Dereferences of policy are all over the map, and would be just as wrong. > > Signed-off-by: Chenyuan Yang > Fixes: 6286bbb40576 ("cpufreq: apple-soc: Add new driver to control Apple SoC CPU P-states") > --- > drivers/cpufreq/apple-soc-cpufreq.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/apple-soc-cpufreq.c b/drivers/cpufreq/apple-soc-cpufreq.c > index 4994c86feb57..3de9bb2b0f22 100644 > --- a/drivers/cpufreq/apple-soc-cpufreq.c > +++ b/drivers/cpufreq/apple-soc-cpufreq.c > @@ -135,10 +135,14 @@ static const struct of_device_id apple_soc_cpufreq_of_match[] __maybe_unused = { > static unsigned int apple_soc_cpufreq_get_rate(unsigned int cpu) > { > struct cpufreq_policy *policy = cpufreq_cpu_get_raw(cpu); > - struct apple_cpu_priv *priv = policy->driver_data; > + struct apple_cpu_priv *priv; > struct cpufreq_frequency_table *p; > unsigned int pstate; > > + if (!policy) > + return 0; > + priv = policy->driver_data; > + > if (priv->info->cur_pstate_mask) { > u32 reg = readl_relaxed(priv->reg_base + APPLE_DVFS_STATUS); > So while this is not wrong, I don't think this serves any real purpose. Thanks, M. -- Without deviation from the norm, progress is not possible.