[edk2-devel] [Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable Feature" Menu disappear issue.

Dong, Eric posted 3 patches 6 years, 9 months ago
[edk2-devel] [Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable Feature" Menu disappear issue.
Posted by Dong, Eric 6 years, 9 months ago
https://bugzilla.tianocore.org/show_bug.cgi?id=1782

After change behavior to send BlockSid command at EndOfDxe point,
check device ownership command will return un-authority error, it
finally caused opal driver can't show "Enable Feature" menu.

Update the code logic to send detect device ownership command
before send BlockSID command.

Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
---
 .../Tcg/Opal/OpalPassword/OpalDriver.c        | 11 +++++
 .../Tcg/Opal/OpalPassword/OpalDriver.h        |  1 +
 SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c   | 46 +++++++++++++++----
 SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h   | 15 ++++++
 4 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
index 009a97f66f..965205c0b2 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
@@ -458,6 +458,11 @@ SendBlockSidCommand (
           DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
           break;
         }
+
+        //
+        // Record BlockSID command has been sent.
+        //
+        Itr->OpalDisk.SentBlockSID = TRUE;
       }
 
       Itr = Itr->Next;
@@ -2204,6 +2209,12 @@ ProcessOpalRequest (
         ProcessOpalRequestEnableFeature (Dev, L"Enable Feature:");
       }
 
+      //
+      // Update Device ownership.
+      // Later BlockSID command may block the update.
+      //
+      OpalDiskUpdateOwnerShip (&Dev->OpalDisk);
+
       break;
     }
 
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
index a056e06106..beeabb1c0a 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
@@ -143,6 +143,7 @@ typedef struct {
   UINT8                                           Password[OPAL_MAX_PASSWORD_SIZE];
 
   UINT32                                          EstimateTimeCost;
+  BOOLEAN                                         SentBlockSID;           // Check whether BlockSid command has been sent.
 } OPAL_DISK;
 
 //
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
index d0f3eda1e8..f101ca1c20 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
@@ -1215,6 +1215,40 @@ OpalDiskInitialize (
   return OpalDiskUpdateStatus (&Dev->OpalDisk);
 }
 
+/**
+  Update the device ownship
+
+  @param OpalDisk                The Opal device.
+
+  @retval EFI_SUCESS             Get ownership success.
+  @retval EFI_ACCESS_DENIED      Has send BlockSID command, can't change ownership.
+  @retval EFI_INVALID_PARAMETER  Not get Msid info before get ownership info.
+
+**/
+EFI_STATUS
+OpalDiskUpdateOwnerShip (
+  OPAL_DISK        *OpalDisk
+  )
+{

+  OPAL_SESSION  Session;
+
+  if (OpalDisk->MsidLength == 0) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (OpalDisk->SentBlockSID) {
+    return EFI_ACCESS_DENIED;
+  }
+
+  ZeroMem(&Session, sizeof(Session));
+  Session.Sscp = OpalDisk->Sscp;
+  Session.MediaId = OpalDisk->MediaId;
+  Session.OpalBaseComId = OpalDisk->OpalBaseComId;
+
+  OpalDisk->Owner = OpalUtilDetermineOwnership(&Session, OpalDisk->Msid, OpalDisk->MsidLength);

+  return EFI_SUCCESS;
+}
+
 /**
   Update the device info.
 
@@ -1223,6 +1257,7 @@ OpalDiskInitialize (
   @retval EFI_SUCESS             Initialize the device success.
   @retval EFI_DEVICE_ERROR       Get info from device failed.
   @retval EFI_INVALID_PARAMETER  Not get Msid info before get ownership info.
+  @retval EFI_ACCESS_DENIED      Has send BlockSID command, can't change ownership.
 
 **/
 EFI_STATUS
@@ -1243,15 +1278,6 @@ OpalDiskUpdateStatus (
     return EFI_DEVICE_ERROR;
   }
 
-  if (OpalDisk->MsidLength == 0) {
-    return EFI_INVALID_PARAMETER;
-  } else {
-    //
-    // Base on the Msid info to get the ownership, so Msid info must get first.
-    //
-    OpalDisk->Owner = OpalUtilDetermineOwnership(&Session, OpalDisk->Msid, OpalDisk->MsidLength);
-  }
-
-  return EFI_SUCCESS;
+  return OpalDiskUpdateOwnerShip (OpalDisk);
 }
 
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
index d3e236e2fe..89c709df99 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
@@ -357,4 +357,19 @@ OpalDiskInitialize (
   IN OPAL_DRIVER_DEVICE          *Dev
   );
 
+/**
+  Update the device ownership
+
+  @param OpalDisk                The Opal device.
+
+  @retval EFI_SUCESS             Get ownership success.
+  @retval EFI_ACCESS_DENIED      Has send BlockSID command, can't change ownership.
+  @retval EFI_INVALID_PARAMETER  Not get Msid info before get ownership info.
+
+**/
+EFI_STATUS
+OpalDiskUpdateOwnerShip (
+  OPAL_DISK        *OpalDisk
+  );
+
 #endif // _HII_H_
-- 
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#40165): https://edk2.groups.io/g/devel/message/40165
Mute This Topic: https://groups.io/mt/31539608/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable Feature" Menu disappear issue.
Posted by Wu, Hao A 6 years, 9 months ago
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Dong,
> Eric
> Sent: Wednesday, May 08, 2019 11:02 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A
> Subject: [edk2-devel] [Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable
> Feature" Menu disappear issue.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
> 
> After change behavior to send BlockSid command at EndOfDxe point,
> check device ownership command will return un-authority error, it
> finally caused opal driver can't show "Enable Feature" menu.
> 
> Update the code logic to send detect device ownership command
> before send BlockSID command.

Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu

> 
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> ---
>  .../Tcg/Opal/OpalPassword/OpalDriver.c        | 11 +++++
>  .../Tcg/Opal/OpalPassword/OpalDriver.h        |  1 +
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c   | 46 +++++++++++++++----
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h   | 15 ++++++
>  4 files changed, 63 insertions(+), 10 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> index 009a97f66f..965205c0b2 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> @@ -458,6 +458,11 @@ SendBlockSidCommand (
>            DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
>            break;
>          }
> +
> +        //
> +        // Record BlockSID command has been sent.
> +        //
> +        Itr->OpalDisk.SentBlockSID = TRUE;
>        }
> 
>        Itr = Itr->Next;
> @@ -2204,6 +2209,12 @@ ProcessOpalRequest (
>          ProcessOpalRequestEnableFeature (Dev, L"Enable Feature:");
>        }
> 
> +      //
> +      // Update Device ownership.
> +      // Later BlockSID command may block the update.
> +      //
> +      OpalDiskUpdateOwnerShip (&Dev->OpalDisk);
> +
>        break;
>      }
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> index a056e06106..beeabb1c0a 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> @@ -143,6 +143,7 @@ typedef struct {
>    UINT8                                           Password[OPAL_MAX_PASSWORD_SIZE];
> 
>    UINT32                                          EstimateTimeCost;
> +  BOOLEAN                                         SentBlockSID;           // Check whether
> BlockSid command has been sent.
>  } OPAL_DISK;
> 
>  //
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> index d0f3eda1e8..f101ca1c20 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> @@ -1215,6 +1215,40 @@ OpalDiskInitialize (
>    return OpalDiskUpdateStatus (&Dev->OpalDisk);
>  }
> 
> +/**
> +  Update the device ownship
> +
> +  @param OpalDisk                The Opal device.
> +
> +  @retval EFI_SUCESS             Get ownership success.
> +  @retval EFI_ACCESS_DENIED      Has send BlockSID command, can't change
> ownership.
> +  @retval EFI_INVALID_PARAMETER  Not get Msid info before get ownership
> info.
> +
> +**/
> +EFI_STATUS
> +OpalDiskUpdateOwnerShip (
> +  OPAL_DISK        *OpalDisk
> +  )
> +{
> 
> +  OPAL_SESSION  Session;
> +
> +  if (OpalDisk->MsidLength == 0) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (OpalDisk->SentBlockSID) {
> +    return EFI_ACCESS_DENIED;
> +  }
> +
> +  ZeroMem(&Session, sizeof(Session));
> +  Session.Sscp = OpalDisk->Sscp;
> +  Session.MediaId = OpalDisk->MediaId;
> +  Session.OpalBaseComId = OpalDisk->OpalBaseComId;
> +
> +  OpalDisk->Owner = OpalUtilDetermineOwnership(&Session, OpalDisk->Msid,
> OpalDisk->MsidLength);
> 
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    Update the device info.
> 
> @@ -1223,6 +1257,7 @@ OpalDiskInitialize (
>    @retval EFI_SUCESS             Initialize the device success.
>    @retval EFI_DEVICE_ERROR       Get info from device failed.
>    @retval EFI_INVALID_PARAMETER  Not get Msid info before get ownership
> info.
> +  @retval EFI_ACCESS_DENIED      Has send BlockSID command, can't change
> ownership.
> 
>  **/
>  EFI_STATUS
> @@ -1243,15 +1278,6 @@ OpalDiskUpdateStatus (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if (OpalDisk->MsidLength == 0) {
> -    return EFI_INVALID_PARAMETER;
> -  } else {
> -    //
> -    // Base on the Msid info to get the ownership, so Msid info must get first.
> -    //
> -    OpalDisk->Owner = OpalUtilDetermineOwnership(&Session, OpalDisk-
> >Msid, OpalDisk->MsidLength);
> -  }
> -
> -  return EFI_SUCCESS;
> +  return OpalDiskUpdateOwnerShip (OpalDisk);
>  }
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> index d3e236e2fe..89c709df99 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> @@ -357,4 +357,19 @@ OpalDiskInitialize (
>    IN OPAL_DRIVER_DEVICE          *Dev
>    );
> 
> +/**
> +  Update the device ownership
> +
> +  @param OpalDisk                The Opal device.
> +
> +  @retval EFI_SUCESS             Get ownership success.
> +  @retval EFI_ACCESS_DENIED      Has send BlockSID command, can't change
> ownership.
> +  @retval EFI_INVALID_PARAMETER  Not get Msid info before get ownership
> info.
> +
> +**/
> +EFI_STATUS
> +OpalDiskUpdateOwnerShip (
> +  OPAL_DISK        *OpalDisk
> +  );
> +
>  #endif // _HII_H_
> --
> 2.21.0.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#40228): https://edk2.groups.io/g/devel/message/40228
Mute This Topic: https://groups.io/mt/31539608/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-