From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6DC33C43334 for ; Thu, 16 Jun 2022 02:59:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1355146AbiFPC7s (ORCPT ); Wed, 15 Jun 2022 22:59:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59504 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1350287AbiFPC7k (ORCPT ); Wed, 15 Jun 2022 22:59:40 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 5EA1F5A151 for ; Wed, 15 Jun 2022 19:59:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1655348378; 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: in-reply-to:in-reply-to:references:references; bh=SyBYL1pY81YOYBd6hVWvRLbn1C7svVUvtneGDVFIWeM=; b=IE3PZx/SIVepSm/nwDu3yOCCcOhajU5uYADJxPUH53yCqLfrDxPhVEP8+AvgZEOf44vTo1 a1pjlwcHKz5l21I9ORgbq4ehZnP6tuHbhGwrwxfbWgtTjYEvTZi/ueF2VkeZyTlVXyn0Vt S5cu+Rb/6p1RlwUlrovG6d0g6FAhfws= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-528-e7JYHVoOPaK7Uddguyd0jw-1; Wed, 15 Jun 2022 22:59:33 -0400 X-MC-Unique: e7JYHVoOPaK7Uddguyd0jw-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C5FE9802C17; Thu, 16 Jun 2022 02:59:32 +0000 (UTC) Received: from localhost (ovpn-12-237.pek2.redhat.com [10.72.12.237]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8B4881410DD8; Thu, 16 Jun 2022 02:59:31 +0000 (UTC) Date: Thu, 16 Jun 2022 10:59:27 +0800 From: Baoquan He To: Mimi Zohar Cc: Jonathan McDowell , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , "x86@kernel.org" , "H. Peter Anvin" , Dmitry Kasatkin , James Morris , "Serge E. Hallyn" , "linux-kernel@vger.kernel.org" , "linux-integrity@vger.kernel.org" , "linux-security-module@vger.kernel.org" , "kexec@lists.infradead.org" Subject: Re: [PATCH v5] x86/kexec: Carry forward IMA measurement log on kexec Message-ID: References: <60813f86e960d12ed3738531a14382769a061a02.camel@linux.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <60813f86e960d12ed3738531a14382769a061a02.camel@linux.ibm.com> X-Scanned-By: MIMEDefang 2.85 on 10.11.54.7 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/13/22 at 05:01pm, Mimi Zohar wrote: > On Mon, 2022-06-13 at 10:30 +0000, Jonathan McDowell wrote: > > On kexec file load Integrity Measurement Architecture (IMA) subsystem > > may verify the IMA signature of the kernel and initramfs, and measure > > it. The command line parameters passed to the kernel in the kexec call > > may also be measured by IMA. A remote attestation service can verify > > a TPM quote based on the TPM event log, the IMA measurement list, and > > the TPM PCR data. This can be achieved only if the IMA measurement log > > is carried over from the current kernel to the next kernel across > > the kexec call. > > > > powerpc and ARM64 both achieve this using device tree with a > > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of > > device tree, so use the setup_data mechanism to pass the IMA buffer to > > the new kernel. > > > > (Mimi, Baoquan, I haven't included your reviewed-bys because this has > > changed the compile guards around the ima_(free|get)_kexec_buffer > > functions in order to fix the warning the kernel test robot found. I > > think this is the right thing to do and avoids us compiling them on > > platforms where they won't be used. The alternative would be to drop > > the guards in ima.h that Mimi requested for v4.)hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh > > > > Signed-off-by: Jonathan McDowell > > --- > > v5: > > - Guard ima_(free|get)_kexec_buffer functions with > > CONFIG_HAVE_IMA_KEXEC (kernel test robot) > > - Use setup_data_offset in setup_boot_parameters and update rather than > > calculating in call to setup_ima_state. > > v4: > > - Guard ima.h function prototypes with CONFIG_HAVE_IMA_KEXEC > > > diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c > > index 8d374cc552be..42a6c5721a43 100644 > > --- a/drivers/of/kexec.c > > +++ b/drivers/of/kexec.c > > @@ -9,6 +9,7 @@ > > * Copyright (C) 2016 IBM Corporation > > */ > > > > +#include > > #include > > #include > > #include > > @@ -115,6 +116,7 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr, > > return 0; > > } > > > > +#ifdef CONFIG_HAVE_IMA_KEXEC > > /** > > * ima_get_kexec_buffer - get IMA buffer from the previous kernel > > * @addr: On successful return, set to point to the buffer contents. > > @@ -173,6 +175,7 @@ int ima_free_kexec_buffer(void) > > > > return memblock_phys_free(addr, size); > > } > > +#endif > > Inside ima_{get,free}_kexec_buffer(), there's no need now to test > whether CONFIG_HAVE_IMA_KEXEC is enabled. > > if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC)) > return -ENOTSUPP; Indeed. The #ifdef added by Jonathan is redundant. Not sure if the original IS_ENABLED(CONFIG_HAVE_IMA_KEXEC) checking inside ima_{get,free}_kexec_buffer() is intended. I ever reviewed below patch, the IS_ENABLED(CONFIG_XX) inside static function will make the function compiled, and will be optimized out if the CONFIG_XX is not enabled. However, it only has effect on static function. Here, ima_{get,free}_kexec_buffer() is not static, likely we should remove the inside IS_ENABLED() checking. commit 4ece09be9913a87ff99ea347fd7e7adad5bdbc8f Author: Jisheng Zhang Date: Wed Mar 23 16:06:39 2022 -0700 x86/setup: use IS_ENABLED(CONFIG_KEXEC_CORE) instead of #ifdef Replace the conditional compilation using "#ifdef CONFIG_KEXEC_CORE" by a check for "IS_ENABLED(CONFIG_KEXEC_CORE)", to simplify the code and increase compile coverage. Other than this one Mimi pointed out, this patch looks good to me, thx. Reviewed-by: Baoquan He > > > > > > /** > > * remove_ima_buffer - remove the IMA buffer property and reservation from @fdt > > diff --git a/include/linux/ima.h b/include/linux/ima.h > > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E6790C433EF for ; Thu, 16 Jun 2022 02:59:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ln/L1jnJ0+41O7fi80wzluAhwkdaQ7SRcfBMTjeaTO8=; b=AQKAFBSCTdVViq qWP2ibCkqYScefiT5yI1fk/6CmI7nAqOtsr1D+iAhUzMfB5qoiVQiOpOeZyBKiNzKLY5/k79o7gAb kBJKjOXniCMcdCNEHvh56DcF5R8cUCeCfvzj2dDznnK0Wnoi89mGWMzosMRV8OCYhcmEMk6NF24Er hZWlDmelyjLxkM4nEuHnJKplS4aTDsFAWeb72eiKzKTJmAI5y9wsLiwP+9eqd12FMrBVbuYpFq40m e4uspkcN9hK0wtoOsQV/IqTRS0nkF9Sq/vVn2Z6Nh8EIcdAT7jTUWD2eGwiblb9IKWLoPKBm5uHJu h93gqnihp5qLmW1DMFcw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o1fjS-000B8I-2b; Thu, 16 Jun 2022 02:59:42 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1o1fjO-000B7U-Ow for kexec@lists.infradead.org; Thu, 16 Jun 2022 02:59:40 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1655348377; 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: in-reply-to:in-reply-to:references:references; bh=SyBYL1pY81YOYBd6hVWvRLbn1C7svVUvtneGDVFIWeM=; b=Mwz5n1BfWLTnfM1tqe34w6E1c22tAAOwKXYlshuIcoZCiKwJOev5hJF3HjTAEG6DAaL55Z bxGGARYkGQrzsQWG4PyUK66e8Ezko6qypfowWlPRSalKusc9BdrIUKaUIoj2H80vHNJohe yVCOyR7kP0V9Fd/aaKT3goSbdg88lt4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-528-e7JYHVoOPaK7Uddguyd0jw-1; Wed, 15 Jun 2022 22:59:33 -0400 X-MC-Unique: e7JYHVoOPaK7Uddguyd0jw-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C5FE9802C17; Thu, 16 Jun 2022 02:59:32 +0000 (UTC) Received: from localhost (ovpn-12-237.pek2.redhat.com [10.72.12.237]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8B4881410DD8; Thu, 16 Jun 2022 02:59:31 +0000 (UTC) Date: Thu, 16 Jun 2022 10:59:27 +0800 From: Baoquan He To: Mimi Zohar Cc: Jonathan McDowell , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , "x86@kernel.org" , "H. Peter Anvin" , Dmitry Kasatkin , James Morris , "Serge E. Hallyn" , "linux-kernel@vger.kernel.org" , "linux-integrity@vger.kernel.org" , "linux-security-module@vger.kernel.org" , "kexec@lists.infradead.org" Subject: Re: [PATCH v5] x86/kexec: Carry forward IMA measurement log on kexec Message-ID: References: <60813f86e960d12ed3738531a14382769a061a02.camel@linux.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <60813f86e960d12ed3738531a14382769a061a02.camel@linux.ibm.com> X-Scanned-By: MIMEDefang 2.85 on 10.11.54.7 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220615_195938_925980_0C29F733 X-CRM114-Status: GOOD ( 35.15 ) X-BeenThere: kexec@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+kexec=archiver.kernel.org@lists.infradead.org On 06/13/22 at 05:01pm, Mimi Zohar wrote: > On Mon, 2022-06-13 at 10:30 +0000, Jonathan McDowell wrote: > > On kexec file load Integrity Measurement Architecture (IMA) subsystem > > may verify the IMA signature of the kernel and initramfs, and measure > > it. The command line parameters passed to the kernel in the kexec call > > may also be measured by IMA. A remote attestation service can verify > > a TPM quote based on the TPM event log, the IMA measurement list, and > > the TPM PCR data. This can be achieved only if the IMA measurement log > > is carried over from the current kernel to the next kernel across > > the kexec call. > > > > powerpc and ARM64 both achieve this using device tree with a > > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of > > device tree, so use the setup_data mechanism to pass the IMA buffer to > > the new kernel. > > > > (Mimi, Baoquan, I haven't included your reviewed-bys because this has > > changed the compile guards around the ima_(free|get)_kexec_buffer > > functions in order to fix the warning the kernel test robot found. I > > think this is the right thing to do and avoids us compiling them on > > platforms where they won't be used. The alternative would be to drop > > the guards in ima.h that Mimi requested for v4.)hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh > > > > Signed-off-by: Jonathan McDowell > > --- > > v5: > > - Guard ima_(free|get)_kexec_buffer functions with > > CONFIG_HAVE_IMA_KEXEC (kernel test robot) > > - Use setup_data_offset in setup_boot_parameters and update rather than > > calculating in call to setup_ima_state. > > v4: > > - Guard ima.h function prototypes with CONFIG_HAVE_IMA_KEXEC > > > diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c > > index 8d374cc552be..42a6c5721a43 100644 > > --- a/drivers/of/kexec.c > > +++ b/drivers/of/kexec.c > > @@ -9,6 +9,7 @@ > > * Copyright (C) 2016 IBM Corporation > > */ > > > > +#include > > #include > > #include > > #include > > @@ -115,6 +116,7 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr, > > return 0; > > } > > > > +#ifdef CONFIG_HAVE_IMA_KEXEC > > /** > > * ima_get_kexec_buffer - get IMA buffer from the previous kernel > > * @addr: On successful return, set to point to the buffer contents. > > @@ -173,6 +175,7 @@ int ima_free_kexec_buffer(void) > > > > return memblock_phys_free(addr, size); > > } > > +#endif > > Inside ima_{get,free}_kexec_buffer(), there's no need now to test > whether CONFIG_HAVE_IMA_KEXEC is enabled. > > if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC)) > return -ENOTSUPP; Indeed. The #ifdef added by Jonathan is redundant. Not sure if the original IS_ENABLED(CONFIG_HAVE_IMA_KEXEC) checking inside ima_{get,free}_kexec_buffer() is intended. I ever reviewed below patch, the IS_ENABLED(CONFIG_XX) inside static function will make the function compiled, and will be optimized out if the CONFIG_XX is not enabled. However, it only has effect on static function. Here, ima_{get,free}_kexec_buffer() is not static, likely we should remove the inside IS_ENABLED() checking. commit 4ece09be9913a87ff99ea347fd7e7adad5bdbc8f Author: Jisheng Zhang Date: Wed Mar 23 16:06:39 2022 -0700 x86/setup: use IS_ENABLED(CONFIG_KEXEC_CORE) instead of #ifdef Replace the conditional compilation using "#ifdef CONFIG_KEXEC_CORE" by a check for "IS_ENABLED(CONFIG_KEXEC_CORE)", to simplify the code and increase compile coverage. Other than this one Mimi pointed out, this patch looks good to me, thx. Reviewed-by: Baoquan He > > > > > > /** > > * remove_ima_buffer - remove the IMA buffer property and reservation from @fdt > > diff --git a/include/linux/ima.h b/include/linux/ima.h > > _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec