From patchwork Mon Mar 12 14:56:08 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 10276555 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 601BC602BD for ; Mon, 12 Mar 2018 14:56:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4CE8C285B9 for ; Mon, 12 Mar 2018 14:56:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4152C286B3; Mon, 12 Mar 2018 14:56:12 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 76820285B9 for ; Mon, 12 Mar 2018 14:56:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751315AbeCLO4L (ORCPT ); Mon, 12 Mar 2018 10:56:11 -0400 Received: from mail-it0-f67.google.com ([209.85.214.67]:39431 "EHLO mail-it0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751293AbeCLO4J (ORCPT ); Mon, 12 Mar 2018 10:56:09 -0400 Received: by mail-it0-f67.google.com with SMTP id l187-v6so11805595ith.4 for ; Mon, 12 Mar 2018 07:56:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=HxY0eHxLE+Z0BVTFsvPnDXvhJit71PXUyaERAI41OV8=; b=L6rDSKGnbUcxWlXFX8314b8im9IsX1/lgR3TwRKe7+1tlRFAtnasi8EDcYGzZH+bOm nyqH7juhN+2ugAp7WgrNP+T2ZWpJ9YSmBmQs+HOeD9owV/iHd7baAuh/Njfzf4Q+KzLS 02YQXACu7vjIHdwowJc6U/BmJiVJiG104TgRU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=HxY0eHxLE+Z0BVTFsvPnDXvhJit71PXUyaERAI41OV8=; b=hFVGRtIz4JCmwJWgUdiVTSwhjNs521+f+8/k81nyXopLkBizkuaGJG9tvBWhiOEeFT jvSNrCYD9G/lvEKNP6tfX4lNXzI9aDGoc7qQe6kRhsYNHiKWMo4anBFDTiev+f68Rrsd AhBaREQIcGXxR7hpm/BlCbbptTgHuZmX4HeUWE34wAnKZvYnPKwhpdMooQxm6ZiFwxkY w6OpG6xYJTcMsm3pcNbnQJeLMPNsUQt8XsscfFcA/EGYEHuiLxCmj/Rb0CpC4D4mwnZG Zs14+DiKs/6P79zdQN705Jh9YzueH8USE/9wWKNTRe6Dp0AFT3RF/BT0z0iXqSgKT4rB wiUA== X-Gm-Message-State: AElRT7FHGV9NhgZemlwAyE/ML7UcjwDiyX8VIGMMgT1qQ9T2m5jlkYIf XeQJonu/1R+FgEyG6kvkJG4J4BZiwS+CfqGUhUDj+A== X-Google-Smtp-Source: AG47ELvQV8bW18eQDd6HE3CBSEwSqCKIwiiDVC9RQhbrNTAh4cY9ajreM2Xnf1ff5fyePYKJq+S3w0JlTvfnFZfAHI0= X-Received: by 10.36.60.216 with SMTP id m207mr8914362ita.68.1520866569111; Mon, 12 Mar 2018 07:56:09 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.138.209 with HTTP; Mon, 12 Mar 2018 07:56:08 -0700 (PDT) In-Reply-To: <010001621a9e5069-0b1a6328-97e4-4396-9438-b90f5b8c82a4-000000@email.amazonses.com> References: <01000161fc0b4755-df0621f4-ab5d-479a-b425-adf98427a308-000000@email.amazonses.com> <0100016206a68850-bd5c96b3-f275-46ea-98b1-1317e02a5d6e-000000@email.amazonses.com> <29c1640a-cf19-ca19-7de9-96f202edfb5a@redhat.com> <010001620bafa06b-41525407-603e-40a9-ba11-6033b2f5dcc7-000000@email.amazonses.com> <010001621a9e5069-0b1a6328-97e4-4396-9438-b90f5b8c82a4-000000@email.amazonses.com> From: Ard Biesheuvel Date: Mon, 12 Mar 2018 14:56:08 +0000 Message-ID: Subject: Re: Regression from efi: call get_event_log before ExitBootServices To: Jeremy Cline Cc: Thiebaud Weksteen , Hans de Goede , Javier Martinez Canillas , Jarkko Sakkinen , linux-efi@vger.kernel.org, linux-integrity@vger.kernel.org, tpmdd-devel@lists.sourceforge.net, Linux Kernel Mailing List Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 12 March 2018 at 14:30, Jeremy Cline wrote: > On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: >> On 10 March 2018 at 10:45, Thiebaud Weksteen wrote: >>> On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline wrote: >>> >>>> On Fri, Mar 09, 2018 at 10:43:50AM +0000, Thiebaud Weksteen wrote: >>>>> Thanks a lot for trying out the patch! >>>>> >>>>> Please don't modify your install at this stage, I think we are hitting a >>>>> firmware bug and that would be awesome if we can fix how we are >>> handling it. >>>>> So, if we reach that stage in the function it could either be that: >>>>> * The allocation did not succeed, somehow, but the firmware still >>> returned >>>>> EFI_SUCCEED. >>>>> * The size requested is incorrect (I'm thinking something like a 1G of >>>>> log). This would be due to either a miscalculation of log_size >>> (possible) >>>>> or; the returned values of GetEventLog are not correct. >>>>> I'm sending a patch to add checks for these. Could you please apply and >>>>> retest? >>>>> Again, thanks for helping debugging this. >>> >>>> No problem, thanks for the help :) >>> >>>> With the new patch: >>> >>>> Locating the TCG2Protocol >>>> Calling GetEventLog on TCG2Protocol >>>> Log returned >>>> log_location is not empty >>>> log_size != 0 >>>> log_size < 1M >>>> Allocating memory for storing the logs >>>> Returned from memory allocation >>>> Copying log to new location >>> >>>> And then it hangs. I added a couple more print statements: >>> >>>> diff --git a/drivers/firmware/efi/libstub/tpm.c >>> b/drivers/firmware/efi/libstub/tpm.c >>>> index ee3fac109078..1ab5638bc50e 100644 >>>> --- a/drivers/firmware/efi/libstub/tpm.c >>>> +++ b/drivers/firmware/efi/libstub/tpm.c >>>> @@ -148,8 +148,11 @@ void >>> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >>>> efi_printk(sys_table_arg, "Copying log to new location\n"); >>> >>>> memset(log_tbl, 0, sizeof(*log_tbl) + log_size); >>>> + efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n"); >>>> log_tbl->size = log_size; >>>> + efi_printk(sys_table_arg, "Set log_tbl->size\n"); >>>> log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; >>>> + efi_printk(sys_table_arg, "Set log_tbl-version\n"); >>>> memcpy(log_tbl->log, (void *) first_entry_addr, log_size); >>> >>>> efi_printk(sys_table_arg, "Installing the log into the >>> configuration table\n"); >>> >>>> and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" >>> >>> Thanks. Well, it looks like the memory that is supposedly allocated is not >>> usable. I'm thinking this is a firmware bug. >>> Ard, would you agree on this assumption? Thoughts on how to proceed? >>> >> >> I am rather puzzled why the allocate_pool() should succeed and the >> subsequent memset() should fail. This does not look like an issue that >> is intimately related to TPM2 support, rather an issue in the firmware >> that happens to get tickled after the change. >> >> Would you mind trying replacing EFI_LOADER_DATA with >> EFI_BOOT_SERVICES_DATA in the allocate_pool() call? > > Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at the > memset() call. > Could you try the following please? (attached as well in case gmail mangles it) &tcg2_protocol); @@ -104,9 +106,12 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) } /* Allocate space for the logs and copy them. */ - status = efi_call_early(allocate_pool, EFI_LOADER_DATA, - sizeof(*log_tbl) + log_size, - (void **) &log_tbl); + num_pages = DIV_ROUND_UP(sizeof(*log_tbl) + log_size, EFI_PAGE_SIZE); + status = efi_call_early(allocate_pages, + EFI_ALLOCATE_ANY_PAGES, + EFI_LOADER_DATA, + num_pages, + &log_tbl_alloc); if (status != EFI_SUCCESS) { efi_printk(sys_table_arg, @@ -114,6 +119,7 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) return; } + log_tbl = (struct linux_efi_tpm_eventlog *)(unsigned long)log_tbl_alloc; memset(log_tbl, 0, sizeof(*log_tbl) + log_size); log_tbl->size = log_size; log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; @@ -126,7 +132,7 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) return; err_free: - efi_call_early(free_pool, log_tbl); + efi_call_early(free_pages, log_tbl_alloc, num_pages); } void efi_retrieve_tpm2_eventlog(efi_system_table_t *sys_table_arg) diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c index 2298560cea72..30d960a344b7 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -70,6 +70,8 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) size_t log_size, last_entry_size; efi_bool_t truncated; void *tcg2_protocol; + unsigned long num_pages; + efi_physical_addr_t log_tbl_alloc; status = efi_call_early(locate_protocol, &tcg2_guid, NULL, &tcg2_protocol); @@ -104,9 +106,12 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) } /* Allocate space for the logs and copy them. */ - status = efi_call_early(allocate_pool, EFI_LOADER_DATA, - sizeof(*log_tbl) + log_size, - (void **) &log_tbl); + num_pages = DIV_ROUND_UP(sizeof(*log_tbl) + log_size, EFI_PAGE_SIZE); + status = efi_call_early(allocate_pages, + EFI_ALLOCATE_ANY_PAGES, + EFI_LOADER_DATA, + num_pages, + &log_tbl_alloc); if (status != EFI_SUCCESS) { efi_printk(sys_table_arg, @@ -114,6 +119,7 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) return; } + log_tbl = (struct linux_efi_tpm_eventlog *)(unsigned long)log_tbl_alloc; memset(log_tbl, 0, sizeof(*log_tbl) + log_size); log_tbl->size = log_size; log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; @@ -126,7 +132,7 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) return; err_free: - efi_call_early(free_pool, log_tbl); + efi_call_early(free_pages, log_tbl_alloc, num_pages); } void efi_retrieve_tpm2_eventlog(efi_system_table_t *sys_table_arg)