readdir() and read() errors ignored

[jw schultz]

On Sun, Aug 24, 2003 at 12:16:47AM +0100, Michael Brown wrote:
> 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.

There is a patch on-list for the read error.   Your fix is
incorrect because it causes rsync to exit if a file is
truncated during read.

I'll look closer at the readir error when i get a chance.

> 
> I am not a list member; please Cc me on all replies.
> 
> Michael
> 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);
>  	

> -- 
> To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync
> Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html

-- 
________________________________________________________________
	J.W. Schultz            Pegasystems Technologies
	email address:		jw at pegasys.ws

		Remember Cernan and Schmitt