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 <quasisec@google.com> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/4313546 Commit-Queue: Edward O'Callaghan <quasisec@chromium.org> Auto-Submit: Edward O'Callaghan <quasisec@chromium.org> Tested-by: Edward O'Callaghan <quasisec@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> Commit-Queue: Hung-Te Lin <hungte@chromium.org>
diff --git a/futility/cmd_gscvd.c b/futility/cmd_gscvd.c index e68faf9..e9f4967 100644 --- a/futility/cmd_gscvd.c +++ b/futility/cmd_gscvd.c
@@ -1293,7 +1293,8 @@ } 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 474ff56..220070a 100644 --- a/futility/cmd_load_fmap.c +++ b/futility/cmd_load_fmap.c
@@ -144,10 +144,11 @@ 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 383d39d..4126717 100644 --- a/futility/cmd_sign.c +++ b/futility/cmd_sign.c
@@ -1139,7 +1139,8 @@ 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 221590c..3a27699 100644 --- a/futility/futility.h +++ b/futility/futility.h
@@ -113,8 +113,8 @@ 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 bcbc61c..3f3fb65 100644 --- a/futility/misc.c +++ b/futility/misc.c
@@ -8,6 +8,9 @@ #include <fcntl.h> #if !defined(HAVE_MACOS) && !defined(__FreeBSD__) && !defined(__OpenBSD__) #include <linux/fs.h> /* For BLKGETSIZE64 */ +#include <sys/sendfile.h> +#else +#include <copyfile.h> #endif #include <stdarg.h> #include <stdint.h> @@ -210,58 +213,39 @@ 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); + int ifd, ofd; + if ((ifd = open(infile, O_RDONLY)) == -1) { + ERROR("Cannot open '%s', %s.\n", infile, strerror(errno)); + return -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); + if ((ofd = creat(outfile, 0660)) == -1) { + ERROR("Cannot open '%s', %s.\n", outfile, strerror(errno)); + close(ifd); + 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); + 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 (WIFEXITED(status)) { - status = WEXITSTATUS(status); - /* zero is normal exit */ - if (!status) - return; - fprintf(stderr, "/bin/cp exited with status %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)); } - - if (WIFSIGNALED(status)) { - status = WTERMSIG(status); - fprintf(stderr, "/bin/cp was killed with signal %d\n", status); - exit(1); - } - - 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,