[PATCH] MdeModulePkg/Variable: SCT run AuthVar_conf is failed

[PATCH] MdeModulePkg/Variable: SCT run AuthVar_conf is failed

[xueshengfeng]

From: Lijun10x <junx1.li@intel.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3969

Attr are EFI_VARIABLE_NON_VOLATILE|VARIABLE_AUTHENTICATED_WRITE_ACCESS,
will return EFI_INVALID_PARAMETER.
Added one case, only when one attribute is EFI_VARIABLE_NON_VOLATILE
will EFI_INVALID_PARAMETER be returned.
If attr are EFI_VARIABLE_NON_VOLATILE|VARIABLE_AUTHENTICATED_WRITE_ACCESS
will return EFI_UNSUPPORTED.
In the UEFI2.7 spec, there is a description as below:
EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should not be
used. Platforms should return EFI_UNSUPPORTED if a caller to
SetVariable() specifies this attribute.

Signed-off-by: JunX1 Li <junx1.li@intel.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: G Edhaya Chandran <edhaya.chandran@arm.com>
Reviewed-by: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Reviewed-by: Sunny Wang <sunny.wang@arm.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 6c1a3440ac..14c176887a 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -2676,7 +2676,11 @@ VariableServiceSetVariable (
     //
     // Only EFI_VARIABLE_NON_VOLATILE attribute is invalid
     //
-    return EFI_INVALID_PARAMETER;
+    if ((Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) != 0) {
+      return EFI_UNSUPPORTED;
+    } else {
+      return EFI_INVALID_PARAMETER;
+    }
   } else if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
     if (!mVariableModuleGlobal->VariableGlobal.AuthSupport) {
       //
-- 
2.26.2.windows.1

回复: [PATCH] MdeModulePkg/Variable: SCT run AuthVar_conf is failed

[gaoliming]

Jun:
  I agree this change. You can add my reviewed-by. 

Thanks
Liming
> -----邮件原件-----
> 发件人: xueshengfeng <xueshengfeng@byosoft.com.cn>
> 发送时间: 2022年7月4日 18:00
> 收件人: devel@edk2.groups.io
> 抄送: gaoliming@byosoft.com.cn; edhaya.chandran@arm.com;
> samer.el-haj-mahmoud@arm.com; sunny.wang@arm.com; Lijun10x
> <junx1.li@intel.com>
> 主题: [PATCH] MdeModulePkg/Variable: SCT run AuthVar_conf is failed
> 
> From: Lijun10x <junx1.li@intel.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3969
> 
> Attr are
> EFI_VARIABLE_NON_VOLATILE|VARIABLE_AUTHENTICATED_WRITE_ACCESS
> ,
> will return EFI_INVALID_PARAMETER.
> Added one case, only when one attribute is EFI_VARIABLE_NON_VOLATILE
> will EFI_INVALID_PARAMETER be returned.
> If attr are
> EFI_VARIABLE_NON_VOLATILE|VARIABLE_AUTHENTICATED_WRITE_ACCESS
> will return EFI_UNSUPPORTED.
> In the UEFI2.7 spec, there is a description as below:
> EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should
> not be
> used. Platforms should return EFI_UNSUPPORTED if a caller to
> SetVariable() specifies this attribute.
> 
> Signed-off-by: JunX1 Li <junx1.li@intel.com>
> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> Reviewed-by: G Edhaya Chandran <edhaya.chandran@arm.com>
> Reviewed-by: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Reviewed-by: Sunny Wang <sunny.wang@arm.com>
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index 6c1a3440ac..14c176887a 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -2676,7 +2676,11 @@ VariableServiceSetVariable (
>      //
>      // Only EFI_VARIABLE_NON_VOLATILE attribute is invalid
>      //
> -    return EFI_INVALID_PARAMETER;
> +    if ((Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) != 0)
> {
> +      return EFI_UNSUPPORTED;
> +    } else {
> +      return EFI_INVALID_PARAMETER;
> +    }
>    } else if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
>      if (!mVariableModuleGlobal->VariableGlobal.AuthSupport) {
>        //
> --
> 2.26.2.windows.1

Re: [PATCH] MdeModulePkg/Variable: SCT run AuthVar_conf is failed

[Sunny Wang]

Ah.. good catch. I was not aware that EFI_VARIABLE_ATTRIBUTES_MASK doesn't include EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS bit.
Thanks for fixing the issue, JunX1 Li and xueshengfeng.
Moreover, I assumed that you haven't got Edhaya's and Samer's review, right? If so, you should use "Cc:" instead of "Reviewed-by:" in your commit message.

The code change looks good to me.
Reviewed-by: Sunny Wang <sunny.wang@arm.com>

-----Original Message-----
From: xueshengfeng <xueshengfeng@byosoft.com.cn>
Sent: 04 July 2022 11:00
To: devel@edk2.groups.io
Cc: gaoliming@byosoft.com.cn; G Edhaya Chandran <Edhaya.Chandran@arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <Sunny.Wang@arm.com>; Lijun10x <junx1.li@intel.com>
Subject: [PATCH] MdeModulePkg/Variable: SCT run AuthVar_conf is failed

From: Lijun10x <junx1.li@intel.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3969

Attr are EFI_VARIABLE_NON_VOLATILE|VARIABLE_AUTHENTICATED_WRITE_ACCESS,
will return EFI_INVALID_PARAMETER.
Added one case, only when one attribute is EFI_VARIABLE_NON_VOLATILE
will EFI_INVALID_PARAMETER be returned.
If attr are EFI_VARIABLE_NON_VOLATILE|VARIABLE_AUTHENTICATED_WRITE_ACCESS
will return EFI_UNSUPPORTED.
In the UEFI2.7 spec, there is a description as below:
EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should not be
used. Platforms should return EFI_UNSUPPORTED if a caller to
SetVariable() specifies this attribute.

Signed-off-by: JunX1 Li <junx1.li@intel.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: G Edhaya Chandran <edhaya.chandran@arm.com>
Reviewed-by: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Reviewed-by: Sunny Wang <sunny.wang@arm.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 6c1a3440ac..14c176887a 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -2676,7 +2676,11 @@ VariableServiceSetVariable (
     //
     // Only EFI_VARIABLE_NON_VOLATILE attribute is invalid
     //
-    return EFI_INVALID_PARAMETER;
+    if ((Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) != 0) {
+      return EFI_UNSUPPORTED;
+    } else {
+      return EFI_INVALID_PARAMETER;
+    }
   } else if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
     if (!mVariableModuleGlobal->VariableGlobal.AuthSupport) {
       //
--
2.26.2.windows.1


IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.