From 5ac70383c8b32eeec80426e837960977971c7c2b Mon Sep 17 00:00:00 2001 From: Chris Rees Date: Mon, 15 Feb 2021 11:37:06 +0000 Subject: [PATCH] elftoolchain: stop leaving tempfiles on error Temporary files were not cleaned up, resulting in $TMPDIR or even the current directory becoming littered with ecp.* files. This happened with error and even sometimes on success! Approved by: dim MFC after: 4 weeks Accepted upstream: https://sourceforge.net/p/elftoolchain/code/3918/ Differential Revision: https://reviews.freebsd.org/D28651 --- contrib/elftoolchain/elfcopy/archive.c | 31 ++++++++--- contrib/elftoolchain/elfcopy/elfcopy.h | 1 + contrib/elftoolchain/elfcopy/main.c | 77 +++++++++++++++++++++----- 3 files changed, 86 insertions(+), 23 deletions(-) diff --git a/contrib/elftoolchain/elfcopy/archive.c b/contrib/elftoolchain/elfcopy/archive.c index 9e23b831fdca..e57caad58e35 100644 --- a/contrib/elftoolchain/elfcopy/archive.c +++ b/contrib/elftoolchain/elfcopy/archive.c @@ -69,9 +69,11 @@ process_ar_obj(struct elfcopy *ecp, struct ar_obj *obj) /* Output to a temporary file. */ create_tempfile(NULL, &tempfile, &fd); - if ((ecp->eout = elf_begin(fd, ELF_C_WRITE, NULL)) == NULL) + if ((ecp->eout = elf_begin(fd, ELF_C_WRITE, NULL)) == NULL) { + cleanup_tempfile(tempfile); errx(EXIT_FAILURE, "elf_begin() failed: %s", elf_errmsg(-1)); + } elf_flagelf(ecp->eout, ELF_C_SET, ELF_F_LAYOUT); create_elf(ecp); elf_end(ecp->ein); @@ -80,27 +82,40 @@ process_ar_obj(struct elfcopy *ecp, struct ar_obj *obj) obj->buf = NULL; /* Extract archive symbols. */ - if (lseek(fd, 0, SEEK_SET) < 0) + if (lseek(fd, 0, SEEK_SET) < 0) { + cleanup_tempfile(tempfile); err(EXIT_FAILURE, "lseek failed for '%s'", tempfile); - if ((ecp->eout = elf_begin(fd, ELF_C_READ, NULL)) == NULL) + } + if ((ecp->eout = elf_begin(fd, ELF_C_READ, NULL)) == NULL) { + cleanup_tempfile(tempfile); errx(EXIT_FAILURE, "elf_begin() failed: %s", elf_errmsg(-1)); + } extract_arsym(ecp); elf_end(ecp->eout); - if (fstat(fd, &sb) == -1) + if (fstat(fd, &sb) == -1) { + cleanup_tempfile(tempfile); err(EXIT_FAILURE, "fstat %s failed", tempfile); - if (lseek(fd, 0, SEEK_SET) < 0) + } + if (lseek(fd, 0, SEEK_SET) < 0) { + cleanup_tempfile(tempfile); err(EXIT_FAILURE, "lseek %s failed", tempfile); + } obj->size = sb.st_size; - if ((obj->maddr = malloc(obj->size)) == NULL) + if ((obj->maddr = malloc(obj->size)) == NULL) { + cleanup_tempfile(tempfile); err(EXIT_FAILURE, "memory allocation failed for '%s'", tempfile); - if ((size_t) read(fd, obj->maddr, obj->size) != obj->size) + } + if ((size_t) read(fd, obj->maddr, obj->size) != obj->size) { + cleanup_tempfile(tempfile); err(EXIT_FAILURE, "read failed for '%s'", tempfile); - if (unlink(tempfile)) + } + if (cleanup_tempfile(tempfile) < 0) err(EXIT_FAILURE, "unlink %s failed", tempfile); free(tempfile); + tempfile = NULL; close(fd); if (strlen(obj->name) > _MAXNAMELEN_SVR4) add_to_ar_str_table(ecp, obj->name); diff --git a/contrib/elftoolchain/elfcopy/elfcopy.h b/contrib/elftoolchain/elfcopy/elfcopy.h index b8845a574428..17d8b803156d 100644 --- a/contrib/elftoolchain/elfcopy/elfcopy.h +++ b/contrib/elftoolchain/elfcopy/elfcopy.h @@ -277,6 +277,7 @@ void add_to_symtab(struct elfcopy *_ecp, const char *_name, unsigned char _st_info, unsigned char _st_other, int _ndx_known); int add_to_inseg_list(struct elfcopy *_ecp, struct section *_sec); void adjust_addr(struct elfcopy *_ecp); +int cleanup_tempfile(char *_fn); void copy_content(struct elfcopy *_ecp); void copy_data(struct section *_s); void copy_phdr(struct elfcopy *_ecp); diff --git a/contrib/elftoolchain/elfcopy/main.c b/contrib/elftoolchain/elfcopy/main.c index c02bb296c4a3..964d3358de60 100644 --- a/contrib/elftoolchain/elfcopy/main.c +++ b/contrib/elftoolchain/elfcopy/main.c @@ -510,6 +510,27 @@ free_elf(struct elfcopy *ecp) } } +/* + * Remove a temporary file, without freeing its filename. + * + * Safe to pass NULL, will just ignore it. + */ +int +cleanup_tempfile(char *fn) +{ + int errno_save, retval; + + if (fn == NULL) + return 0; + errno_save = errno; + if ((retval = unlink(fn)) < 0) { + warn("unlink tempfile %s failed", fn); + errno = errno_save; + return retval; + } + return 0; +} + /* Create a temporary file. */ void create_tempfile(const char *src, char **fn, int *fd) @@ -656,8 +677,10 @@ create_file(struct elfcopy *ecp, const char *src, const char *dst) } #endif - if (lseek(ifd, 0, SEEK_SET) < 0) + if (lseek(ifd, 0, SEEK_SET) < 0) { + cleanup_tempfile(tempfile); err(EXIT_FAILURE, "lseek failed"); + } /* * If input object is not ELF file, convert it to an intermediate @@ -677,9 +700,12 @@ create_file(struct elfcopy *ecp, const char *src, const char *dst) ecp->oed = ELFDATA2LSB; } create_tempfile(src, &elftemp, &efd); - if ((ecp->eout = elf_begin(efd, ELF_C_WRITE, NULL)) == NULL) + if ((ecp->eout = elf_begin(efd, ELF_C_WRITE, NULL)) == NULL) { + cleanup_tempfile(elftemp); + cleanup_tempfile(tempfile); errx(EXIT_FAILURE, "elf_begin() failed: %s", elf_errmsg(-1)); + } elf_flagelf(ecp->eout, ELF_C_SET, ELF_F_LAYOUT); if (ecp->itf == ETF_BINARY) create_elf_from_binary(ecp, ifd, src); @@ -687,31 +713,45 @@ create_file(struct elfcopy *ecp, const char *src, const char *dst) create_elf_from_ihex(ecp, ifd); else if (ecp->itf == ETF_SREC) create_elf_from_srec(ecp, ifd); - else + else { + cleanup_tempfile(elftemp); + cleanup_tempfile(tempfile); errx(EXIT_FAILURE, "Internal: invalid target flavour"); + } elf_end(ecp->eout); /* Open intermediate ELF object as new input object. */ close(ifd); - if ((ifd = open(elftemp, O_RDONLY)) == -1) + if ((ifd = open(elftemp, O_RDONLY)) == -1) { + cleanup_tempfile(elftemp); + cleanup_tempfile(tempfile); err(EXIT_FAILURE, "open %s failed", src); + } close(efd); - if (unlink(elftemp) < 0) + if (cleanup_tempfile(elftemp) < 0) { + cleanup_tempfile(tempfile); err(EXIT_FAILURE, "unlink %s failed", elftemp); + } free(elftemp); + elftemp = NULL; } - if ((ecp->ein = elf_begin(ifd, ELF_C_READ, NULL)) == NULL) + if ((ecp->ein = elf_begin(ifd, ELF_C_READ, NULL)) == NULL) { + cleanup_tempfile(tempfile); errx(EXIT_FAILURE, "elf_begin() failed: %s", elf_errmsg(-1)); + } switch (elf_kind(ecp->ein)) { case ELF_K_NONE: + cleanup_tempfile(tempfile); errx(EXIT_FAILURE, "file format not recognized"); case ELF_K_ELF: - if ((ecp->eout = elf_begin(ofd, ELF_C_WRITE, NULL)) == NULL) + if ((ecp->eout = elf_begin(ofd, ELF_C_WRITE, NULL)) == NULL) { + cleanup_tempfile(tempfile); errx(EXIT_FAILURE, "elf_begin() failed: %s", elf_errmsg(-1)); + } /* elfcopy(1) manage ELF layout by itself. */ elf_flagelf(ecp->eout, ELF_C_SET, ELF_F_LAYOUT); @@ -730,21 +770,21 @@ create_file(struct elfcopy *ecp, const char *src, const char *dst) * Create (another) tempfile for binary/srec/ihex * output object. */ - if (tempfile != NULL) { - if (unlink(tempfile) < 0) - err(EXIT_FAILURE, "unlink %s failed", - tempfile); - free(tempfile); - } + if (cleanup_tempfile(tempfile) < 0) + errx(EXIT_FAILURE, "unlink %s failed", + tempfile); + free(tempfile); create_tempfile(src, &tempfile, &ofd0); /* * Rewind the file descriptor being processed. */ - if (lseek(ofd, 0, SEEK_SET) < 0) + if (lseek(ofd, 0, SEEK_SET) < 0) { + cleanup_tempfile(tempfile); err(EXIT_FAILURE, "lseek failed for the output object"); + } /* * Call flavour-specific conversion routine. @@ -765,11 +805,13 @@ create_file(struct elfcopy *ecp, const char *src, const char *dst) #if WITH_PE create_pe(ecp, ofd, ofd0); #else + cleanup_tempfile(tempfile); errx(EXIT_FAILURE, "PE/EFI support not enabled" " at compile time"); #endif break; default: + cleanup_tempfile(tempfile); errx(EXIT_FAILURE, "Internal: unsupported" " output flavour %d", ecp->oec); } @@ -784,6 +826,7 @@ create_file(struct elfcopy *ecp, const char *src, const char *dst) /* XXX: Not yet supported. */ break; default: + cleanup_tempfile(tempfile); errx(EXIT_FAILURE, "file format not supported"); } @@ -802,9 +845,13 @@ create_file(struct elfcopy *ecp, const char *src, const char *dst) in_place = 1; } - if (copy_from_tempfile(tempfile, dst, ofd, &tfd, in_place) < 0) + if (copy_from_tempfile(tempfile, dst, ofd, + &tfd, in_place) < 0) { + cleanup_tempfile(tempfile); err(EXIT_FAILURE, "creation of %s failed", dst); + } + /* 'tempfile' has been removed by copy_from_tempfile(). */ free(tempfile); tempfile = NULL;