linux-kernel - Re: [PATCH] venus: firmware: Ignore secure call error on first resume

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]

Message-ID: <20200308220057.GA1098305@builder>
Date:   Sun, 8 Mar 2020 15:00:57 -0700
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Stanimir Varbanov <stanimir.varbanov@...aro.org>
Cc:     linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-media@...r.kernel.org, Andy Gross <agross@...nel.org>,
        Elliot Berman <eberman@...eaurora.org>
Subject: Re: [PATCH] venus: firmware: Ignore secure call error on first resume

On Wed 04 Mar 07:58 PST 2020, Stanimir Varbanov wrote:

> With the latest cleanup in qcom scm driver

I've seen similar issues in other drivers, but I think the culprit is
the following bug fix 'ff34f3cce278 ("firmware: qcom: scm: Ensure 'a0'
status code is treated as signed")', rather than the recent refactoring.

As this is a bug fix it's quite likely to show up in linux-stable as
well and regress older versions, so you should probably Cc stable as
well.

> the secure monitor
> call for setting the remote processor state returns EINVAL when
> it is called for the first time and after another scm call
> auth_and_reset. The error returned from scm call could be ignored
> because the state transition is already done in auth_and_reset.
> 

I'm not sure what the appropriate mechanism is here, but your workaround
seems reasonable.

Acked-by: Bjorn Andersson <bjorn.andersson@...aro.org>

Regards,
Bjorn

> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@...aro.org>
> ---
>  drivers/media/platform/qcom/venus/firmware.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> index de6812fb55f4..8801a6a7543d 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c
> @@ -44,8 +44,14 @@ static void venus_reset_cpu(struct venus_core *core)
>  
>  int venus_set_hw_state(struct venus_core *core, bool resume)
>  {
> -	if (core->use_tz)
> -		return qcom_scm_set_remote_state(resume, 0);
> +	int ret;
> +
> +	if (core->use_tz) {
> +		ret = qcom_scm_set_remote_state(resume, 0);
> +		if (resume && ret == -EINVAL)
> +			ret = 0;
> +		return ret;
> +	}
>  
>  	if (resume)
>  		venus_reset_cpu(core);
> -- 
> 2.17.1
>