readdir() and read() errors ignored

Michael Brown mbrown at fensystems.co.uk
Sun Aug 24 09:16:47 EST 2003


We've just been hit rather badly by a very nasty bug that can cause rsync 
to silently discard files or fill them with zeroes.  It happens when e.g. 
opendir() succeeds but readdir() returns an error; rsync does not check 
for an error from readdir() and so simply ignores the error (along with 
any remaining files in the directory).  No error is reported to the user, 
who will then happily assume that their backup of 50GB worth of 
data succeeded...

This issue is discussed wrt the GNU fileutils at
http://www.mail-archive.com/bug-fileutils@gnu.org/msg01847.html
GNU fileutils *do* report errors under these circumstances.  Although it 
is arguably a kernel bug, I think rsync should err on the side of paranoia 
in the same way as the GNU utilities.

A similar error can occur when open() succeeds but subsequent calls to 
read() fail: under these circumstances rsync will transfer zeroes instead 
of the actual file data and will again report no errors to the user.

The attached patch fixes both of these problems.

I am not a list member; please Cc me on all replies.

Michael
-------------- next part --------------
Index: fileio.c
===================================================================
RCS file: /cvsroot/rsync/fileio.c,v
retrieving revision 1.6
diff -u -r1.6 fileio.c
--- fileio.c	22 May 2003 23:24:44 -0000	1.6
+++ fileio.c	23 Aug 2003 23:03:01 -0000
@@ -191,7 +191,10 @@
 		}
 
 		if ((nread=read(map->fd,map->p + read_offset,read_size)) != read_size) {
-			if (nread < 0) nread = 0;
+			if (nread < 0) {
+				rprintf(FERROR,"read failed in map_ptr\n");
+				exit_cleanup(RERR_FILEIO);
+			}
 			/* the best we can do is zero the buffer - the file
 			   has changed mid transfer! */
 			memset(map->p+read_offset+nread, 0, read_size - nread);
Index: flist.c
===================================================================
RCS file: /cvsroot/rsync/flist.c,v
retrieving revision 1.139
diff -u -r1.139 flist.c
--- flist.c	17 Aug 2003 21:29:11 -0000	1.139
+++ flist.c	23 Aug 2003 23:03:02 -0000
@@ -877,13 +877,18 @@
 		}
 	}
 
-	for (di = readdir(d); di; di = readdir(d)) {
+	for (errno = 0, di = readdir(d); di; errno = 0, di = readdir(d)) {
 		char *dname = d_name(di);
 		if (dname[0] == '.' && (dname[1] == '\0' ||
 		    (dname[1] == '.' && dname[2] == '\0')))
 			continue;
 		strlcpy(p, dname, MAXPATHLEN - l);
 		send_file_name(f, flist, fname, recurse, 0);
+	}
+	if ( errno ) {
+		io_error = 1;
+		rprintf(FERROR, "readdir(%s): %s\n", dir, strerror(errno));
+		/* Don't return yet; we want to clean up first */
 	}
 
 	if (local_exclude_list)
Index: rsync.c
===================================================================
RCS file: /cvsroot/rsync/rsync.c,v
retrieving revision 1.120
diff -u -r1.120 rsync.c
--- rsync.c	18 Feb 2003 18:07:36 -0000	1.120
+++ rsync.c	23 Aug 2003 23:03:02 -0000
@@ -86,7 +86,7 @@
 		return -1;
 	}
 
-	for (di=readdir(d); di; di=readdir(d)) {
+	for (errno = 0, di=readdir(d); di; errno = 0, di=readdir(d)) {
 		char *dname = d_name(di);
 		if (strcmp(dname,".")==0 ||
 		    strcmp(dname,"..")==0)
@@ -99,6 +99,12 @@
 			return -1;
 		}
 	}	
+	if ( errno ) {
+		closedir(d);
+		rprintf(FERROR,"delete_file: readdir(%s): %s\n",
+			fname,strerror(errno));
+		return -1;
+	}
 
 	closedir(d);
 	


More information about the rsync mailing list