rsync - Oct 2006 - Rsync 2.6.9pre2 tries to read ACLs of nonexistent files

Dear rsync people,

Today I tried to back up my computer using rsnapshot with the RPM
version of rsync-acl 2.6.9pre1 that I built.  I tried twice, and both
times, rsync encountered some kind of assertion failure.  I was trying
to reproduce the crash with rsync-acl 2.6.9pre2 and noticed a
different bug (described below); when I have a chance, I will go back
and investigate the crash further.

Rsync 2.6.9pre2 produced the following errors:

/home/matt/rsync/rsync-acl/rsync -aAPx --del --numeric-ids --relative \
    --delete-excluded --exclude=media/external-disk/snapshots \
    --link-dest=/media/external-disk/snapshots/occasional.0/mattlaptop/ /. \
    /media/external-disk/snapshots/.sync/mattlaptop/
[...]
/etc/
rsync: get_acl: sys_acl_get_file(etc/adjtime, SMB_ACL_TYPE_ACCESS): No
such file or directory (2)
/etc/adjtime
          46 100%    0.00kB/s    0:00:00 (xfer#1, to-check=283360/283617)
rsync: get_acl: sys_acl_get_file(etc/group, SMB_ACL_TYPE_ACCESS): No
such file or directory (2)
/etc/group
         582 100%  568.36kB/s    0:00:00 (xfer#2, to-check=283334/283617)
[...]

I reran the command with / as the current directory and the same thing
happened, but /etc/adjtime definitely exists, so the errors must have
come from the receiver.  It looks like the receiver tried to read each
file's ACL before it actually created the file.

I think I have tracked down the problem.  In lines 1392-1393, the
generator reads the old ACL of a destination file about to be
overwritten so it knows whether to itemize an ACL change:

		if (preserve_acls && real_ret == 0)
			get_acl(fname, &real_sx);

Presumably real_ret is zero if and only if the destination file
actually exists.  However, back in lines 1210-1226, the generator sets
real_ret to 0 if a basis file is found, even though the destination
file does not exist:

	if (statret != 0 && basis_dir[0] != NULL) {
		int j = try_dests_reg(file, fname, ndx, fnamecmpbuf, &sx,
				      itemizing, maybe_ATTRS_REPORT, code);
		if (j == -2) {
			if (remove_source_files == 1)
				goto return_with_success;
			goto cleanup;
		}
		if (j >= 0) {
			fnamecmp = fnamecmpbuf;
			fnamecmp_type = j;
			statret = 0;
		}
	}

	real_ret = statret;
	real_sx = sx;

A few lines later, rsync checks for a fuzzy basis file; if it finds
one, it sets statret = 0, and that change does /not/ propagate to
real_ret.  Thus, I'm guessing real_ret and real_sx should actually
have been set /before/ the basis directories are checked.

Matt

On Sat, Oct 21, 2006 at 04:27:17PM -0400, Matt McCutchen
wrote:
> 		if (preserve_acls && real_ret == 0)
> 			get_acl(fname, &real_sx);
That should be checking fnamecmp, not fname.  I've just commited a fix
for patches/acls.diff.

Let me know what you find out with the assert failure.  Also note that
I fixed a bug in the handling of -H that is only in CVS, not in either
pre-release.  I'll attach the patch that I committed to CVS earlier.

..wayne..
-------------- next part --------------
--- hlink.c	13 Oct 2006 07:48:35 -0000	1.65
+++ hlink.c	17 Oct 2006 18:49:04 -0000	1.66
@@ -83,6 +83,7 @@ static void link_idev_data(void)
 
 			FPTR(cur)->F_HLINDEX = to;
 			FPTR(cur)->F_NEXT = hlink_list[++from];
+			FPTR(cur)->link_u.links->link_dest_used = 0;
 		}
 		pool_free(idev_pool, 0, FPTR(cur)->link_u.idev);
 		if (from > start) {
@@ -94,6 +95,7 @@ static void link_idev_data(void)
 			FPTR(cur)->F_HLINDEX = to;
 			FPTR(cur)->F_NEXT = head;
 			FPTR(cur)->flags |= FLAG_HLINK_EOL;
+			FPTR(cur)->link_u.links->link_dest_used = 0;
 			hlink_list[to++] = head;
 		} else
 			FPTR(cur)->link_u.links = NULL;