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 8859AC43334 for ; Thu, 16 Jun 2022 02:59:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1354540AbiFPC7i (ORCPT ); Wed, 15 Jun 2022 22:59:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59492 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1348954AbiFPC7i (ORCPT ); Wed, 15 Jun 2022 22:59:38 -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 729875A17A for ; Wed, 15 Jun 2022 19:59:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1655348376; 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=N1ekUpYOkbZnpHthobxo7TijCae/IuzIpd6MBeryTaYhQR498q14nBSVKWyKcZ2VieRC3p NJaTJz8KkGHMSn7iGPzkMHQr8JawB5h1gafkgaE+xA4B4DeIAuG8HKB8U5l8nVBw1804F1 CCgPVNI0878kLnqCAUGhWzG44qcWr/8= 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: 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 > >