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/[EMAIL PROTECTED]/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:          [EMAIL PROTECTED]

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

Reply via email to