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 5AB3EC4167B for ; Thu, 30 Nov 2023 12:37:18 +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=2SJAapHvkjexA6WqFV1+QZbZRVLOQMMKr952Ac1B8FU=; b=G/maHg66MeGl7S UTMYwjmaag9Rw2VHmJIp+6PMBviAKTvtmz9Ty/lThIrlUPzF2MYdPpZh25F3al3R3kU0Xr/Jwvi1W S/va+jG60SE7He6D9RnoF8pqZ04Fsn6y1jcImneJu+rzSe3DbjxSwxacQBGgfDtIHvyGQIa2SbHDi TUrtRDUrJgG88LlICekNLv+MKRLUeYyopI0FOtRaou34M4v0I0HOaAzQob012SASaAQXo2iow6hrZ Ifw+CRVd8fKzUZ2hSSOs/zQSLuyRySJt65iPgKq0Zr+ho0DPGl0cuss87B2Namfun1sRB9HzZai9H Uki6lNDqHZwKK1YLbgLw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r8gHe-00AsYj-2F; Thu, 30 Nov 2023 12:36:46 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r8gHc-00AsWl-00 for linux-arm-kernel@lists.infradead.org; Thu, 30 Nov 2023 12:36:45 +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 6B0CD1042; Thu, 30 Nov 2023 04:37:27 -0800 (PST) Received: from pluto (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6AA393F5A1; Thu, 30 Nov 2023 04:36:39 -0800 (PST) Date: Thu, 30 Nov 2023 12:36:36 +0000 From: Cristian Marussi To: Sibi Sankar Cc: sudeep.holla@arm.com, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, quic_mdtipton@quicinc.com, linux-arm-kernel@lists.infradead.org, quic_asartor@quicinc.com, quic_lingutla@quicinc.com Subject: Re: [PATCH 1/3] firmware: arm_scmi: Fix null pointer dereference during fastchannel init Message-ID: References: <20231129065748.19871-1-quic_sibis@quicinc.com> <20231129065748.19871-2-quic_sibis@quicinc.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20231129065748.19871-2-quic_sibis@quicinc.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231130_043644_301738_BEE238B7 X-CRM114-Status: GOOD ( 18.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="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 Wed, Nov 29, 2023 at 12:27:46PM +0530, Sibi Sankar wrote: > The scmi_perf_domain_lookup requires the protocol handle to have the > private data set, which is yet to happen during the fastchannel init > scenario. This results in a null pointer dereference. Fix this by using > the pre-populated perf_dom_info to pass on the required information > instead. > > Call trace: > scmi_perf_protocol_init+0x434/0x678 > scmi_get_protocol_instance+0x168/0x29c > scmi_devres_protocol_instance_get+0x50/0xa0 > scmi_devm_protocol_get+0x20/0x50 > scmi_cpufreq_probe+0x34/0xd4 > scmi_dev_probe+0x28/0x3c > really_probe+0x148/0x2ac > __driver_probe_device+0x78/0x12c > driver_probe_device+0x40/0x160 > __device_attach_driver+0xb8/0x134 > bus_for_each_drv+0x80/0xdc > __device_attach+0xa8/0x1b0 > device_initial_probe+0x14/0x20 > bus_probe_device+0xa8/0xac > device_add+0x5cc/0x778 > device_register+0x20/0x30 > __scmi_device_create.part.0+0xec/0x1cc > scmi_device_create+0x180/0x1c4 > scmi_create_protocol_devices+0x4c/0xb0 > scmi_probe+0x660/0x738 > platform_probe+0x68/0xdc > really_probe+0x148/0x2ac > __driver_probe_device+0x78/0x12c > driver_probe_device+0x40/0x160 > __device_attach_driver+0xb8/0x134 > bus_for_each_drv+0x80/0xdc > __device_attach+0xa8/0x1b0 > device_initial_probe+0x14/0x20 > bus_probe_device+0xa8/0xac > deferred_probe_work_func+0x88/0xc0 > process_one_work+0x13c/0x264 > worker_thread+0x32c/0x438 > kthread+0x118/0x11c > ret_from_fork+0x10/0x20 > > Fixes: 619bc6e034f3 ("firmware: arm_scmi: Populate fastchannel info only if set operations are allowed") > Signed-off-by: Sibi Sankar > --- Yes indeed, I made this same error in the past and then I missed it when reviewing the offending patch :< ... Reviewed-by: Cristian Marussi Thanks, Cristian > drivers/firmware/arm_scmi/perf.c | 19 +++++++------------ > 1 file changed, 7 insertions(+), 12 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > index d1323c5d9c27..a648521e04a3 100644 > --- a/drivers/firmware/arm_scmi/perf.c > +++ b/drivers/firmware/arm_scmi/perf.c > @@ -759,40 +759,35 @@ static int scmi_perf_level_limits_notify(const struct scmi_protocol_handle *ph, > } > > static void scmi_perf_domain_init_fc(const struct scmi_protocol_handle *ph, > - u32 domain, struct scmi_fc_info **p_fc) > + struct perf_dom_info *dom) > { > struct scmi_fc_info *fc; > - struct perf_dom_info *dom; > - > - dom = scmi_perf_domain_lookup(ph, domain); > - if (IS_ERR(dom)) > - return; > > fc = devm_kcalloc(ph->dev, PERF_FC_MAX, sizeof(*fc), GFP_KERNEL); > if (!fc) > return; > > ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, > - PERF_LEVEL_GET, 4, domain, > + PERF_LEVEL_GET, 4, dom->id, > &fc[PERF_FC_LEVEL].get_addr, NULL); > > ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, > - PERF_LIMITS_GET, 8, domain, > + PERF_LIMITS_GET, 8, dom->id, > &fc[PERF_FC_LIMIT].get_addr, NULL); > > if (dom->info.set_perf) > ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, > - PERF_LEVEL_SET, 4, domain, > + PERF_LEVEL_SET, 4, dom->id, > &fc[PERF_FC_LEVEL].set_addr, > &fc[PERF_FC_LEVEL].set_db); > > if (dom->set_limits) > ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, > - PERF_LIMITS_SET, 8, domain, > + PERF_LIMITS_SET, 8, dom->id, > &fc[PERF_FC_LIMIT].set_addr, > &fc[PERF_FC_LIMIT].set_db); > > - *p_fc = fc; > + dom->fc_info = fc; > } > > static int scmi_dvfs_device_opps_add(const struct scmi_protocol_handle *ph, > @@ -1102,7 +1097,7 @@ static int scmi_perf_protocol_init(const struct scmi_protocol_handle *ph) > scmi_perf_describe_levels_get(ph, dom, version); > > if (dom->perf_fastchannels) > - scmi_perf_domain_init_fc(ph, dom->id, &dom->fc_info); > + scmi_perf_domain_init_fc(ph, dom); > } > > ret = devm_add_action_or_reset(ph->dev, scmi_perf_xa_destroy, pinfo); > -- > 2.17.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel