Re: [PATCHv2 2/3] kexec: Introduce UKI image parser

[Simon Horman <horms@kernel.org>]

On Wed, Oct 16, 2024 at 07:34:14PM +0800, Pingfan Liu wrote:
> A UKI image is a PE file that consists of several sections, typically
> including: .text, .data, .linux, .initrd, .cmdline, and others.
> 
> The kernel image is stored in the .linux section, which is one of the
> formats currently recognized by kexec-tools. Therefore, the UKI parser
> can be used to strip away the UKI layer, allowing the other parser to
> continue the procession of the kernel image.
> 
> Signed-off-by: Pingfan Liu <piliu@redhat.com>

...

> diff --git a/kexec/kexec-uki.c b/kexec/kexec-uki.c

...

> +int uki_image_probe(const char *file_buf, off_t buf_sz)
> +{
> +	struct pe_hdr *pe_hdr;
> +	struct pe32plus_opt_hdr *opt_hdr;
> +	struct section_header *sect_hdr;
> +	int pe_hdr_offset, section_nr, linux_sz = -1;
> +	char *pe_part_buf, *linux_src;
> +	char *initrd_fname = NULL;
> +	int initrd_fd = -1;
> +
> +	pe_hdr_offset = get_pehdr_offset(file_buf);
> +	pe_part_buf = (char *)file_buf + pe_hdr_offset;
> +	pe_hdr = (struct pe_hdr *)pe_part_buf;
> +	if (pe_hdr->opt_hdr_size == 0) {
> +		printf("ERR: optional header is missing\n");
> +		return -1;
> +	}
> +	section_nr = pe_hdr->sections;
> +	opt_hdr = (struct pe32plus_opt_hdr *)(pe_part_buf + sizeof(struct pe_hdr));
> +	sect_hdr = (struct section_header *)((char *)opt_hdr + pe_hdr->opt_hdr_size);
> +
> +	for (int i = 0; i < section_nr; i++) {
> +		if (!strcmp(sect_hdr->name, UKI_LINUX_SECTION)) {
> +			/* data_addr is relative to the whole file */
> +			linux_src = (char *)file_buf + sect_hdr->data_addr;
> +			linux_sz = sect_hdr->raw_data_size;
> +
> +		} else if (!strcmp(sect_hdr->name, UKI_INITRD_SECTION)) {
> +			if (!(initrd_fname = strdup(FILENAME_UKI_INITRD))) {
> +				dbgprintf("%s: Can't duplicate strings\n", __func__);
> +				goto next;
> +			}
> +
> +			if ((initrd_fd = mkstemp(initrd_fname)) < 0) {
> +				dbgprintf("%s: Can't open file %s\n", __func__,	initrd_fname);
> +				goto next;
> +			}
> +
> +			if (write(initrd_fd, (char *)file_buf + sect_hdr->data_addr,
> +					sect_hdr->raw_data_size) != sect_hdr->raw_data_size) {
> +				dbgprintf("%s: Can't write the compressed file %s\n",
> +						__func__, initrd_fname);
> +				goto next;

I plan to apply this series, but initrd_fd appears to be leaked here.
Could you send a follow-up patch to address that? Perhaps like this:

			err = write(...);
			close(initrd_fd)
			if (err != sect_hdr->raw_data_size) {
				...
			} else {
				...
			}

Thanks!

> +			} else {
> +				implicit_initrd_fd = open(initrd_fname, O_RDONLY);
> +				close(initrd_fd);
> +			}
> +		}
> +next:
> +		sect_hdr++;
> +	}

...

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec