From e29eac41dce1a5e423d739fc47abfd2b2bda22df Mon Sep 17 00:00:00 2001 From: Edward O'Callaghan Date: Tue, 7 Mar 2023 21:59:47 +1100 Subject: futility/: Replace futil_copy_file_or_die() impl Replace shell-script C with actual library calls to copy file content. Don't die, dying is bad. Use '0660' as the default dest file perm mask over the default system umask inherited form the environment applied to the source file permissions. Add error handling so we have a idea what happened. BUG=b:268397597 TEST=`emerge-nissa vboot_reference`. TEST=`cros_run_unit_tests --host --packages vboot_reference`. TEST=`cros_run_unit_tests --board nissa --packages vboot_reference`. Change-Id: Ibe4745dbad20504a1ff7e39e10cbf18ed1831354 Signed-off-by: Edward O'Callaghan Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/4313546 Commit-Queue: Edward O'Callaghan Auto-Submit: Edward O'Callaghan Tested-by: Edward O'Callaghan Reviewed-by: Hung-Te Lin Commit-Queue: Hung-Te Lin --- futility/cmd_gscvd.c | 3 +- futility/cmd_load_fmap.c | 7 +++-- futility/cmd_sign.c | 3 +- futility/futility.h | 4 +-- futility/misc.c | 74 +++++++++++++++++++----------------------------- 5 files changed, 39 insertions(+), 52 deletions(-) diff --git a/futility/cmd_gscvd.c b/futility/cmd_gscvd.c index e68faf9c..e9f49679 100644 --- a/futility/cmd_gscvd.c +++ b/futility/cmd_gscvd.c @@ -1293,7 +1293,8 @@ static int do_gscvd(int argc, char *argv[]) } if (outfile) { - futil_copy_file_or_die(infile, outfile); + if (futil_copy_file(infile, outfile) < 0) + exit(1); work_file = outfile; } else { work_file = infile; diff --git a/futility/cmd_load_fmap.c b/futility/cmd_load_fmap.c index 474ff56e..220070a2 100644 --- a/futility/cmd_load_fmap.c +++ b/futility/cmd_load_fmap.c @@ -144,10 +144,11 @@ static int do_load_fmap(int argc, char *argv[]) infile = argv[optind++]; /* okay, let's do it ... */ - if (outfile) - futil_copy_file_or_die(infile, outfile); - else + if (!outfile) outfile = infile; + else + if (futil_copy_file(infile, outfile) < 0) + exit(1); errorcnt |= futil_open_and_map_file(outfile, &fd, FILE_RW, &buf, &len); diff --git a/futility/cmd_sign.c b/futility/cmd_sign.c index 383d39d7..4126717d 100644 --- a/futility/cmd_sign.c +++ b/futility/cmd_sign.c @@ -1139,7 +1139,8 @@ static int do_sign(int argc, char *argv[]) if (!sign_option.create_new_outfile) { /* We'll read-modify-write the output file */ if (sign_option.inout_file_count > 1) - futil_copy_file_or_die(infile, sign_option.outfile); + if (futil_copy_file(infile, sign_option.outfile) < 0) + goto done; infile = sign_option.outfile; } diff --git a/futility/futility.h b/futility/futility.h index 221590c8..3a276996 100644 --- a/futility/futility.h +++ b/futility/futility.h @@ -113,8 +113,8 @@ void update_hwid_digest(struct vb2_gbb_header *gbb); int print_hwid_digest(struct vb2_gbb_header *gbb, const char *banner, const char *footer); -/* Copies a file or dies with an error message */ -void futil_copy_file_or_die(const char *infile, const char *outfile); +/* Copies a file. */ +int futil_copy_file(const char *infile, const char *outfile); /* Possible file operation errors */ enum futil_file_err { diff --git a/futility/misc.c b/futility/misc.c index bcbc61ca..3f3fb656 100644 --- a/futility/misc.c +++ b/futility/misc.c @@ -8,6 +8,9 @@ #include #if !defined(HAVE_MACOS) && !defined(__FreeBSD__) && !defined(__OpenBSD__) #include /* For BLKGETSIZE64 */ +#include +#else +#include #endif #include #include @@ -210,58 +213,39 @@ int futil_set_gbb_hwid(struct vb2_gbb_header *gbb, const char *hwid) return VB2_SUCCESS; } -/* - * TODO: All sorts of race conditions likely here, and everywhere this is used. - * Do we care? If so, fix it. - */ -void futil_copy_file_or_die(const char *infile, const char *outfile) +int futil_copy_file(const char *infile, const char *outfile) { - pid_t pid; - int status; - VB2_DEBUG("%s -> %s\n", infile, outfile); - pid = fork(); - - if (pid < 0) { - fprintf(stderr, "Couldn't fork /bin/cp process: %s\n", - strerror(errno)); - exit(1); - } - - /* child */ - if (!pid) { - execl("/bin/cp", "/bin/cp", infile, outfile, NULL); - fprintf(stderr, "Child couldn't exec /bin/cp: %s\n", - strerror(errno)); - exit(1); + int ifd, ofd; + if ((ifd = open(infile, O_RDONLY)) == -1) { + ERROR("Cannot open '%s', %s.\n", infile, strerror(errno)); + return -1; } - - /* parent - wait for child to finish */ - if (wait(&status) == -1) { - fprintf(stderr, - "Couldn't wait for /bin/cp process to exit: %s\n", - strerror(errno)); - exit(1); + if ((ofd = creat(outfile, 0660)) == -1) { + ERROR("Cannot open '%s', %s.\n", outfile, strerror(errno)); + close(ifd); + return -1; } - - if (WIFEXITED(status)) { - status = WEXITSTATUS(status); - /* zero is normal exit */ - if (!status) - return; - fprintf(stderr, "/bin/cp exited with status %d\n", status); - exit(1); + struct stat finfo = {0}; + if (fstat(ifd, &finfo) < 0) { + ERROR("Cannot fstat '%s' as %s.\n", infile, strerror(errno)); + close (ifd); + close (ofd); + return -1; } - - if (WIFSIGNALED(status)) { - status = WTERMSIG(status); - fprintf(stderr, "/bin/cp was killed with signal %d\n", status); - exit(1); +#if !defined(HAVE_MACOS) && !defined(__FreeBSD__) && !defined(__OpenBSD__) + ssize_t ret = sendfile(ofd, ifd, NULL, finfo.st_size); +#else + ssize_t ret = fcopyfile(ifd, ofd, 0, COPYFILE_ALL); +#endif + close(ifd); + close(ofd); + if (ret == -1) { + ERROR("Cannot copy '%s'->'%s', %s.\n", infile, + outfile, strerror(errno)); } - - fprintf(stderr, "I have no idea what just happened\n"); - exit(1); + return ret; } enum futil_file_err futil_open_file(const char *infile, int *fd, -- cgit v1.2.1