From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::232; helo=mail-wm0-x232.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x232.google.com (mail-wm0-x232.google.com [IPv6:2a00:1450:400c:c09::232]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9E2D1202E60E3 for ; Tue, 17 Oct 2017 12:39:05 -0700 (PDT) Received: by mail-wm0-x232.google.com with SMTP id i124so6054409wmf.3 for ; Tue, 17 Oct 2017 12:42:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=mv8i+6o2UBTC/2IdRziDCqzaX+14F/48YHr4o7FdFUw=; b=Xsw4XOn+g/zhhyl30eGlzgCqIz6+/FEWTFn9rmwc4BHOHDnFTVrV8+aFY208PXhQHT HAtIGFDayy+VwsklpF1+ljwN4ANhzsUsla7vm+YNXVUgKC5Z3hz3W8g9emsTiNMtOGEe 2/9Lyg2QPL9uVl9NtwqJmjAZQcVEZK5ZMmv84= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=mv8i+6o2UBTC/2IdRziDCqzaX+14F/48YHr4o7FdFUw=; b=cKQ54aIqmfwjkvh8MynKetbypGOjEHUhIjuDt+ibzwLq7UtFVUTcB8D2aKNUoat/og sM4slesm9xLPenDyO5i+B+DSeWy0SaE3zjSWtiBgEYfWhKZ/48qZdpAEO4ZQoYmlDCEI vI/r+zl8mmpO/EFnGXRet/o49kc4C+pwKHhozXjUaM9DYSRu3ay7gbYGYlut9r2/bVLk 0aehnvvPvJi5/8gkVSPVJcgobRhaMxKEGZZcTlXJEzbGl/x6Bqv/46iBZd2O6eRllf9i yKtccNk9q9SF5rXSJ5tQuVKOKQHW2bnhXnZNTTGWXgPs6xKYe1kAzQb4XjYuRsaCZds9 DyEw== X-Gm-Message-State: AMCzsaUJ+r76Xox4pqu+PRkMcZdvERzus3Z5mjMy6dEQyeMmZRfXeAty KOw/9uXY8VxnAJQY4vKTMBrPYw== X-Google-Smtp-Source: ABhQp+SZNwN27jXxpJZ6jtoaYdtVSgkodWi5z+1b6yBLSdKNkNQni/PATza9uNX116azh67I58i96Q== X-Received: by 10.28.216.143 with SMTP id p137mr4900544wmg.155.1508269360561; Tue, 17 Oct 2017 12:42:40 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id v5sm10150759wrf.29.2017.10.17.12.42.38 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 17 Oct 2017 12:42:39 -0700 (PDT) Date: Tue, 17 Oct 2017 20:42:37 +0100 From: Leif Lindholm To: Laszlo Ersek Cc: edk2-devel-01 , Ard Biesheuvel , Chao Zhang , Gary Lin , Long Qin Message-ID: <20171017194237.fqn4rlcokhlhkyqz@bivouac.eciton.net> References: <20171017191646.26065-1-lersek@redhat.com> MIME-Version: 1.0 In-Reply-To: <20171017191646.26065-1-lersek@redhat.com> User-Agent: NeoMutt/20170113 (1.7.2) 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:39:05 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Oct 17, 2017 at 09:16:46PM +0200, 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 On the one hand, I am wondering whether that API _should_ be using UINT8 rather than an architecture-dependent type. On the other hand, that would be a much bigger change, and master is currently broken. Reviewed-by: Leif Lindholm / Leif > --- > > 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; > } > -- > 2.14.1.3.gb7cf6e02401b >