Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v5 3/3] arm64: Add IMA kexec buffer to DTB
To:     Thiago Jung Bauermann <bauerman@linux.ibm.com>
Cc:     zohar@linux.ibm.com, robh@kernel.org, gregkh@linuxfoundation.org,
        james.morse@arm.com, catalin.marinas@arm.com, sashal@kernel.org,
        will@kernel.org, mpe@ellerman.id.au, benh@kernel.crashing.org,
        paulus@samba.org, robh+dt@kernel.org, frowand.list@gmail.com,
        vincenzo.frascino@arm.com, mark.rutland@arm.com,
        dmitry.kasatkin@gmail.com, jmorris@namei.org, serge@hallyn.com,
        pasha.tatashin@soleen.com, allison@lohutok.net,
        kstewart@linuxfoundation.org, takahiro.akashi@linaro.org,
        tglx@linutronix.de, masahiroy@kernel.org, bhsharma@redhat.com,
        mbrugger@suse.com, hsinyi@chromium.org, tao.li@vivo.com,
        christophe.leroy@c-s.fr, linux-integrity@vger.kernel.org,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        prsriva@linux.microsoft.com, balajib@linux.microsoft.com
References: <20200901195029.30039-1-nramas@linux.microsoft.com>
 <20200901195029.30039-4-nramas@linux.microsoft.com>
 <87sgbyy0gr.fsf@morokweng.localdomain>
From:   Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Message-ID: <35e3e3b0-541c-8a93-8c2f-b7659090d41f@linux.microsoft.com>
Date:   Thu, 3 Sep 2020 21:45:04 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <87sgbyy0gr.fsf@morokweng.localdomain>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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