linux-kernel - Re: [PATCH] arm64: kexec_file: print appropriate variable

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]

Date:   Thu, 30 Apr 2020 13:49:40 +0200
From:   Lukasz Stelmach <l.stelmach@...sung.com>
To:     Mark Rutland <mark.rutland@....com>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        AKASHI Takahiro <takahiro.akashi@...aro.org>,
        James Morse <james.morse@....com>
Subject: Re: [PATCH] arm64: kexec_file: print appropriate variable

It was <2020-04-30 czw 12:19>, when Mark Rutland wrote:
> On Thu, Apr 30, 2020 at 12:50:34PM +0200, Łukasz Stelmach wrote:
>> Fixes: 4312057681929 ("arm64: kexec_file: load initrd and device-tree")
>
> This commit ID is bogus (doesn't exist in mainline or the arm64 tree).
>
> The upstream commit ID seems to be: 52b2a8af7436044cfcb27e4b0f72c2ce1f3890da

Fixing.

> As will said, this needs a commit message. Please explain what you think
> is wrong here.

Fixing.

> Also, when sending a fix, *please* Cc the author of the original patch.
>
> I've added parties relevant to the original patch (Takahiro and James).

Thank you.

>> Signed-off-by: Łukasz Stelmach <l.stelmach@...sung.com>
>> ---
>>  arch/arm64/kernel/machine_kexec_file.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
>> index b40c3b0def92..2776bdaa83a5 100644
>> --- a/arch/arm64/kernel/machine_kexec_file.c
>> +++ b/arch/arm64/kernel/machine_kexec_file.c
>> @@ -332,7 +332,7 @@ int load_other_segments(struct kimage *image,
>>  	image->arch.dtb_mem = kbuf.mem;
>>  
>>  	pr_debug("Loaded dtb at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
>> -			kbuf.mem, dtb_len, dtb_len);
>> +			kbuf.mem, dtb_len, kbuf.memsz);
>
> It's worth noting that we follow the same pattern repeatedly in this
> file, so if you think this instance is wrong you should consider whether
> the others are correct.
>
> Earlier in this file we have:
>
> |	pr_debug("Loaded elf core header at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> |		 image->arch.elf_headers_mem, headers_sz, headers_sz)
>
> |	pr_debug("Loaded initrd at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> |		 initrd_load_addr, initrd_len, initrd_len);

Fixing.

> ... and it looks like x86 does similar in kexec-bzimage64.c, for some
> sort of consistency with the old kexec logging.

When I fix it for x86, should I combine changes in one patch or prepare
two separate patches?

> If <foo>_len and kbuf.memsz can differ, we should log that in all cases.
> If not, we should remove the redundant logging.

Yes, memsz is page-aligned in kexec_add_buffer();

Kind regards,
-- 
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics

Download attachment "signature.asc" of type "application/pgp-signature" (488 bytes)