linux-kernel - Re: [PATCH 3/6] LoongArch/kexec

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]
Message-ID: <aJredf2InXhHY7ek@pie>
Date: Tue, 12 Aug 2025 06:25:57 +0000
From: Yao Zi <ziyao@...root.org>
To: Youling Tang <youling.tang@...ux.dev>,
	Huacai Chen <chenhuacai@...nel.org>
Cc: WANG Xuerui <kernel@...0n.name>, Baoquan He <bhe@...hat.com>,
	kexec@...ts.infradead.org, loongarch@...ts.linux.dev,
	linux-kernel@...r.kernel.org, Youling Tang <tangyouling@...inos.cn>
Subject: Re: [PATCH 3/6] LoongArch/kexec_file: Add initrd loading

On Tue, Aug 12, 2025 at 12:05:30PM +0800, Youling Tang wrote:
> Hi, Yao
> On 2025/8/12 01:58, Yao Zi wrote:
> > On Mon, Aug 11, 2025 at 05:26:56PM +0800, Youling Tang wrote:
> > > From: Youling Tang <tangyouling@...inos.cn>
> > > 
> > > Add inird loading support and pass it to the second kernel via the
> > > cmdline 'initrd=start,size'.
> > I think This won't work if the exec'ed kernel enables
> > CONFIG_CMDLINE_FORCE. Is it possible to mimic libstub's behavior of
> > installing a configuration table LINUX_EFI_INITRD_MEDIA_GUID?
> The command line passed by kexec to the second kernel has no effect if
> CONFIG_CMDLINE_FORCE is enabled, which is not quite suitable for the
> kexec scenario.
> 
> Currently, the initrd, elfcorehdr, and mem parameters will all be passed
> through the command line to maintain consistency with the implementation
> behavior of kexec-tools. It is possible that the content of systab will
> be modified in the future and some parts will be integrated into systab
> (the current cmdline mode will be better compatible with the elf kernel).
> > 
> > > Signed-off-by: Youling Tang <tangyouling@...inos.cn>
> > > ---
> > >   arch/loongarch/kernel/machine_kexec_file.c | 71 ++++++++++++++++++++++
> > >   1 file changed, 71 insertions(+)
> > > 
> > > diff --git a/arch/loongarch/kernel/machine_kexec_file.c b/arch/loongarch/kernel/machine_kexec_file.c
> > > index bc91ae0afa4c..e1240644f529 100644
> > > --- a/arch/loongarch/kernel/machine_kexec_file.c
> > > +++ b/arch/loongarch/kernel/machine_kexec_file.c
> > > @@ -34,13 +34,84 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
> > >   	return kexec_image_post_load_cleanup_default(image);
> > >   }
> > > +/* Adds the "initrd=start,size" command line parameter to command line. */
> > > +static void cmdline_add_initrd(struct kimage *image, unsigned long *cmdline_tmplen,
> > > +				char *modified_cmdline, unsigned long initrd)
> > > +{
> > > +	int initrd_strlen;
> > > +
> > > +	initrd_strlen = sprintf(modified_cmdline + (*cmdline_tmplen), "initrd=0x%lx,0x%lx ",
> > modified_cmdline is allocated as COMMAND_LINE_SIZE bytes, thus I think
> > it's possible to overflow the buffer.
> At this point, modified_cmdline can clearly know that it only stores
> the additional commands we add (initrd,mem,elfcorehdr), and will not
> exceed COMMAND_LINE_SIZE.
> > 
> > > +		initrd, image->initrd_buf_len);
> > > +	*cmdline_tmplen += initrd_strlen;
> > > +}
> > > +
> > > +/*
> > > + * Tries to add the initrd to the image. If it is not possible to find
> > > + * valid locations, this function will undo changes to the image and return non
> > > + * zero.
> > > + */
> > >   int load_other_segments(struct kimage *image,
> > >   			unsigned long kernel_load_addr,
> > >   			unsigned long kernel_size,
> > >   			char *initrd, unsigned long initrd_len,
> > >   			char *cmdline, unsigned long cmdline_len)
> > >   {
> > > +	struct kexec_buf kbuf;
> > > +	unsigned long orig_segments = image->nr_segments;
> > > +	char *modified_cmdline = NULL;
> > > +	unsigned long cmdline_tmplen = 0;
> > > +	unsigned long initrd_load_addr = 0;
> > > +	int ret = 0;
> > > +
> > > +
> > > +	kbuf.image = image;
> > > +	/* not allocate anything below the kernel */
> > > +	kbuf.buf_min = kernel_load_addr + kernel_size;
> > > +
> > > +	modified_cmdline = kzalloc(COMMAND_LINE_SIZE, GFP_KERNEL);
> > > +	if (!modified_cmdline)
> > > +		return -EINVAL;
> > > +
> > > +	/* Ensure it's nul terminated */
> > > +	modified_cmdline[COMMAND_LINE_SIZE - 1] = '\0';
> > > +
> > > +	/* load initrd */
> > > +	if (initrd) {
> > > +		kbuf.buffer = initrd;
> > > +		kbuf.bufsz = initrd_len;
> > > +		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> > > +		kbuf.memsz = initrd_len;
> > > +		kbuf.buf_align = 0;
> > > +		/* within 1GB-aligned window of up to 32GB in size */
> > > +		kbuf.buf_max = round_down(kernel_load_addr, SZ_1G)
> > > +						+ (unsigned long)SZ_1G * 32;
> > > +		kbuf.top_down = false;
> > > +
> > > +		ret = kexec_add_buffer(&kbuf);
> > > +		if (ret)
> > > +			goto out_err;
> > > +		initrd_load_addr = kbuf.mem;
> > > +
> > > +		kexec_dprintk("Loaded initrd at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> > > +			      initrd_load_addr, kbuf.bufsz, kbuf.memsz);
> > > +
> > > +		/* Add the initrd=start,size parameter to the command line */
> > > +		cmdline_add_initrd(image, &cmdline_tmplen, modified_cmdline, initrd_load_addr);
> > > +	}
> > > +
> > > +	if (cmdline_len + cmdline_tmplen > COMMAND_LINE_SIZE) {
> > It's too later to check for overflowing here, where the data after
> > modified_cmdline may already be overwritten.
> At this point, we append the original command line to modified_cmdline,
> so it is appropriate to determine whether the command line length exceeds
> the limit.

Thanks, I misunderstood the code logic: before cmdline_add_initrd
appends new arguments, modified_cmdline is empty instead of containing
a copy of cmdline, so this is fine. Sorry for the noise.

Best regards,
Yao Zi