linux-kernel - Re: [PATCH v5 1/5] remoteproc: qcom: qcom

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 30 Sep 2022 09:58:36 +0530
From:   "Sireesh Kodali" <sireeshkodali1@...il.com>
To:     "Stephan Gerhold" <stephan@...hold.net>
Cc:     <devicetree@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-remoteproc@...r.kernel.org>,
        <~postmarketos/upstreaming@...ts.sr.ht>,
        <linux-kernel@...r.kernel.org>, <andersson@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>,
        "Vladimir Lypak" <vladimir.lypak@...il.com>,
        "Andy Gross" <agross@...nel.org>,
        "Bjorn Andersson" <bjorn.andersson@...aro.org>,
        "Konrad Dybcio" <konrad.dybcio@...ainline.org>,
        "Mathieu Poirier" <mathieu.poirier@...aro.org>
Subject: Re: [PATCH v5 1/5] remoteproc: qcom: qcom_wcnss: Add support for
 pronto-v3

On Thu Sep 29, 2022 at 3:09 PM IST, Stephan Gerhold wrote:
> On Thu, Sep 29, 2022 at 10:32:05AM +0530, Sireesh Kodali wrote:
> > From: Vladimir Lypak <vladimir.lypak@...il.com>
> > 
> > Pronto-v3 is similar to pronto-v2. It requires two power domains, and it
> > requires the xo clock. It is used on the MSM8953 platform.
> > 
> > Signed-off-by: Vladimir Lypak <vladimir.lypak@...il.com>
> > Signed-off-by: Sireesh Kodali <sireeshkodali1@...il.com>
> > ---
> >  drivers/remoteproc/qcom_wcnss.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
> > index 68f37296b151..ff18bfae5eb6 100644
> > --- a/drivers/remoteproc/qcom_wcnss.c
> > +++ b/drivers/remoteproc/qcom_wcnss.c
> > @@ -141,6 +141,18 @@ static const struct wcnss_data pronto_v2_data = {
> >  	.num_vregs = 1,
> >  };
> >  
> > +static const struct wcnss_data pronto_v3_data = {
> > +	.pmu_offset = 0x1004,
> > +	.spare_offset = 0x1088,
> > +
> > +	.pd_names = { "mx", "cx" },
> > +	.vregs = (struct wcnss_vreg_info[]) {
> > +		{ "vddpx", 1800000, 1800000, 0 },
> > +	},
> > +	.num_pd_vregs = 2,
>
> Can you try dropping this line? num_pd_vregs = 2 means:
> "If power domains are specified in the device tree, skip the first two
>  entries in 'vregs'." For pronto-v1/v2 it would skip the "vddmx" and
> "vddcx" entry, since those are already covered by the power domains.
>
> However, since pronto-v3 should always have power domains in DT and
> "vregs" has just a single entry this means that it would always access
> the array out of bounds at runtime and request some garbage regulator.
> I'm confused why this does not crash more spectacularly...

Indeed it should have crashed, thanks for catching this. Will be fixed
in v6
>
> Thanks,
> Stephan
>
> > +	.num_vregs = 1,
> > +};
> > +
> >  static int wcnss_load(struct rproc *rproc, const struct firmware *fw)
> >  {
> >  	struct qcom_wcnss *wcnss = (struct qcom_wcnss *)rproc->priv;
> > @@ -675,6 +687,7 @@ static const struct of_device_id wcnss_of_match[] = {
> >  	{ .compatible = "qcom,riva-pil", &riva_data },
> >  	{ .compatible = "qcom,pronto-v1-pil", &pronto_v1_data },
> >  	{ .compatible = "qcom,pronto-v2-pil", &pronto_v2_data },
> > +	{ .compatible = "qcom,pronto-v3-pil", &pronto_v3_data },
> >  	{ },
> >  };
> >  MODULE_DEVICE_TABLE(of, wcnss_of_match);
> > -- 
> > 2.37.3
> >