linux-kernel - Re: [PATCH v4 3/8] 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 for Android: free password hash cracker in your pocket

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

Message-ID: <4ylx7fxgwozm4yaavltlq6disd54acm3iko6tte2rxhufgq6rj@scwldtfbelg6>
Date: Wed, 16 Jul 2025 14:13:57 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: 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>, 
	Ard Biesheuvel <ardb@...nel.org>, Johan Hovold <johan@...nel.org>, 
	Steev Klimaszewski <steev@...i.org>, linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-efi@...r.kernel.org, 
	Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Subject: Re: [PATCH v4 3/8] firmware: qcom: uefisecapp: add support for R/O
 UEFI vars

On Wed, Jun 25, 2025 at 01:53:22AM +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.
> 

In order to implement UEFI variable storage on any device where the OS
owns the one storage device requires some form of bridge service running
in the OS.

We should expect that such devices will exist in the future as well (due
to BOM cost) and as such a solution for this will have to present itself
in a upstream compliant fashion.

There's a lot of infrastructure being introduced here to convey a single
boolean flag which I'm hoping to be dead code sooner rather than later.



Now there's some differences in your wording between the patches and the
responses. In some places you're talking about the C630 crashing, in
others you describe it as EFI variable writes aren't persistent. That
makes me wonder if we have two problems, or what you're refering to here
is just the same problem we see on all UFS-based systems (Qualcomm and
others).

Regards,
Bjorn

> Reviewed-by: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
> 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,
> +};
> +
>  static const struct efivar_operations qcom_efivar_ops = {
>  	.get_variable = qcuefi_get_variable,
>  	.set_variable = qcuefi_set_variable,
> @@ -804,7 +810,9 @@ static const struct efivar_operations qcom_efivar_ops = {
>  static int qcom_uefisecapp_probe(struct auxiliary_device *aux_dev,
>  				 const struct auxiliary_device_id *aux_dev_id)
>  {
> +	unsigned long *quirks = aux_dev->dev.platform_data;
>  	struct qcom_tzmem_pool_config pool_config;
> +	const struct efivar_operations *ops;
>  	struct qcuefi_client *qcuefi;
>  	int status;
>  
> @@ -829,7 +837,15 @@ static int qcom_uefisecapp_probe(struct auxiliary_device *aux_dev,
>  	if (status)
>  		return status;
>  
> -	status = efivars_register(&qcuefi->efivars, &qcom_efivar_ops);
> +	if (quirks &&
> +	    *quirks & QCOM_QSEECOM_QUIRK_RO_UEFIVARS) {
> +		dev_dbg(&aux_dev->dev, "R/O UEFI vars implementation\n");
> +		ops = &qcom_efivars_ro_ops;
> +	} else {
> +		ops = &qcom_efivar_ops;
> +	}
> +
> +	status = efivars_register(&qcuefi->efivars, ops);
>  	if (status)
>  		qcuefi_set_reference(NULL);
>  
> diff --git a/include/linux/firmware/qcom/qcom_qseecom.h b/include/linux/firmware/qcom/qcom_qseecom.h
> index 3387897bf36843cccd0bd933dd562390bf674b14..8d6d660e854fdb0fabbef10ab5ee6ff23ad79826 100644
> --- a/include/linux/firmware/qcom/qcom_qseecom.h
> +++ b/include/linux/firmware/qcom/qcom_qseecom.h
> @@ -51,4 +51,6 @@ static inline int qcom_qseecom_app_send(struct qseecom_client *client,
>  	return qcom_scm_qseecom_app_send(client->app_id, req, req_size, rsp, rsp_size);
>  }
>  
> +#define QCOM_QSEECOM_QUIRK_RO_UEFIVARS		BIT(0)
> +
>  #endif /* __QCOM_QSEECOM_H */
> 
> -- 
> 2.39.5
>