linux-kernel - Re: [PATCH v7 3/7] ima: kexec: skip IMA segment validation after kexec soft reboot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <00eeeb8b-cc28-42af-873f-3478cd22fb6e@linux.ibm.com>
Date: Tue, 4 Feb 2025 14:39:07 -0500
From: Stefan Berger <stefanb@...ux.ibm.com>
To: steven chen <chenste@...ux.microsoft.com>, zohar@...ux.ibm.com,
        roberto.sassu@...weicloud.com, roberto.sassu@...wei.com,
        eric.snowberg@...cle.com, ebiederm@...ssion.com, paul@...l-moore.com,
        code@...icks.com, bauermann@...abnow.com,
        linux-integrity@...r.kernel.org, kexec@...ts.infradead.org,
        linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: madvenka@...ux.microsoft.com, nramas@...ux.microsoft.com,
        James.Bottomley@...senPartnership.com
Subject: Re: [PATCH v7 3/7] ima: kexec: skip IMA segment validation after
 kexec soft reboot

On 2/3/25 6:20 PM, steven chen wrote:
> kexec_calculate_store_digests() calculates and stores the digest of the
> segment at kexec_file_load syscall where the IMA segment is also
> allocated.  With this series, the IMA segment will be updated with the
> measurement log at kexec excute stage when soft reboot is initiated.

s/excute/execute

> Therefore, it may fail digest verification in verify_sha256_digest()
> after kexec soft reboot into the new kernel. Therefore, the digest
> calculation/verification of the IMA segment needs to be skipped.
> 
> Skip IMA segment from calculating and storing digest in function

Skip the calculation and storing of the digest of the IMA segment in 
kexec_calculate_store_digests() so that ...


> kexec_calculate_store_digests() so that it is not added to the
> 'purgatory_sha_regions'.
> 
> Since verify_sha256_digest() only verifies 'purgatory_sha_regions',
> no change is needed in verify_sha256_digest() in this context.
> 
> With this change, the IMA segment is not included in the digest
> calculation, storage, and verification.
> 
> Author: Tushar Sugandhi <tusharsu@...ux.microsoft.com>
> Signed-off-by: Tushar Sugandhi <tusharsu@...ux.microsoft.com>
> Signed-off-by: steven chen <chenste@...ux.microsoft.com>
 > --->   include/linux/kexec.h              |  3 +++
>   kernel/kexec_file.c                | 23 +++++++++++++++++++++++
>   security/integrity/ima/ima_kexec.c |  3 +++
>   3 files changed, 29 insertions(+)
> 
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index f8413ea5c8c8..f3246e881ac8 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -362,6 +362,9 @@ struct kimage {
>   
>   	phys_addr_t ima_buffer_addr;
>   	size_t ima_buffer_size;
> +
> +	unsigned long ima_segment_index;
> +	bool is_ima_segment_index_set;
>   #endif
>   
>   	/* Core ELF header buffer */
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 3eedb8c226ad..a3370a0dce20 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -38,6 +38,22 @@ void set_kexec_sig_enforced(void)
>   }
>   #endif
>   
> +#ifdef CONFIG_IMA_KEXEC
> +static bool check_ima_segment_index(struct kimage *image, int i)
> +{
> +	if (image->is_ima_segment_index_set &&
> +			i == image->ima_segment_index)

The 'i =' should be indented under 'image->'.

With these nits fixed:

Reviewed-by: Stefan Berger <stefanb@...ux.ibm.com>

> +		return true;
> +	else
> +		return false;
> +}
> +#else
> +static bool check_ima_segment_index(struct kimage *image, int i)
> +{
> +	return false;
> +}
> +#endif
> +
>   static int kexec_calculate_store_digests(struct kimage *image);
>   
>   /* Maximum size in bytes for kernel/initrd files. */
> @@ -764,6 +780,13 @@ static int kexec_calculate_store_digests(struct kimage *image)
>   		if (ksegment->kbuf == pi->purgatory_buf)
>   			continue;
>   
> +		/*
> +		 * Skip the segment if ima_segment_index is set and matches
> +		 * the current index
> +		 */
> +		if (check_ima_segment_index(image, i))
> +			continue;
> +
>   		ret = crypto_shash_update(desc, ksegment->kbuf,
>   					  ksegment->bufsz);
>   		if (ret)
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index b60a902460e2..283860d20521 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -162,6 +162,7 @@ void ima_add_kexec_buffer(struct kimage *image)
>   	kbuf.buffer = kexec_buffer;
>   	kbuf.bufsz = kexec_buffer_size;
>   	kbuf.memsz = kexec_segment_size;
> +	image->is_ima_segment_index_set = false;
>   	ret = kexec_add_buffer(&kbuf);
>   	if (ret) {
>   		pr_err("Error passing over kexec measurement buffer.\n");
> @@ -172,6 +173,8 @@ void ima_add_kexec_buffer(struct kimage *image)
>   	image->ima_buffer_addr = kbuf.mem;
>   	image->ima_buffer_size = kexec_segment_size;
>   	image->ima_buffer = kexec_buffer;
> +	image->ima_segment_index = image->nr_segments - 1;
> +	image->is_ima_segment_index_set = true;
>   
>   	/*
>   	 * kexec owns kexec_buffer after kexec_add_buffer() is called