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: <0767b8fe-7c04-4e73-9235-ee326ee058cc@linux.dev>
Date: Tue, 12 Aug 2025 12:05:30 +0800
From: Youling Tang <youling.tang@...ux.dev>
To: Yao Zi <ziyao@...root.org>, 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

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.
>
>> +		pr_err("Appending kdump cmdline exceeds cmdline size\n");
> I think load_other_segments could be invoked without kdump involved. If
> that's correct, this message is inaccurate.
Yes, it should be corrected.


Thanks,
Youling.
>
>> +		ret = -EINVAL;
>> +		goto out_err;
>> +	}
> Regards,
> Yao Zi
>
>> +	memcpy(modified_cmdline + cmdline_tmplen, cmdline, cmdline_len);
>> +	cmdline = modified_cmdline;
>>   	image->arch.cmdline_ptr = (unsigned long)cmdline;
>>   
>>   	return 0;
>> +
>> +out_err:
>> +	image->nr_segments = orig_segments;
>> +	kfree(modified_cmdline);
>> +	return ret;
>>   }
>> -- 
>> 2.34.1
>>
>>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ