diff mbox series

[v5,3/3] arm64: Add IMA kexec buffer to DTB

Message ID 20200901195029.30039-4-nramas@linux.microsoft.com (mailing list archive)
State New, archived
Headers show
Series Carry forward IMA measurement log on kexec on ARM64 | expand

Commit Message

Lakshmi Ramasubramanian Sept. 1, 2020, 7:50 p.m. UTC
The address and size of the current kernel's IMA measurement log
need to be added to the device tree's IMA kexec buffer node for
the log to be carried over to the next kernel on the kexec call.

Add the IMA measurement log buffer properties to the device tree for
ARM64 and reserve the memory for storing the IMA log.
Update CONFIG_KEXEC_FILE to select CONFIG_HAVE_IMA_KEXEC to
indicate that the IMA measurement log information is present in
the device tree.

Co-developed-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 arch/arm64/Kconfig                     |  1 +
 arch/arm64/kernel/machine_kexec_file.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

Comments

Thiago Jung Bauermann Sept. 3, 2020, 10:11 p.m. UTC | #1
Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:

> The address and size of the current kernel's IMA measurement log
> need to be added to the device tree's IMA kexec buffer node for
> the log to be carried over to the next kernel on the kexec call.
>
> Add the IMA measurement log buffer properties to the device tree for
> ARM64 and reserve the memory for storing the IMA log.
> Update CONFIG_KEXEC_FILE to select CONFIG_HAVE_IMA_KEXEC to
> indicate that the IMA measurement log information is present in
> the device tree.
>
> Co-developed-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
> Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
>  arch/arm64/Kconfig                     |  1 +
>  arch/arm64/kernel/machine_kexec_file.c | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 6d232837cbee..9f03c8245e5b 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1077,6 +1077,7 @@ config KEXEC
>  config KEXEC_FILE
>  	bool "kexec file based system call"
>  	select KEXEC_CORE
> +	select HAVE_IMA_KEXEC
>  	help
>  	  This is new version of kexec system call. This system call is
>  	  file based and takes file descriptors as system call argument
> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index 361a1143e09e..0fe3d629eefe 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c
> @@ -136,6 +136,21 @@ static int setup_dtb(struct kimage *image,
>  				FDT_PROP_KASLR_SEED);
>  	}
>  
> +	/* add ima-kexec-buffer */
> +	if (image->arch.ima_buffer_size > 0) {
> +		ret = fdt_appendprop_addrrange(dtb, 0, off,
> +				FDT_PROP_IMA_KEXEC_BUFFER,
> +				image->arch.ima_buffer_addr,
> +				image->arch.ima_buffer_size);
> +		if (ret)
> +			return (ret == -FDT_ERR_NOSPACE ? -ENOMEM : -EINVAL);
> +
> +		ret = fdt_add_mem_rsv(dtb, image->arch.ima_buffer_addr,
> +				      image->arch.ima_buffer_size);
> +		if (ret)
> +			goto out;
> +	}
> +
>  	/* add rng-seed */
>  	if (rng_is_initialized()) {
>  		void *rng_seed;

I just noticed one more thing this code isn't doing compared to the
powerpc version (sorry to bring these issues piecemeal, I didn't realize
this before):

You're not checking whether there already is a device tree property and
corresponding memory reservation for an IMA kexec buffer that the
currently running kernel might have received from a previous kernel. In
that case, this code will do the wrong thing because
fdt_appendprop_addrrange() will append the range to the existing
property, which is not what you want. You'll also have a memory
reservation entry for a stale IMA kexec buffer, which just wastes
memory.

So one thing you need to do, whether or not there's an IMA kexec buffer
to be passed to the next kernel, is to remove any existing
FDT_PROP_IMA_KEXEC_BUFFER property and also its corresponding memory
reservation, so that you avoid accumulating stale memory reservations
for non-existing IMA kexec buffers from previous kexecs.
Lakshmi Ramasubramanian Sept. 4, 2020, 4:45 a.m. UTC | #2
On 9/3/20 3:11 PM, Thiago Jung Bauermann wrote:
> 
> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> 
>> The address and size of the current kernel's IMA measurement log
>> need to be added to the device tree's IMA kexec buffer node for
>> the log to be carried over to the next kernel on the kexec call.
>>
>> Add the IMA measurement log buffer properties to the device tree for
>> ARM64 and reserve the memory for storing the IMA log.
>> Update CONFIG_KEXEC_FILE to select CONFIG_HAVE_IMA_KEXEC to
>> indicate that the IMA measurement log information is present in
>> the device tree.
>>
>> Co-developed-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
>> Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>> ---
>>   arch/arm64/Kconfig                     |  1 +
>>   arch/arm64/kernel/machine_kexec_file.c | 15 +++++++++++++++
>>   2 files changed, 16 insertions(+)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 6d232837cbee..9f03c8245e5b 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -1077,6 +1077,7 @@ config KEXEC
>>   config KEXEC_FILE
>>   	bool "kexec file based system call"
>>   	select KEXEC_CORE
>> +	select HAVE_IMA_KEXEC
>>   	help
>>   	  This is new version of kexec system call. This system call is
>>   	  file based and takes file descriptors as system call argument
>> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
>> index 361a1143e09e..0fe3d629eefe 100644
>> --- a/arch/arm64/kernel/machine_kexec_file.c
>> +++ b/arch/arm64/kernel/machine_kexec_file.c
>> @@ -136,6 +136,21 @@ static int setup_dtb(struct kimage *image,
>>   				FDT_PROP_KASLR_SEED);
>>   	}
>>   
>> +	/* add ima-kexec-buffer */
>> +	if (image->arch.ima_buffer_size > 0) {
>> +		ret = fdt_appendprop_addrrange(dtb, 0, off,
>> +				FDT_PROP_IMA_KEXEC_BUFFER,
>> +				image->arch.ima_buffer_addr,
>> +				image->arch.ima_buffer_size);
>> +		if (ret)
>> +			return (ret == -FDT_ERR_NOSPACE ? -ENOMEM : -EINVAL);
>> +
>> +		ret = fdt_add_mem_rsv(dtb, image->arch.ima_buffer_addr,
>> +				      image->arch.ima_buffer_size);
>> +		if (ret)
>> +			goto out;
>> +	}
>> +
>>   	/* add rng-seed */
>>   	if (rng_is_initialized()) {
>>   		void *rng_seed;
> 
> I just noticed one more thing this code isn't doing compared to the
> powerpc version (sorry to bring these issues piecemeal, I didn't realize
> this before):
> 
> You're not checking whether there already is a device tree property and
> corresponding memory reservation for an IMA kexec buffer that the
> currently running kernel might have received from a previous kernel. In
> that case, this code will do the wrong thing because
> fdt_appendprop_addrrange() will append the range to the existing
> property, which is not what you want. You'll also have a memory
> reservation entry for a stale IMA kexec buffer, which just wastes
> memory.
> 
> So one thing you need to do, whether or not there's an IMA kexec buffer
> to be passed to the next kernel, is to remove any existing
> FDT_PROP_IMA_KEXEC_BUFFER property and also its corresponding memory
> reservation, so that you avoid accumulating stale memory reservations
> for non-existing IMA kexec buffers from previous kexecs.
> 

That's good catch Thiago. Thanks for the feedback. I'll make this change 
and update.

thanks,
  -lakshmi
diff mbox series

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 6d232837cbee..9f03c8245e5b 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1077,6 +1077,7 @@  config KEXEC
 config KEXEC_FILE
 	bool "kexec file based system call"
 	select KEXEC_CORE
+	select HAVE_IMA_KEXEC
 	help
 	  This is new version of kexec system call. This system call is
 	  file based and takes file descriptors as system call argument
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index 361a1143e09e..0fe3d629eefe 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -136,6 +136,21 @@  static int setup_dtb(struct kimage *image,
 				FDT_PROP_KASLR_SEED);
 	}
 
+	/* add ima-kexec-buffer */
+	if (image->arch.ima_buffer_size > 0) {
+		ret = fdt_appendprop_addrrange(dtb, 0, off,
+				FDT_PROP_IMA_KEXEC_BUFFER,
+				image->arch.ima_buffer_addr,
+				image->arch.ima_buffer_size);
+		if (ret)
+			return (ret == -FDT_ERR_NOSPACE ? -ENOMEM : -EINVAL);
+
+		ret = fdt_add_mem_rsv(dtb, image->arch.ima_buffer_addr,
+				      image->arch.ima_buffer_size);
+		if (ret)
+			goto out;
+	}
+
 	/* add rng-seed */
 	if (rng_is_initialized()) {
 		void *rng_seed;