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 12:19:39 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Łukasz Stelmach <l.stelmach@...sung.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

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

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

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).

> 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);

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

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

Thanks,
Mark.