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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <qy3d2wqqi7y2l6lyjrlsjf4dlzoigvawwze7ulapjujozi7lhq@7tluyf4j6nbu>
Date: Wed, 29 Oct 2025 11:11:30 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
	Ilia Lin <ilia.lin@...nel.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Viresh Kumar <viresh.kumar@...aro.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 04:32:35PM +0100, Christian Marangi wrote:
> 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.
> 

I'd say ENODEV represents the error better than EINVAL, so I don't have
any concerns with you changing the return value.

> > ...
> > 
> > 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
> 

Specifically, I should merge patch 1 and 2 through the qcom/soc tree,
and patch 3 can be merged completely independently through the cpufreq
tree.

Regards,
Bjorn

> > 
> > 
> 
> -- 
> 	Ansuel