Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 18 Apr 2023 19:52:09 +0200
From:   Simon Horman <horms@kernel.org>
To:     Stefan Berger <stefanb@linux.ibm.com>
Cc:     kexec@lists.infradead.org, devicetree@vger.kernel.org,
        linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
        linuxppc-dev@lists.ozlabs.org, nayna@linux.ibm.com,
        nasastry@in.ibm.com, mpe@ellerman.id.au,
        Rob Herring <robh+dt@kernel.org>,
        Frank Rowand <frowand.list@gmail.com>,
        Eric Biederman <ebiederm@xmission.com>,
        Nageswara R Sastry <rnsastry@linux.ibm.com>,
        Coiby Xu <coxu@redhat.com>, Rob Herring <robh@kernel.org>
Subject: Re: [PATCH v9 4/4] tpm/kexec: Duplicate TPM measurement log in
 of-tree for kexec
Message-ID: <ZD7YybmnioAs0yKR@kernel.org>
References: <20230418134409.177485-1-stefanb@linux.ibm.com>
 <20230418134409.177485-5-stefanb@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230418134409.177485-5-stefanb@linux.ibm.com>
Precedence: bulk

On Tue, Apr 18, 2023 at 09:44:09AM -0400, Stefan Berger wrote:
> The memory area of the TPM measurement log is currently not properly
> duplicated for carrying it across kexec when an Open Firmware
> Devicetree is used. Therefore, the contents of the log get corrupted.
> Fix this for the kexec_file_load() syscall by allocating a buffer and
> copying the contents of the existing log into it. The new buffer is
> preserved across the kexec and a pointer to it is available when the new
> kernel is started. To achieve this, store the allocated buffer's address
> in the flattened device tree (fdt) under the name linux,tpm-kexec-buffer
> and search for this entry early in the kernel startup before the TPM
> subsystem starts up. Adjust the pointer in the of-tree stored under
> linux,sml-base to point to this buffer holding the preserved log. The TPM
> driver can then read the base address from this entry when making the log
> available. Invalidate the log by removing 'linux,sml-base' from the
> devicetree if anything goes wrong with updating the buffer.
> 
> Use subsys_initcall() to call the function to restore the buffer even if
> the TPM subsystem or driver are not used. This allows the buffer to be
> carried across the next kexec without involvement of the TPM subsystem
> and ensures a valid buffer pointed to by the of-tree.

Hi Stefan,

some minor feedback from my side.

> Use the subsys_initcall(), rather than an ealier initcall, since

nit via checkpatch.pl --codespell: s/ealier/earlier/

> page_is_ram() in get_kexec_buffer() only starts working at this stage.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> Tested-by: Coiby Xu <coxu@redhat.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

...

> +void tpm_add_kexec_buffer(struct kimage *image)
> +{
> +	struct kexec_buf kbuf = { .image = image, .buf_align = 1,
> +				  .buf_min = 0, .buf_max = ULONG_MAX,
> +				  .top_down = true };
> +	struct device_node *np;
> +	void *buffer;
> +	u32 size;
> +	u64 base;
> +	int ret;
> +
> +	if (!IS_ENABLED(CONFIG_PPC64))
> +		return;
> +
> +	np = of_find_node_by_name(NULL, "vtpm");
> +	if (!np)
> +		return;
> +
> +	if (of_tpm_get_sml_parameters(np, &base, &size) < 0)
> +		return;
> +
> +	buffer = vmalloc(size);
> +	if (!buffer)
> +		return;
> +	memcpy(buffer, __va(base), size);
> +
> +	kbuf.buffer = buffer;
> +	kbuf.bufsz = size;
> +	kbuf.memsz = size;
> +	ret = kexec_add_buffer(&kbuf);
> +	if (ret) {
> +		pr_err("Error passing over kexec TPM measurement log buffer: %d\n",
> +		       ret);

Does buffer need to be freed here?

> +		return;
> +	}
> +
> +	image->tpm_buffer = buffer;
> +	image->tpm_buffer_addr = kbuf.mem;
> +	image->tpm_buffer_size = size;
> +}