Hi Simon,
Thanks for your review.
On Mon, Oct 21, 2024 at 6:16 PM Simon Horman <horms@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx>
>
> ...
>
> > 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:
>
I have sent out a dedicated v3 "[PATCHv3 2/3] kexec: Introduce UKI
image parser" to fix this leak issue.
> err = write(...);
> close(initrd_fd)
My fix duplicates the close(initrd_fd) in each branch. Since initrd_fd
is the only reference to mkstemp(initrd_fname), I think if closing it,
the temp file may disappear before implicit_initrd_fd=
open(initrd_fname, O_RDONLY).
Best Regards,
Pingfan
> 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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
