On 17 October 2017 at 20:16, Laszlo Ersek <[email protected]> wrote: > Commit 53c6ff180327 ("SecurityPkg:AuthVariableLib:Implement ECR1707 for > Private Auth Variable", 2017-09-12) introduced the following build > failure under several GCC toolchain versions: > >> SecurityPkg/Library/AuthVariableLib/AuthService.c: In function >> 'CalculatePrivAuthVarSignChainSHA256Digest': >> SecurityPkg/Library/AuthVariableLib/AuthService.c:1567:58: error: >> pointer targets in passing argument 3 of 'X509GetCommonName' differ in >> signedness [-Werror=pointer-sign] >> Status = X509GetCommonName(SignerCert, SignerCertSize, CertCommonName, >> &CertCommonNameSize); >> ^~~~~~~~~~~~~~ >> In file included from >> SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h:34:0, >> from >> SecurityPkg/Library/AuthVariableLib/AuthService.c:32: >> CryptoPkg/Include/Library/BaseCryptLib.h:2202:1: note: expected 'CHAR8 * >> {aka char *}' but argument is of type 'UINT8 * {aka unsigned char *}' >> X509GetCommonName ( >> ^~~~~~~~~~~~~~~~~ >> cc1: all warnings being treated as errors > > Fix it by changing the type of "CertCommonName" to array-of-CHAR8. > > Locations where "CertCommonName" is used in the > CalculatePrivAuthVarSignChainSHA256Digest() function: > > - it is taken the size of -- not impacted by this patch; > > - passed to X509GetCommonName() as an argument -- the patch fixes the > build error; > > - passed to Sha256Update() as argument for "IN CONST VOID *Data" -- not > impacted by the patch; > > - passed to AsciiStrLen() as argument -- drop the now-superfluous explicit > cast. > > Since we are touching the Sha256Update() function call, fix the coding > style too: > > - the line is overlong, so break each argument to its own line; > > - insert a space between "AsciiStrLen" and the opening paren "(". > > Cc: Ard Biesheuvel <[email protected]> > Cc: Chao Zhang <[email protected]> > Cc: Gary Lin <[email protected]> > Cc: Leif Lindholm <[email protected]> > Cc: Long Qin <[email protected]> > Reported-by: Gary Lin <[email protected]> > Suggested-by: Gary Lin <[email protected]> > Suggested-by: Long Qin <[email protected]> > Fixes: 53c6ff18032737fabb644a9e0c781d91a6830248 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek <[email protected]> > --- > > Notes: > The GCC build has been broken for too long by now; I'll push the patch > as soon as I get any Reviewed-by. >
Reviewed-by: Ard Biesheuvel <[email protected]> > SecurityPkg/Library/AuthVariableLib/AuthService.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c > b/SecurityPkg/Library/AuthVariableLib/AuthService.c > index 7188ff600823..2966811fa7ff 100644 > --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c > +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c > @@ -1554,7 +1554,7 @@ CalculatePrivAuthVarSignChainSHA256Digest( > { > UINT8 *TbsCert; > UINTN TbsCertSize; > - UINT8 CertCommonName[128]; > + CHAR8 CertCommonName[128]; > UINTN CertCommonNameSize; > BOOLEAN CryptoStatus; > EFI_STATUS Status; > @@ -1590,7 +1590,11 @@ CalculatePrivAuthVarSignChainSHA256Digest( > // > // '\0' is forced in CertCommonName. No overflow issue > // > - CryptoStatus = Sha256Update (mHashCtx, CertCommonName, AsciiStrLen((CHAR8 > *)CertCommonName)); > + CryptoStatus = Sha256Update ( > + mHashCtx, > + CertCommonName, > + AsciiStrLen (CertCommonName) > + ); > if (!CryptoStatus) { > return EFI_ABORTED; > } > -- > 2.14.1.3.gb7cf6e02401b > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
