Bug 12806 - Deleting in a row of hardlinked snapshots resets file permissions.
Deleting in a row of hardlinked snapshots resets file permissions.
Status: NEW
Product: rsync
Classification: Unclassified
Component: core
3.1.0
All All
: P5 normal
: ---
Assigned To: Wayne Davison
Rsync QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-05-26 14:24 UTC by Heinz-Willi Wyes
Modified: 2017-06-12 07:57 UTC (History)
0 users

See Also:


Attachments
Log file of bug reproduction (4.40 KB, text/plain)
2017-05-26 14:24 UTC, Heinz-Willi Wyes
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Heinz-Willi Wyes 2017-05-26 14:24:29 UTC
Created attachment 13239 [details]
Log file of bug reproduction

Overview:
In an differential backup system with hard-linked copies of unchanged files, deleting one snapshot resets the permission bit on all files.

Steps to reproduce:
1) Set up a source directory, fill in a couple of files. Some text is just fine.
Make one of these files have 444 permissions.

2) Set up a different directory to hold snapshots of the source directory.

3) Make a unique name for a snapshot, using e.g. the current date/time.

4) Make a snapshot using rsync:
rsync -ar --itemize-changes src/ snapshots/`echo $snap`/
$snap contains the name of the snapshot. Save this name in a file "lastsnapshot".

5) Make a new name for a new snapshot and make it using:
rsync -ar --itemize-changes --link-dest=`pwd`/snapshots/`cat lastsnapshot`/ src/ snapshots/`echo $snap`/. Again, save the name of the last snapshot.

6) Repeat step 5) a couple of times. In the snapshot directory, you will get a row of snapshots. All files in each snapshot is hard-linked to the counterparts in the other snapshots. Watch the permission bits of the one file with 444 permissions: Everything is ok.

7) Now, deliberately delete one of the snapshots using e.g.
rsync -r --delete `mktemp -d`/ snapshots/2017-05-26-15-00-53/

8) Now all of the files in all snapshots have 644 permissions. This will cause the re-transmission of that file with the following snapshot.

Actual results: On deletion of a snapshot, hardlinked files get the wrong permissions.

Expected results: Permission bits should be left intact.

Attached is a log file where I documented this behaviour.
Comment 1 Heinz-Willi Wyes 2017-05-28 06:16:25 UTC
Addendum.
Thought that the --inplace option could play the trick. I just employed the following:

rsync -r --delete --inplace `mktemp -d`/ snapshots/2017-05-28-08-10-11/

But this does not help either. Permission bits get reset to 644 anyway.
Comment 2 Kevin Korb 2017-05-28 10:20:26 UTC
Just use rm -rf to delete a backup.
Comment 3 Heinz-Willi Wyes 2017-05-29 05:11:58 UTC
(In reply to Kevin Korb from comment #2)
I'd like to, but I can't. I set up a local scenario in order to make the problem most easily reproducible. But the real use case involves rsync in a cloud backup application where I get no acces to the cloud provider's shell. Using any other possible protocol so as e.g. ftp or cifs is practically not possible. Each file operation with these protocols requires a roundtrip to my local server. Such an attempt lasts for hours considering that there are some 100.000 files per snapshot. rsync does this in less than two minutes - but resets the file permissions.
Comment 4 Heinz-Willi Wyes 2017-06-08 07:09:24 UTC
I got a personal message from Lars Ellenberg that went also to samba-bugs@samba.org and rsync-qa@samba.org. He wrote:

------------------
Calling chmod only on (optimization: non-empty) directories would fix this.
I don't need to chmod a *file* before unlinking it, I just need write permission on the directory it is located in.

(Now you have to convince the "appliance" to use a patched rsync ...)

Cheers,

    Lars Ellenberg


diff --git a/delete.c b/delete.c
index 88e4230..223b6d2 100644
--- a/delete.c
+++ b/delete.c
@@ -97,10 +97,10 @@ static enum delret delete_dir_contents(char *fname, uint16 flags)
 		}
 
 		strlcpy(p, fp->basename, remainder);
-		if (!(fp->mode & S_IWUSR) && !am_root && fp->flags & FLAG_OWNED_BY_US)
-			do_chmod(fname, fp->mode | S_IWUSR);
 		/* Save stack by recursing to ourself directly. */
 		if (S_ISDIR(fp->mode)) {
+			if (!(fp->mode & S_IWUSR) && !am_root && fp->flags & FLAG_OWNED_BY_US)
+				do_chmod(fname, fp->mode | S_IWUSR);
 			if (delete_dir_contents(fname, flags | DEL_RECURSE) != DR_SUCCESS)
 				ret = DR_NOT_EMPTY;
 		}
@@ -138,14 +138,13 @@ enum delret delete_item(char *fbuf, uint16 mode, uint16 flags)
 			fbuf, (int)mode, (int)flags);
 	}
 
-	if (flags & DEL_NO_UID_WRITE)
-		do_chmod(fbuf, mode | S_IWUSR);
-
 	if (S_ISDIR(mode) && !(flags & DEL_DIR_IS_EMPTY)) {
 		/* This only happens on the first call to delete_item() since
 		 * delete_dir_contents() always calls us w/DEL_DIR_IS_EMPTY. */
 		ignore_perishable = 1;
 		/* If DEL_RECURSE is not set, this just reports emptiness. */
+		if (!(mode & S_IWUSR) && !am_root && flags & DEL_NO_UID_WRITE && flags & DEL_RECURSE)
+			do_chmod(fbuf, mode | S_IWUSR);
 		ret = delete_dir_contents(fbuf, flags);
 		ignore_perishable = 0;
 		if (ret == DR_NOT_EMPTY || ret == DR_AT_LIMIT)
-----------------------------

As far as I understand, there actually *is* a design flaw in the rsync implementation that causes the behavior I described in my original post.

I suggested the patch to my provider. They replied that they rather wait for a new release of rsync with that patch officially applied.

Now I wonder what the status is. Will there be a patched version of rsync?
Comment 5 Heinz-Willi Wyes 2017-06-12 07:57:43 UTC
Lars Ellenberg provided a workaround for the behaviour. Using

rsync -d --delete --super `mktemp -d`/ snapshots/2017-05-26-15-00-53/

plays the trick.

Not sure whether the --super option has implications not suitable for other scenarios. For me it works just fine. Nevertheless, there should be a patch for rsync in order to overcome the questionable behaviour.