From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: eric.dong@intel.com) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by groups.io with SMTP; Mon, 06 May 2019 18:05:17 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 May 2019 18:05:16 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,440,1549958400"; d="scan'208";a="140741334" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga008.jf.intel.com with ESMTP; 06 May 2019 18:05:16 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 6 May 2019 18:05:16 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.249]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.129]) with mapi id 14.03.0415.000; Tue, 7 May 2019 09:05:14 +0800 From: "Dong, Eric" To: "Chu, Maggie" , "devel@edk2.groups.io" CC: "Zhang, Chao B" , "Yao, Jiewen" Subject: Re: [PATCH] SecurityPkg/OpalPassword: Add warning message for Secure Erase Thread-Topic: [PATCH] SecurityPkg/OpalPassword: Add warning message for Secure Erase Thread-Index: AQHU/0EzNoaCChSgx0K0pYbUx5WKEaZe4tKA Date: Tue, 7 May 2019 01:05:13 +0000 Message-ID: References: <20190430104046.14964-1-maggie.chu@intel.com> In-Reply-To: <20190430104046.14964-1-maggie.chu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: eric.dong@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Maggie, The patch is good, only small comments for the commit message. Please remov= e the "Contributed-under: ... " part, this is not needed now. Please update the patch and send to me. I will help to submit it. Reviewed-by: Eric Dong Thanks, Eric > -----Original Message----- > From: Chu, Maggie > Sent: Tuesday, April 30, 2019 6:41 PM > To: devel@edk2.groups.io > Cc: Zhang, Chao B ; Yao, Jiewen > ; Dong, Eric > Subject: [PATCH] SecurityPkg/OpalPassword: Add warning message for > Secure Erase >=20 > https://bugzilla.tianocore.org/show_bug.cgi?id=3D1753 > Add pop-up warning messages before secure erase action. > In order to notify user the secure erase action will take a longer time. > This change also fix some pop-up windows are unable to show up complete > message due to some strings are too long. >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Maggie Chu > Cc: Chao Zhang > Cc: Jiewen Yao > Cc: Eric Dong > --- > SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c | 131 > ++++++++++++++++++------- > SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c | 23 +++-- > 2 files changed, 112 insertions(+), 42 deletions(-) >=20 > diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c > b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c > index ed7f968255..42999c89f0 100644 > --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c > +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c > @@ -487,6 +487,7 @@ OpalEndOfDxeEventNotify ( > OPAL request. > @param[in] PopUpString Pop up string. > @param[in] PopUpString2 Pop up string in line 2. > + @param[in] PopUpString3 Pop up string in line 3. >=20 > @param[out] PressEsc Whether user escape function through Press E= SC. >=20 > @@ -498,6 +499,7 @@ OpalDriverPopUpPsidInput ( > IN OPAL_DRIVER_DEVICE *Dev, > IN CHAR16 *PopUpString, > IN CHAR16 *PopUpString2, > + IN CHAR16 *PopUpString3, > OUT BOOLEAN *PressEsc > ) > { > @@ -527,15 +529,28 @@ OpalDriverPopUpPsidInput ( > NULL > ); > } else { > - CreatePopUp ( > - EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, > - &InputKey, > - PopUpString, > - PopUpString2, > - L"---------------------", > - Mask, > - NULL > - ); > + if (PopUpString3 =3D=3D NULL) { > + CreatePopUp ( > + EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, > + &InputKey, > + PopUpString, > + PopUpString2, > + L"---------------------", > + Mask, > + NULL > + ); > + } else { > + CreatePopUp ( > + EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, > + &InputKey, > + PopUpString, > + PopUpString2, > + PopUpString3, > + L"---------------------", > + Mask, > + NULL > + ); > + } > } >=20 > // > @@ -625,6 +640,7 @@ OpalDriverPopUpPsidInput ( > process OPAL request. > @param[in] PopUpString1 Pop up string 1. > @param[in] PopUpString2 Pop up string 2. > + @param[in] PopUpString3 Pop up string 3. > @param[out] PressEsc Whether user escape function through Press E= SC. >=20 > @retval Password string if success. NULL if failed. > @@ -635,6 +651,7 @@ OpalDriverPopUpPasswordInput ( > IN OPAL_DRIVER_DEVICE *Dev, > IN CHAR16 *PopUpString1, > IN CHAR16 *PopUpString2, > + IN CHAR16 *PopUpString3, > OUT BOOLEAN *PressEsc > ) > { > @@ -664,15 +681,28 @@ OpalDriverPopUpPasswordInput ( > NULL > ); > } else { > - CreatePopUp ( > - EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, > - &InputKey, > - PopUpString1, > - PopUpString2, > - L"---------------------", > - Mask, > - NULL > - ); > + if (PopUpString3 =3D=3D NULL) { > + CreatePopUp ( > + EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, > + &InputKey, > + PopUpString1, > + PopUpString2, > + L"---------------------", > + Mask, > + NULL > + ); > + } else { > + CreatePopUp ( > + EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, > + &InputKey, > + PopUpString1, > + PopUpString2, > + PopUpString3, > + L"---------------------", > + Mask, > + NULL > + ); > + } > } >=20 > // > @@ -823,7 +853,7 @@ OpalDriverRequestPassword ( > } >=20 > while (Count < MAX_PASSWORD_TRY_COUNT) { > - Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, NULL, > &PressEsc); > + Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, NULL, > + NULL, &PressEsc); > if (PressEsc) { > if (IsLocked) { > // > @@ -988,7 +1018,7 @@ ProcessOpalRequestEnableFeature ( > Session.OpalBaseComId =3D Dev->OpalDisk.OpalBaseComId; >=20 > while (Count < MAX_PASSWORD_TRY_COUNT) { > - Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, L"Pleas= e > type in your new password", &PressEsc); > + Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, L"Pleas= e > + type in your new password", NULL, &PressEsc); > if (PressEsc) { > do { > CreatePopUp ( > @@ -1017,7 +1047,7 @@ ProcessOpalRequestEnableFeature ( > } > PasswordLen =3D (UINT32) AsciiStrLen(Password); >=20 > - PasswordConfirm =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > L"Please confirm your new password", &PressEsc); > + PasswordConfirm =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > + L"Please confirm your new password", NULL, &PressEsc); > if (PasswordConfirm =3D=3D NULL) { > ZeroMem (Password, PasswordLen); > FreePool (Password); > @@ -1132,7 +1162,7 @@ ProcessOpalRequestDisableUser ( > Session.OpalBaseComId =3D Dev->OpalDisk.OpalBaseComId; >=20 > while (Count < MAX_PASSWORD_TRY_COUNT) { > - Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, NULL, > &PressEsc); > + Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, NULL, > + NULL, &PressEsc); > if (PressEsc) { > do { > CreatePopUp ( > @@ -1227,6 +1257,7 @@ ProcessOpalRequestPsidRevert ( > TCG_RESULT Ret; > CHAR16 *PopUpString; > CHAR16 *PopUpString2; > + CHAR16 *PopUpString3; > UINTN BufferSize; >=20 > if (Dev =3D=3D NULL) { > @@ -1238,17 +1269,19 @@ ProcessOpalRequestPsidRevert ( > PopUpString =3D OpalGetPopUpString (Dev, RequestString); >=20 > if (Dev->OpalDisk.EstimateTimeCost > > MAX_ACCEPTABLE_REVERTING_TIME) { > - BufferSize =3D StrSize (L"Warning: Revert action will take about ###= #### > seconds, DO NOT power off system during the revert action!"); > + BufferSize =3D StrSize (L"Warning: Revert action will take about > + ####### seconds"); > PopUpString2 =3D AllocateZeroPool (BufferSize); > ASSERT (PopUpString2 !=3D NULL); > UnicodeSPrint ( > PopUpString2, > BufferSize, > - L"WARNING: Revert action will take about %d seconds, DO NOT powe= r > off system during the revert action!", > + L"WARNING: Revert action will take about %d seconds", > Dev->OpalDisk.EstimateTimeCost > ); > + PopUpString3 =3D L"DO NOT power off system during the revert > + action!"; > } else { > PopUpString2 =3D NULL; > + PopUpString3 =3D NULL; > } >=20 > Count =3D 0; > @@ -1259,7 +1292,7 @@ ProcessOpalRequestPsidRevert ( > Session.OpalBaseComId =3D Dev->OpalDisk.OpalBaseComId; >=20 > while (Count < MAX_PSID_TRY_COUNT) { > - Psid =3D OpalDriverPopUpPsidInput (Dev, PopUpString, PopUpString2, > &PressEsc); > + Psid =3D OpalDriverPopUpPsidInput (Dev, PopUpString, PopUpString2, > + PopUpString3, &PressEsc); > if (PressEsc) { > do { > CreatePopUp ( > @@ -1361,6 +1394,7 @@ ProcessOpalRequestRevert ( > BOOLEAN PasswordFailed; > CHAR16 *PopUpString; > CHAR16 *PopUpString2; > + CHAR16 *PopUpString3; > UINTN BufferSize; >=20 > if (Dev =3D=3D NULL) { > @@ -1373,17 +1407,19 @@ ProcessOpalRequestRevert ( >=20 > if ((!KeepUserData) && > (Dev->OpalDisk.EstimateTimeCost > > MAX_ACCEPTABLE_REVERTING_TIME)) { > - BufferSize =3D StrSize (L"Warning: Revert action will take about ###= #### > seconds, DO NOT power off system during the revert action!"); > + BufferSize =3D StrSize (L"Warning: Revert action will take about > + ####### seconds"); > PopUpString2 =3D AllocateZeroPool (BufferSize); > ASSERT (PopUpString2 !=3D NULL); > UnicodeSPrint ( > PopUpString2, > BufferSize, > - L"WARNING: Revert action will take about %d seconds, DO NOT powe= r > off system during the revert action!", > + L"WARNING: Revert action will take about %d seconds", > Dev->OpalDisk.EstimateTimeCost > ); > + PopUpString3 =3D L"DO NOT power off system during the revert > + action!"; > } else { > PopUpString2 =3D NULL; > + PopUpString3 =3D NULL; > } >=20 > Count =3D 0; > @@ -1394,7 +1430,7 @@ ProcessOpalRequestRevert ( > Session.OpalBaseComId =3D Dev->OpalDisk.OpalBaseComId; >=20 > while (Count < MAX_PASSWORD_TRY_COUNT) { > - Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > PopUpString2, &PressEsc); > + Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > + PopUpString2, PopUpString3, &PressEsc); > if (PressEsc) { > do { > CreatePopUp ( > @@ -1520,6 +1556,9 @@ ProcessOpalRequestSecureErase ( > TCG_RESULT Ret; > BOOLEAN PasswordFailed; > CHAR16 *PopUpString; > + CHAR16 *PopUpString2; > + CHAR16 *PopUpString3; > + UINTN BufferSize; >=20 > if (Dev =3D=3D NULL) { > return; > @@ -1529,6 +1568,21 @@ ProcessOpalRequestSecureErase ( >=20 > PopUpString =3D OpalGetPopUpString (Dev, RequestString); >=20 > + if (Dev->OpalDisk.EstimateTimeCost > > MAX_ACCEPTABLE_REVERTING_TIME) { > + BufferSize =3D StrSize (L"Warning: Secure erase action will take abo= ut > ####### seconds"); > + PopUpString2 =3D AllocateZeroPool (BufferSize); > + ASSERT (PopUpString2 !=3D NULL); > + UnicodeSPrint ( > + PopUpString2, > + BufferSize, > + L"WARNING: Secure erase action will take about %d seconds", > + Dev->OpalDisk.EstimateTimeCost > + ); > + PopUpString3 =3D L"DO NOT power off system during the action!"; } > + else { > + PopUpString2 =3D NULL; > + PopUpString3 =3D NULL; > + } > Count =3D 0; >=20 > ZeroMem(&Session, sizeof(Session)); > @@ -1537,7 +1591,7 @@ ProcessOpalRequestSecureErase ( > Session.OpalBaseComId =3D Dev->OpalDisk.OpalBaseComId; >=20 > while (Count < MAX_PASSWORD_TRY_COUNT) { > - Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, NULL, > &PressEsc); > + Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > + PopUpString2, PopUpString3, &PressEsc); > if (PressEsc) { > do { > CreatePopUp ( > @@ -1551,7 +1605,7 @@ ProcessOpalRequestSecureErase ( >=20 > if (Key.UnicodeChar =3D=3D CHAR_CARRIAGE_RETURN) { > gST->ConOut->ClearScreen(gST->ConOut); > - return; > + goto Done; > } else { > // > // Let user input password again. > @@ -1608,6 +1662,11 @@ ProcessOpalRequestSecureErase ( > } while (Key.UnicodeChar !=3D CHAR_CARRIAGE_RETURN); > gST->ConOut->ClearScreen(gST->ConOut); > } > + > +Done: > + if (PopUpString2 !=3D NULL) { > + FreePool (PopUpString2); > + } > } >=20 > /** > @@ -1647,7 +1706,7 @@ ProcessOpalRequestSetUserPwd ( > Count =3D 0; >=20 > while (Count < MAX_PASSWORD_TRY_COUNT) { > - OldPassword =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > L"Please type in your password", &PressEsc); > + OldPassword =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > + L"Please type in your password", NULL, &PressEsc); > if (PressEsc) { > do { > CreatePopUp ( > @@ -1705,7 +1764,7 @@ ProcessOpalRequestSetUserPwd ( > } > } >=20 > - Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, L"Pleas= e > type in your new password", &PressEsc); > + Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, L"Pleas= e > + type in your new password", NULL, &PressEsc); > if (Password =3D=3D NULL) { > ZeroMem (OldPassword, OldPasswordLen); > FreePool (OldPassword); > @@ -1714,7 +1773,7 @@ ProcessOpalRequestSetUserPwd ( > } > PasswordLen =3D (UINT32) AsciiStrLen(Password); >=20 > - PasswordConfirm =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > L"Please confirm your new password", &PressEsc); > + PasswordConfirm =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > + L"Please confirm your new password", NULL, &PressEsc); > if (PasswordConfirm =3D=3D NULL) { > ZeroMem (OldPassword, OldPasswordLen); > FreePool (OldPassword); > @@ -1846,7 +1905,7 @@ ProcessOpalRequestSetAdminPwd ( > Count =3D 0; >=20 > while (Count < MAX_PASSWORD_TRY_COUNT) { > - OldPassword =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > L"Please type in your password", &PressEsc); > + OldPassword =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > + L"Please type in your password", NULL, &PressEsc); > if (PressEsc) { > do { > CreatePopUp ( > @@ -1899,7 +1958,7 @@ ProcessOpalRequestSetAdminPwd ( > continue; > } >=20 > - Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, L"Pleas= e > type in your new password", &PressEsc); > + Password =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, L"Pleas= e > + type in your new password", NULL, &PressEsc); > if (Password =3D=3D NULL) { > ZeroMem (OldPassword, OldPasswordLen); > FreePool (OldPassword); > @@ -1908,7 +1967,7 @@ ProcessOpalRequestSetAdminPwd ( > } > PasswordLen =3D (UINT32) AsciiStrLen(Password); >=20 > - PasswordConfirm =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > L"Please confirm your new password", &PressEsc); > + PasswordConfirm =3D OpalDriverPopUpPasswordInput (Dev, PopUpString, > + L"Please confirm your new password", NULL, &PressEsc); > if (PasswordConfirm =3D=3D NULL) { > ZeroMem (OldPassword, OldPasswordLen); > FreePool (OldPassword); > diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c > b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c > index 8abb3d028b..d0f3eda1e8 100644 > --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c > +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c > @@ -511,13 +511,15 @@ GetDiskNameStringId( > /** > Confirm whether user truly want to do the revert action. >=20 > - @param OpalDisk The device which need to do the revert = action. > + @param OpalDisk The device which need to perform data r= emoval > action. > + @param ActionString Specifies the action name shown on pop = up > menu. >=20 > @retval EFI_SUCCESS Confirmed user want to do the revert ac= tion. > **/ > EFI_STATUS > -HiiConfirmRevertAction ( > - IN OPAL_DISK *OpalDisk > +HiiConfirmDataRemovalAction ( > + IN OPAL_DISK *OpalDisk, > + IN CHAR16 *ActionString >=20 > ) > { > @@ -537,14 +539,14 @@ HiiConfirmRevertAction ( > ApproveResponse =3D L'Y'; > RejectResponse =3D L'N'; >=20 > - UnicodeSPrint(Unicode, StrSize(L"WARNING: Revert device needs about > ####### seconds"), L"WARNING: Revert device needs about %d seconds", > OpalDisk->EstimateTimeCost); > + UnicodeSPrint(Unicode, StrSize(L"WARNING: ############# action > needs > + about ####### seconds"), L"WARNING: %s action needs about %d > seconds", > + ActionString, OpalDisk->EstimateTimeCost); >=20 > do { > CreatePopUp( > EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, > &Key, > Unicode, > - L" System should not be powered off until revert completion ", > + L" System should not be powered off until action completion ", > L" ", > L" Press 'Y/y' to continue, press 'N/n' to cancal ", > NULL > @@ -634,7 +636,16 @@ DriverCallback( > case HII_KEY_ID_PSID_REVERT: > OpalDisk =3D HiiGetOpalDiskCB(gHiiConfiguration.SelectedDiskInde= x); > if (OpalDisk !=3D NULL) { > - return HiiConfirmRevertAction (OpalDisk); > + return HiiConfirmDataRemovalAction (OpalDisk, L"Revert"); > + } else { > + ASSERT (FALSE); > + return EFI_SUCCESS; > + } > + > + case HII_KEY_ID_SECURE_ERASE: > + OpalDisk =3D HiiGetOpalDiskCB(gHiiConfiguration.SelectedDiskInde= x); > + if (OpalDisk !=3D NULL) { > + return HiiConfirmDataRemovalAction (OpalDisk, L"Secure > + erase"); > } else { > ASSERT (FALSE); > return EFI_SUCCESS; > -- > 2.16.2.windows.1