Module Name:    src
Committed By:   lukem
Date:           Sun May 28 21:42:40 UTC 2023

Modified Files:
        src/bin/pax: ar_subs.c buf_subs.c extern.h file_subs.c

Log Message:
pax: don't overwrite destination if -r -w copy fails

Add more error handling to pax -r -w so that any failure
during the copy to the temporary file (including a failed flush)
prevents any existing destination file from being replaced
with the partial (including possibly empty) temporary file.
The partial temporary file is removed.  pax still exists non-zero.

Thanks to Michael van Elst (mlelstv@) for the analysis
of the problem in the PR.

Should fix PR misc/33753.


To generate a diff of this commit:
cvs rdiff -u -r1.57 -r1.58 src/bin/pax/ar_subs.c
cvs rdiff -u -r1.30 -r1.31 src/bin/pax/buf_subs.c
cvs rdiff -u -r1.60 -r1.61 src/bin/pax/extern.h
cvs rdiff -u -r1.64 -r1.65 src/bin/pax/file_subs.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/bin/pax/ar_subs.c
diff -u src/bin/pax/ar_subs.c:1.57 src/bin/pax/ar_subs.c:1.58
--- src/bin/pax/ar_subs.c:1.57	Sun Dec  5 02:52:17 2021
+++ src/bin/pax/ar_subs.c	Sun May 28 21:42:40 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: ar_subs.c,v 1.57 2021/12/05 02:52:17 msaitoh Exp $	*/
+/*	$NetBSD: ar_subs.c,v 1.58 2023/05/28 21:42:40 lukem Exp $	*/
 
 /*-
  * Copyright (c) 1992 Keith Muller.
@@ -42,7 +42,7 @@
 #if 0
 static char sccsid[] = "@(#)ar_subs.c	8.2 (Berkeley) 4/18/94";
 #else
-__RCSID("$NetBSD: ar_subs.c,v 1.57 2021/12/05 02:52:17 msaitoh Exp $");
+__RCSID("$NetBSD: ar_subs.c,v 1.58 2023/05/28 21:42:40 lukem Exp $");
 #endif
 #endif /* not lint */
 
@@ -1131,10 +1131,14 @@ copy(void)
 		}
 
 		/*
-		 * copy source file data to the destination file
+		 * copy source file data to the destination file.
+		 * if there was a failure, remove the temporary file
+		 * and leave any existing destination file unmodified.
 		 */
-		cp_file(arcn, fdsrc, fddest);
-		file_close(arcn, fddest);
+		if (cp_file(arcn, fdsrc, fddest) < 0)
+			file_cleanup(arcn, fddest);
+		else
+			file_close(arcn, fddest);
 		rdfile_close(arcn, &fdsrc);
 
 		if (vflag && vfpart) {

Index: src/bin/pax/buf_subs.c
diff -u src/bin/pax/buf_subs.c:1.30 src/bin/pax/buf_subs.c:1.31
--- src/bin/pax/buf_subs.c:1.30	Sat May 28 10:36:21 2022
+++ src/bin/pax/buf_subs.c	Sun May 28 21:42:40 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: buf_subs.c,v 1.30 2022/05/28 10:36:21 andvar Exp $	*/
+/*	$NetBSD: buf_subs.c,v 1.31 2023/05/28 21:42:40 lukem Exp $	*/
 
 /*-
  * Copyright (c) 1992 Keith Muller.
@@ -42,7 +42,7 @@
 #if 0
 static char sccsid[] = "@(#)buf_subs.c	8.2 (Berkeley) 4/18/94";
 #else
-__RCSID("$NetBSD: buf_subs.c,v 1.30 2022/05/28 10:36:21 andvar Exp $");
+__RCSID("$NetBSD: buf_subs.c,v 1.31 2023/05/28 21:42:40 lukem Exp $");
 #endif
 #endif /* not lint */
 
@@ -745,8 +745,11 @@ rd_wrfile(ARCHD *arcn, int ofd, off_t *l
 	 * written. just closing with the file offset moved forward may not put
 	 * a hole at the end of the file.
 	 */
-	if (ofd >= 0 && isem && (arcn->sb.st_size > 0L))
-		file_flush(ofd, fnm, isem);
+	if (ofd >= 0 && isem && (arcn->sb.st_size > 0L)) {
+		if (file_flush(ofd, fnm, isem) < 0) {
+			/* write flush errors are not an error here */;
+		}
+	}
 
 	/*
 	 * if we failed from archive read, we do not want to skip
@@ -758,9 +761,11 @@ rd_wrfile(ARCHD *arcn, int ofd, off_t *l
 	 * some formats record a crc on file data. If so, then we compare the
 	 * calculated crc to the crc stored in the archive
 	 */
-	if (docrc && (size == 0L) && (arcn->crc != crc))
+	if (docrc && (size == 0L) && (arcn->crc != crc)) {
 		tty_warn(1,"Actual crc does not match expected crc %s",
 		    arcn->name);
+		/* crc warning is not an error */
+	}
 	return 0;
 }
 
@@ -769,9 +774,11 @@ rd_wrfile(ARCHD *arcn, int ofd, off_t *l
  *	copy the contents of one file to another. used during -rw phase of pax
  *	just as in rd_wrfile() we use a special write function to write the
  *	destination file so we can properly copy files with holes.
+ * Return:
+ *	0 if ok, -1 if any error.
  */
 
-void
+int
 cp_file(ARCHD *arcn, int fd1, int fd2)
 {
 	int cnt;
@@ -783,6 +790,7 @@ cp_file(ARCHD *arcn, int fd1, int fd2)
 	int rem;
 	int sz = MINFBSZ;
 	struct stat sb, origsb;
+	int rv = 0;
 
 	/*
 	 * check for holes in the source file. If none, we will use regular
@@ -797,8 +805,10 @@ cp_file(ARCHD *arcn, int fd1, int fd2)
 	 * if Mflag is set, use the actual mtime instead.
 	 */
 	origsb = arcn->sb;
-	if (Mflag && (fstat(fd1, &origsb) < 0))
+	if (Mflag && (fstat(fd1, &origsb) < 0)) {
 		syswarn(1, errno, "Failed stat on %s", arcn->org_name);
+		rv = -1;
+	}
 
 	/*
 	 * pass the blocksize of the file being written to the write routine,
@@ -830,17 +840,22 @@ cp_file(ARCHD *arcn, int fd1, int fd2)
 	/*
 	 * check to make sure the copy is valid.
 	 */
-	if (res < 0)
+	if (res < 0) {
 		syswarn(1, errno, "Failed write during copy of %s to %s",
 			arcn->org_name, arcn->name);
-	else if (cpcnt != arcn->sb.st_size)
+		rv = -1;
+	} else if (cpcnt != arcn->sb.st_size) {
 		tty_warn(1, "File %s changed size during copy to %s",
 			arcn->org_name, arcn->name);
-	else if (fstat(fd1, &sb) < 0)
+		rv = -1;
+	} else if (fstat(fd1, &sb) < 0) {
 		syswarn(1, errno, "Failed stat of %s", arcn->org_name);
-	else if (origsb.st_mtime != sb.st_mtime)
+		rv = -1;
+	} else if (origsb.st_mtime != sb.st_mtime) {
 		tty_warn(1, "File %s was modified during copy to %s",
 			arcn->org_name, arcn->name);
+		rv = -1;
+	}
 
 	/*
 	 * if the last block has a file hole (all zero), we must make sure this
@@ -848,9 +863,11 @@ cp_file(ARCHD *arcn, int fd1, int fd2)
 	 * written. just closing with the file offset moved forward may not put
 	 * a hole at the end of the file.
 	 */
-	if (!no_hole && isem && (arcn->sb.st_size > 0L))
-		file_flush(fd2, fnm, isem);
-	return;
+	if (!no_hole && isem && (arcn->sb.st_size > 0L)) {
+		if (file_flush(fd2, fnm, isem) < 0)
+			rv = -1;
+	}
+	return rv;
 }
 
 /*

Index: src/bin/pax/extern.h
diff -u src/bin/pax/extern.h:1.60 src/bin/pax/extern.h:1.61
--- src/bin/pax/extern.h:1.60	Fri Apr  3 16:13:32 2020
+++ src/bin/pax/extern.h	Sun May 28 21:42:40 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: extern.h,v 1.60 2020/04/03 16:13:32 joerg Exp $	*/
+/*	$NetBSD: extern.h,v 1.61 2023/05/28 21:42:40 lukem Exp $	*/
 
 /*-
  * Copyright (c) 1992 Keith Muller.
@@ -115,7 +115,7 @@ int rd_wrbuf(char *, int);
 int wr_skip(off_t);
 int wr_rdfile(ARCHD *, int, off_t *);
 int rd_wrfile(ARCHD *, int, off_t *);
-void cp_file(ARCHD *, int, int);
+int cp_file(ARCHD *, int, int);
 int buf_fill(void);
 int buf_flush(int);
 
@@ -150,6 +150,7 @@ extern char *gnu_name_string, *gnu_link_
 extern size_t gnu_name_length, gnu_link_length;
 extern char *xtmp_name;
 int file_creat(ARCHD *, int);
+void file_cleanup(ARCHD *, int);
 void file_close(ARCHD *, int);
 int lnk_creat(ARCHD *, int *);
 int cross_lnk(ARCHD *);
@@ -162,7 +163,7 @@ int set_ids(char *, uid_t, gid_t);
 void set_pmode(char *, mode_t);
 void set_chflags(char *fnm, u_int32_t flags);
 int file_write(int, char *, int, int *, int *, int, char *);
-void file_flush(int, char *, int);
+int file_flush(int, char *, int);
 void rdfile_close(ARCHD *, int *);
 int set_crc(ARCHD *, int);
 

Index: src/bin/pax/file_subs.c
diff -u src/bin/pax/file_subs.c:1.64 src/bin/pax/file_subs.c:1.65
--- src/bin/pax/file_subs.c:1.64	Wed Mar 20 03:13:39 2019
+++ src/bin/pax/file_subs.c	Sun May 28 21:42:40 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: file_subs.c,v 1.64 2019/03/20 03:13:39 gutteridge Exp $	*/
+/*	$NetBSD: file_subs.c,v 1.65 2023/05/28 21:42:40 lukem Exp $	*/
 
 /*-
  * Copyright (c) 1992 Keith Muller.
@@ -42,7 +42,7 @@
 #if 0
 static char sccsid[] = "@(#)file_subs.c	8.1 (Berkeley) 5/31/93";
 #else
-__RCSID("$NetBSD: file_subs.c,v 1.64 2019/03/20 03:13:39 gutteridge Exp $");
+__RCSID("$NetBSD: file_subs.c,v 1.65 2023/05/28 21:42:40 lukem Exp $");
 #endif
 #endif /* not lint */
 
@@ -179,11 +179,43 @@ file_creat(ARCHD *arcn, int write_to_har
 }
 
 /*
+ * file_cleanup()
+ *	Close file descriptor to a file just created by pax.
+ *	Unlinks any temporary file.
+ */
+
+void
+file_cleanup(ARCHD *arcn, int fd)
+{
+	char *tmp_name;
+
+	if (fd < 0)
+		return;
+
+	tmp_name = (arcn->tmp_name != NULL) ? arcn->tmp_name : arcn->name;
+
+	if (close(fd) < 0)
+		syswarn(0, errno, "Cannot close file descriptor on %s",
+		    tmp_name);
+
+	/* No temporary file to cleanup? */
+	if (arcn->tmp_name == NULL)
+		return;
+
+	/* Cleanup the temporary file. */
+	if (unlink(arcn->tmp_name) < 0) {
+		syswarn(0, errno, "Cannot unlink %s", arcn->tmp_name);
+	}
+
+	free(arcn->tmp_name);
+	arcn->tmp_name = NULL;
+	xtmp_name = NULL;
+}
+
+/*
  * file_close()
  *	Close file descriptor to a file just created by pax. Sets modes,
  *	ownership and times as required.
- * Return:
- *	0 for success, -1 for failure
  */
 
 void
@@ -1039,9 +1071,11 @@ file_write(int fd, char *str, int cnt, i
  *	when the last file block in a file is zero, many file systems will not
  *	let us create a hole at the end. To get the last block with zeros, we
  *	write the last BYTE with a zero (back up one byte and write a zero).
+ * Return:
+ *	0 if was able to flush the file, -1 otherwise
  */
 
-void
+int
 file_flush(int fd, char *fname, int isempt)
 {
 	static char blnk[] = "\0";
@@ -1051,19 +1085,21 @@ file_flush(int fd, char *fname, int isem
 	 * filled with all zeros.
 	 */
 	if (!isempt)
-		return;
+		return 0;
 
 	/*
 	 * move back one byte and write a zero
 	 */
 	if (lseek(fd, (off_t)-1, SEEK_CUR) < 0) {
 		syswarn(1, errno, "Failed seek on file %s", fname);
-		return;
+		return -1;
 	}
 
-	if (write_with_restart(fd, blnk, 1) < 0)
+	if (write_with_restart(fd, blnk, 1) < 0) {
 		syswarn(1, errno, "Failed write to file %s", fname);
-	return;
+		return -1;
+	}
+	return 0;
 }
 
 /*

Reply via email to