linux-kernel - Re: [PATCH v5 3/3] arm64: Add IMA kexec buffer to DTB

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] [day] [month] [year] [list]

Message-ID: <35e3e3b0-541c-8a93-8c2f-b7659090d41f@linux.microsoft.com>
Date:   Thu, 3 Sep 2020 21:45:04 -0700
From:   Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>
To:     Thiago Jung Bauermann <bauerman@...ux.ibm.com>
Cc:     zohar@...ux.ibm.com, robh@...nel.org, gregkh@...uxfoundation.org,
        james.morse@....com, catalin.marinas@....com, sashal@...nel.org,
        will@...nel.org, mpe@...erman.id.au, benh@...nel.crashing.org,
        paulus@...ba.org, robh+dt@...nel.org, frowand.list@...il.com,
        vincenzo.frascino@....com, mark.rutland@....com,
        dmitry.kasatkin@...il.com, jmorris@...ei.org, serge@...lyn.com,
        pasha.tatashin@...een.com, allison@...utok.net,
        kstewart@...uxfoundation.org, takahiro.akashi@...aro.org,
        tglx@...utronix.de, masahiroy@...nel.org, bhsharma@...hat.com,
        mbrugger@...e.com, hsinyi@...omium.org, tao.li@...o.com,
        christophe.leroy@....fr, linux-integrity@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        prsriva@...ux.microsoft.com, balajib@...ux.microsoft.com
Subject: Re: [PATCH v5 3/3] arm64: Add IMA kexec buffer to DTB

On 9/3/20 3:11 PM, Thiago Jung Bauermann wrote:
> 
> Lakshmi Ramasubramanian <nramas@...ux.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@...ux.microsoft.com>
>> Signed-off-by: Prakhar Srivastava <prsriva@...ux.microsoft.com>
>> Signed-off-by: Lakshmi Ramasubramanian <nramas@...ux.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