linux-kernel - Re: [PATCH v2 2/4] firmware: qcom: uefisecapp: add support for R/O UEFI vars

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: <diarijcqernpm4v5s6u22jep3gzdrzy7o4dtw5wzmlec75og6y@wlbyjbtvnv3s>
Date: Tue, 24 Jun 2025 04:13:34 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Johan Hovold <johan@...nel.org>
Cc: Bjorn Andersson <andersson@...nel.org>,
        Maximilian Luz <luzmaximilian@...il.com>,
        Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 2/4] firmware: qcom: uefisecapp: add support for R/O
 UEFI vars

On Mon, Jun 23, 2025 at 04:50:27PM +0200, Johan Hovold wrote:
> On Mon, Jun 23, 2025 at 04:45:30PM +0200, Johan Hovold wrote:
> > On Sat, Jun 21, 2025 at 10:56:11PM +0300, Dmitry Baryshkov wrote:
> > > For some platforms (e.g. Lenovo Yoga C630) we don't yet know a way to
> > > update variables in the permanent storage. However being able to read
> > > the vars is still useful as it allows us to get e.g. RTC offset.
> > > 
> > > Add a quirk for QSEECOM specifying that UEFI variables for this platform
> > > should be registered in read-only mode.
> > > 
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
> > > ---
> > >  drivers/firmware/qcom/qcom_qseecom_uefisecapp.c | 18 +++++++++++++++++-
> > >  include/linux/firmware/qcom/qcom_qseecom.h      |  2 ++
> > >  2 files changed, 19 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c b/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
> > > index 98a463e9774bf04f2deb0f7fa1318bd0d2edfa49..05f700dcb8cf3189f640237ff0e045564abb8264 100644
> > > --- a/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
> > > +++ b/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
> > > @@ -792,6 +792,12 @@ static efi_status_t qcuefi_query_variable_info(u32 attr, u64 *storage_space, u64
> > >  	return status;
> > >  }
> > >  
> > > +static const struct efivar_operations qcom_efivars_ro_ops = {
> > > +	.get_variable = qcuefi_get_variable,
> > > +	.get_next_variable = qcuefi_get_next_variable,
> > > +	.query_variable_info = qcuefi_query_variable_info,
> > > +};
> > 
> > It looks like the efivars implementation does not support read-only
> > efivars and this will lead to NULL pointer dereferences whenever you try
> > to write a variable.
> 
> Ok, efivarfs seems to support it, but you'd crash when setting a
> variable from the kernel (e.g. from the RTC driver).

Ack, I'll fix it.

> 
> > Also not sure how useful it is to only be able to read variables,
> > including for the RTC where you'll end up with an RTC that's always
> > slightly off due to drift (even if you can set it when booting into
> > Windows or possibly from the UEFI setup).
> > 
> > Don't you have any SDAM blocks in the PMICs that you can use instead for
> > a proper functioning RTC on these machines?

I'd rather not poke into an SDAM, especially since we don't have docs
which SDAM blocks are used and which are not.

I think the slightly drifted RTC is still much better than ending up
with an RTC value which is significantly off, because it was set via the
file modification time.

Anyway, let me pick up some more patches in the next revision, maybe it
would be more obvious why I'd like to get R/O support.

-- 
With best wishes
Dmitry