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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 29 Sep 2022 11:39:41 +0200
From:   Stephan Gerhold <stephan@...hold.net>
To:     Sireesh Kodali <sireeshkodali1@...il.com>
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 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...

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
>