From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web11.56057.1598888794271318156 for ; Mon, 31 Aug 2020 08:46:34 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=K0aMoYkv; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1598888793; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MxVCqhgvmvhJBMEwnOrHXCydwBQNSy9rnXO6SfLPtWY=; b=K0aMoYkv2LA6Lz5s8qzN6EmuAsfYKbhh0PLDtCS+3mHIQ6cGmkn0kIT+wBTyV8K6dQoBTg twEj8EPumEEFRuUOmFkpaCZVBxRZpef3QUD57JmyJ+JTcRnXFfCFyqaxSLfZcyOzO7brsz vJKqYGGQitAARn9D5g1Eg7R4HdQ2S54= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-159-CuHo5Gt_O4a2toMWGyc03A-1; Mon, 31 Aug 2020 11:46:24 -0400 X-MC-Unique: CuHo5Gt_O4a2toMWGyc03A-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 894C664080; Mon, 31 Aug 2020 15:46:22 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-125.ams2.redhat.com [10.36.113.125]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3F6516198C; Mon, 31 Aug 2020 15:46:21 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] SecurityPkg: Initailize variable Status before it is consumed. To: devel@edk2.groups.io, zhiguang.liu@intel.com Cc: Jiewen Yao , Jian J Wang , Qi Zhang , Rahul Kumar References: <20200831081540.1431-1-zhiguang.liu@intel.com> From: "Laszlo Ersek" Message-ID: Date: Mon, 31 Aug 2020 17:46:20 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200831081540.1431-1-zhiguang.liu@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0.001 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US On 08/31/20 10:15, Zhiguang Liu wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2945 > > Cc: Jiewen Yao > Cc: Jian J Wang > Cc: Qi Zhang > Cc: Rahul Kumar > Signed-off-by: Zhiguang Liu > --- > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > index 0e770f4485..5e883f0cc5 100644 > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > @@ -449,6 +449,7 @@ HashLogExtendEvent ( > EFI_STATUS Status; > TPML_DIGEST_VALUES DigestList; > > + Status = EFI_SUCCESS; > if (GetFirstGuidHob (&gTpmErrorHobGuid) != NULL) { > return EFI_DEVICE_ERROR; > } > I agree that there is a path in the code where Status is read without having been set. It seems that using EFI_SUCCESS as default value makes sense (assuming the LogHashEvent() call is the important one). I'll let SecurityPkg maintainers decide about this. I think it would be nicer to set Status to EFI_SUCCESS either on the (currently missing) "else" branch, or else just before the EDKII_TCG_PRE_HASH check. In particular, setting Status so early that we may still exit with EFI_DEVICE_ERROR is wasteful. So at least I'd move the assignment past the "return EFI_DEVICE_ERROR" statement. But... it's late. I agree that this patch qualifies for the stable tag. So I'm not asking for a repost. I just wish more thought had been given to the placement of the assignment. Laszlo