From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1kXfNZ-0001Y9-D9 for mharc-grub-devel@gnu.org; Wed, 28 Oct 2020 02:56:17 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49056) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kXfNX-0001Xk-Fu for grub-devel@gnu.org; Wed, 28 Oct 2020 02:56:15 -0400 Received: from out30-54.freemail.mail.aliyun.com ([115.124.30.54]:45092) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kXfNP-0005ZS-Bu for grub-devel@gnu.org; Wed, 28 Oct 2020 02:56:10 -0400 X-Alimail-AntiSpam: AC=PASS; BC=-1|-1; BR=01201311R131e4; CH=green; DM=||false|; DS=||; FP=0|-1|-1|-1|0|-1|-1|-1; HT=e01e04357; MF=tianjia.zhang@linux.alibaba.com; NM=1; PH=DS; RN=2; SR=0; TI=SMTPD_---0UDQzS2D_1603868152; Received: from B-455UMD6M-2027.local(mailfrom:tianjia.zhang@linux.alibaba.com fp:SMTPD_---0UDQzS2D_1603868152) by smtp.aliyun-inc.com(127.0.0.1); Wed, 28 Oct 2020 14:55:53 +0800 Subject: Re: [PATCH] tpm: Add debug information for device protocol and eventlog To: Daniel Kiper Cc: grub-devel@gnu.org References: <20200729133327.53814-1-tianjia.zhang@linux.alibaba.com> <20201027205825.hvporq7q7rkew4jz@tomti.i.net-space.pl> From: Tianjia Zhang Message-ID: <26f73d12-8a31-8092-534a-8eaf89e540b8@linux.alibaba.com> Date: Wed, 28 Oct 2020 14:55:52 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.3.2 MIME-Version: 1.0 In-Reply-To: <20201027205825.hvporq7q7rkew4jz@tomti.i.net-space.pl> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=115.124.30.54; envelope-from=tianjia.zhang@linux.alibaba.com; helo=out30-54.freemail.mail.aliyun.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/10/28 02:55:54 X-ACL-Warn: Detected OS = Linux 3.x [generic] [fuzzy] X-Spam_score_int: -120 X-Spam_score: -12.1 X-Spam_bar: ------------ X-Spam_report: (-12.1 / 5.0 requ) BAYES_00=-1.9, ENV_AND_HDR_SPF_MATCH=-0.5, NICE_REPLY_A=-2.167, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001, USER_IN_DEF_SPF_WL=-7.5 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Oct 2020 06:56:15 -0000 Thanks for pointing it out, this patch is too careless, very sorry, I will revise another version. Best regards, Tianjia On 10/28/20 4:58 AM, Daniel Kiper wrote: > Hi, > > First of all, sorry for late reply... > > On Wed, Jul 29, 2020 at 09:33:27PM +0800, Tianjia Zhang wrote: >> Add a number of debug logs to the tpm module. The condition tag >> for opening debugging is `tpm`. On TPM machines, this will bring >> great convenience to diagnosis and debugging. >> >> Signed-off-by: Tianjia Zhang >> --- >> grub-core/commands/efi/tpm.c | 21 +++++++++++++++++---- >> 1 file changed, 17 insertions(+), 4 deletions(-) >> >> diff --git a/grub-core/commands/efi/tpm.c b/grub-core/commands/efi/tpm.c >> index b03b6b296..9b2cd43f1 100644 >> --- a/grub-core/commands/efi/tpm.c >> +++ b/grub-core/commands/efi/tpm.c >> @@ -56,9 +56,12 @@ grub_tpm1_present (grub_efi_tpm_protocol_t *tpm) >> >> if (status != GRUB_EFI_SUCCESS || caps.TPMDeactivatedFlag >> || !caps.TPMPresentFlag) >> - return tpm1_present = 0; >> + tpm1_present = 0; >> >> - return tpm1_present = 1; >> + tpm1_present = 1; >> + >> + grub_dprintf ("tpm", "tpm1 %s present\n", tpm1_present ? "" : "NOT"); >> + return (grub_efi_boolean_t) tpm1_present; > > This does not work as you expect. tpm1 will be always reported as present. > >> } >> >> static grub_efi_boolean_t >> @@ -75,9 +78,12 @@ grub_tpm2_present (grub_efi_tpm2_protocol_t *tpm) >> status = efi_call_2 (tpm->get_capability, tpm, &caps); >> >> if (status != GRUB_EFI_SUCCESS || !caps.TPMPresentFlag) >> - return tpm2_present = 0; >> + tpm2_present = 0; >> + >> + tpm2_present = 1; >> >> - return tpm2_present = 1; >> + grub_dprintf ("tpm", "tpm2 %s present\n", tpm2_present ? "" : "NOT"); >> + return (grub_efi_boolean_t) tpm2_present; >> } > > Ditto except WRT tpm2. > >> static grub_efi_boolean_t >> @@ -102,6 +108,7 @@ grub_tpm_handle_find (grub_efi_handle_t *tpm_handle, >> *tpm_handle = handles[0]; >> grub_tpm_version = 1; >> *protocol_version = 1; >> + grub_dprintf ("tpm", "TPM handle Found, version: 1\n"); >> return 1; >> } >> >> @@ -113,9 +120,11 @@ grub_tpm_handle_find (grub_efi_handle_t *tpm_handle, >> *tpm_handle = handles[0]; >> grub_tpm_version = 2; >> *protocol_version = 2; >> + grub_dprintf ("tpm", "TPM handle Found, version: 2\n"); >> return 1; >> } >> >> + grub_dprintf ("tpm", "NO TPM handle Found\n"); >> return 0; >> } >> >> @@ -147,6 +156,8 @@ grub_tpm1_log_event (grub_efi_handle_t tpm_handle, unsigned char *buf, >> event->EventSize = grub_strlen (description) + 1; >> grub_memcpy (event->Event, description, event->EventSize); >> >> + grub_dprintf ("tpm", "tpm1 log_extend_event, pcr = %d, size = %d, %s\n", >> + pcr, (int)size, description); > > Could you use PRIxGRUB_* macro from include/grub/types.h instead of "%d" for size here? > >> algorithm = TCG_ALG_SHA; >> status = efi_call_7 (tpm->log_extend_event, tpm, (grub_addr_t) buf, (grub_uint64_t) size, >> algorithm, event, &eventnum, &lastevent); >> @@ -199,6 +210,8 @@ grub_tpm2_log_event (grub_efi_handle_t tpm_handle, unsigned char *buf, >> sizeof (*event) - sizeof (event->Event) + grub_strlen (description) + 1; >> grub_memcpy (event->Event, description, grub_strlen (description) + 1); >> >> + grub_dprintf ("tpm", "tpm2 log_extend_event, pcr = %d, size = %d, %s\n", >> + pcr, (int)size, description); > > Ditto. > > However, I would prefer if you print this only from grub_tpm_measure() > function without tpm1/tpm2 prefix. > > Daniel >