linux-kernel - Re: [PATCH 2/3] soc: qcom: smem: better track SMEM uninitialized state

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 for Android: free password hash cracker in your pocket

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

Message-ID: <69023398.df0a0220.25fede.8d9c@mx.google.com>
Date: Wed, 29 Oct 2025 16:32:35 +0100
From: Christian Marangi <ansuelsmth@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Ilia Lin <ilia.lin@...nel.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konradybcio@...nel.org>,
	Raag Jadav <raag.jadav@...el.com>, Arnd Bergmann <arnd@...db.de>,
	linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] soc: qcom: smem: better track SMEM uninitialized
 state

On Wed, Oct 29, 2025 at 05:27:33PM +0200, Andy Shevchenko wrote:
> On Wed, Oct 29, 2025 at 02:33:20PM +0100, Christian Marangi wrote:
> > There is currently a problem where, in the specific case of SMEM not
> > initialized by SBL, any SMEM API wrongly returns PROBE_DEFER
> > communicating wrong info to any user of this API.
> > 
> > A better way to handle this would be to track the SMEM state and return
> > a different kind of error than PROBE_DEFER.
> > 
> > Rework the __smem handle to always init it to the error pointer
> > -EPROBE_DEFER following what is already done by the SMEM API.
> > If we detect that the SBL didn't initialized SMEM, set the __smem handle
> > to the error pointer -ENODEV.
> > Also rework the SMEM API to handle the __smem handle to be an error
> > pointer and return it appropriately.
> 
> ...
> 
> >  	if (le32_to_cpu(header->initialized) != 1 ||
> >  	    le32_to_cpu(header->reserved)) {
> >  		dev_err(&pdev->dev, "SMEM is not initialized by SBL\n");
> > +		__smem = ERR_PTR(-ENODEV);
> >  		return -EINVAL;
> >  	}
> 
> I find this a bit confusing. Why the error code returned to the upper layer is
> different to the stored one?
>

It's INVAL for probe. But for any user of SMEM it's NODEV as there isn't
an actual SMEM usable.

Totally ok to change the error condition in probe if maybe NODEV is
better suited. I assume there isn't a specific pattern of the correct
error condition in probe.

> ...
> 
> Also, the series of patches should include the cover letter to explain not only
> series background but additionally
> - how it should be applied
> - if it has dependencies
> - etc
> 

Didn't add one they are trivial patch but I can add it if needed... it's
pretty stable code so no dependency or branch target

> 
> 

-- 
	Ansuel