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: <zdcfrk4oxu4eq5cjzr67hunpznzv366kalih7z4htbzaelbafq@ok7rux3nqr3d>
Date: Mon, 7 Oct 2024 16:45:01 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Kuldeep Singh <quic_kuldsing@...cinc.com>
Cc: Konrad Dybcio <konradybcio@...nel.org>, 
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>, linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Qingqing Zhou <quic_qqzhou@...cinc.com>
Subject: Re: [PATCH 1/2] firmware: qcom: scm: Return -EOPNOTSUPP for
 unsupported SHM bridge enabling

On Tue, Oct 08, 2024 at 02:10:02AM GMT, Kuldeep Singh wrote:
> On 10/7/2024 7:10 AM, Bjorn Andersson wrote:
> > On Sat, Oct 05, 2024 at 07:31:49PM GMT, Kuldeep Singh wrote:
[..]
> >> +
> >>  #define QSEECOM_MAX_APP_NAME_SIZE		64
> >>  
> >>  /* Each bit configures cold/warm boot address for one of the 4 CPUs */
> >> @@ -1361,6 +1365,8 @@ EXPORT_SYMBOL_GPL(qcom_scm_lmh_dcvsh_available);
> >>  
> >>  int qcom_scm_shm_bridge_enable(void)
> >>  {
> >> +	int ret;
> >> +
> >>  	struct qcom_scm_desc desc = {
> >>  		.svc = QCOM_SCM_SVC_MP,
> >>  		.cmd = QCOM_SCM_MP_SHM_BRIDGE_ENABLE,
> >> @@ -1373,7 +1379,11 @@ int qcom_scm_shm_bridge_enable(void)
> >>  					  QCOM_SCM_MP_SHM_BRIDGE_ENABLE))
> >>  		return -EOPNOTSUPP;
> >>  
> >> -	return qcom_scm_call(__scm->dev, &desc, &res) ?: res.result[0];
> >> +	ret = qcom_scm_call(__scm->dev, &desc, &res);
> >> +	if (!ret && res.result[0] == SHMBRIDGE_RESULT_NOTSUPP)
> >> +		return -EOPNOTSUPP;
> >> +
> >> +	return ret ?: res.result[0];
> > 
> > I'd prefer, with the additional check, that you'd structure it like this:
> > 
> > 	if (ret)
> > 		return ret;
> > 
> > 	if (res.result[0] == SHMBRIDGE_RESULT_NOTSUPP)
> > 		return -EOPNOTSUPP;
> > 
> > 	return res.result[0];
> 
> Sure, above looks more cleaner. Will update in next rev.
> 

Thanks!

> > 
> > That way we deal with SCM-call errors first, otherwise we inspect and
> > act on the returned data.
> > 
> > That said, the return value of this function, if non-zero, will trickle
> > back to and be returned from qcom_scm_probe(), where Linux expects to
> > see a valid error code. Are there any other result[0] values we should
> > handle, which would allow us to end this function with "return 0"?
> 
> As qcom_scm_shm_bridge_enable() is an shm enablement call, need to handle
> supported(or unsupported) scenario appropriately and other errors can be
> propagated to qcom_tzmem/qcom_scm_probe.
> 
> Please note, other return values(related to access control) from QTEE are
> failures and do not require conversion to Linux error codes.
> 

I'm not familiar with the value space of such errors, but any value
other than -EOPNOTSUPP and 0 returned here will propagate back and be
the value returned to the driver core.

It seems reasonable to ensure that the return value space makes sense to
Linux, just in case something up the stack decides to act upon the value
we return.

Regards,
Bjorn

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ