From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C0FAF202E5CF9 for ; Tue, 17 Oct 2017 12:50:30 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A819E4E02A; Tue, 17 Oct 2017 19:54:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com A819E4E02A Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-43.rdu2.redhat.com [10.10.120.43]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2707C6292A; Tue, 17 Oct 2017 19:54:04 +0000 (UTC) From: Laszlo Ersek To: edk2-devel-01 Cc: Long Qin , Gary Lin , Leif Lindholm , Chao Zhang , Ard Biesheuvel References: <20171017191646.26065-1-lersek@redhat.com> Message-ID: <8b22761e-7f63-b0ca-4c9d-76c3ba72a53b@redhat.com> Date: Tue, 17 Oct 2017 21:54:04 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171017191646.26065-1-lersek@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Tue, 17 Oct 2017 19:54:06 +0000 (UTC) Subject: Re: [PATCH] SecurityPkg/AuthVariableLib: fix GCC build error X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Oct 2017 19:50:31 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/17/17 21:16, Laszlo Ersek 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 > Cc: Chao Zhang > Cc: Gary Lin > Cc: Leif Lindholm > Cc: Long Qin > Reported-by: Gary Lin > Suggested-by: Gary Lin > Suggested-by: Long Qin > Fixes: 53c6ff18032737fabb644a9e0c781d91a6830248 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek > --- > > 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. > > 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; > } > Awesome; in such a short time, I got *two* R-b's. :) Commit 11b74aa4724a. Thanks! Laszlo