Created attachment 432938 [details] patch as mentioned in the PR Description of problem: If you are using the -A/--acls option and you are not running as root and are not using the --numeric-ids option then if you have an ACL that includes a group entry for a group you are not a member of on the receiving side, then acl_set_file will return EINVAL, b/c rsync mistakenly maps the group name to gid GID_NONE (-1), which (fortunately) fails Version-Release number of selected component (if applicable): 2.6.8-3.1 How reproducible: tmp=$(mktemp -d -p /some/acl-supporting/directory) cd "$tmp" mkdir src dst touch src/file setfacl -m g:root:--- src/file rsync -A src/file dst/ Steps to Reproduce: 1. see above 2. 3. Actual results: set_acl: sys_acl_set_file(.file.spbAuh, SMB_ACL_TYPE_ACCESS): Invalid argument rsync error: some files could not be transferred (code 23) at main.c(892) [sender=2.6.8] Expected results: no error Additional info: patch attached The problem seems to stem from an optimization in the group-name-mapping code that puts GID_NONEs in the mapping for such gids as a non-root chgrp (chown(..., -1, gid)) is going to be expected to fail for, with an EPERM, when using the --group option, but this breaks things for the --acls patch, in perfectly legitimate cases where an ACL includes an entry for a group of which the user is not a member, since the same mapping list is used to map b/w names and gids for ACLs as is used for file group ownership The attached patch defers the GID_NONE-mapping behavior until after the ACL name-mapping has been done; then another pass through the map is made, and all relevant gids are overwritten with the GID_NONE before the group- ownership name-mapping is done Note it hasn't had any testing yet . . .
I found out that upstream already fixed this in later versions. I tried to backport some patches which take care of this, but the truth is that the fix happened between 2.6.9 and 3.0.0, which means that changes around this functionality were significant. Your patch might be working, but to be honest I'm not comfortable with it, because at the first glance, it doesn't seem to cover all use cases of modified function. Also it would divert rsync from upstream solution of this issue. Therefore I think the only right way here is to rebase rsync to version 3.0.0 or newer. Bug 339971 is a tracker for rsync update. If you'd like to do it without rebase, you might try to backport those patches yourself, but as I said - there might me a large number of patches influencing this.
Created attachment 435214 [details] Backported upstream patch I'm attaching backported upstream patch. I've done some basic testing and it should fix the issue. However as I wrote earlier, this is quite a big change and it is possible that I didn't cherry-pick all needed parts. Before putting this to RHEL, I want it to be thoroughly tested. Use this patch on production machines at your own risk.
(In reply to comment #1) > I found out that upstream already fixed this in later versions. I tried to > backport some patches which take care of this, but the truth is that the fix > happened between 2.6.9 and 3.0.0, which means that changes around this > functionality were significant. That's what i'd initially intended as well, but it looks (comment #2) like you had the wherewithal to go ahead and do so, so, kudos, and thanks for making what i thought was just going to be a throw-it-over-the-wall-to-let- anybody-else-who-discovers-the-same-problem-know-he's-not-imaginging-things report into something constructive that actually helps RH/Fedora users > > Your patch might be working, but to be honest I'm not comfortable with it, > because at the first glance, it doesn't seem to cover all use cases of modified > function. % grep -w match_gid *.c uidlist.c:static gid_t match_gid(gid_t gid) uidlist.c: *id = match_gid(*id); uidlist.c: flist->files[i]->gid = match_gid(flist->files[i]->gid); % diff -pu ../rsync-2.6.8/uidlist.c.orig uidlist.c | \ ruby -e '$stdin.read.split(/^@@/).each{|hunk| \ /\bmatch_gid\b/ === hunk and puts "@@" + hunk;}' -347,10 +347,12 @@ void recv_uid_list(int f, struct file_li #ifdef SUPPORT_ACLS if (preserve_acls && !numeric_ids) { id_t *id; - while ((id = next_acl_uid(flist)) != NULL) + while ((id = next_acl_uid(flist)) != NULL) { *id = match_uid(*id); - while ((id = next_acl_gid(flist)) != NULL) + } + while ((id = next_acl_gid(flist)) != NULL) { *id = match_gid(*id); + } } #endif What other use cases are there? Also it would divert rsync from upstream solution of this issue. > Therefore I think the only right way here is to rebase rsync to version 3.0.0 > or newer. Bug 339971 is a tracker for rsync update. > > If you'd like to do it without rebase, you might try to backport those patches > yourself, but as I said - there might me a large number of patches influencing > this. Again, thanks for backporting. I'm sort of of the other mind about which patch seems like it's the more obviously least-likely to break anything: your rsync-fu backport or my 2-line kludge. Wish i had some sort of re- gression test to be sure . . . Anyways, thanks again for dealing with this so conscientiously
Well if your patch suits you better, feel free to use it. Originally when I was talking about use cases, I meant usage of recv_add_gid, where you removed those two lines. But as I wrote, it was only at a first glance. At a second look it doesn't seem to break something. I still think the right solution is to rebase the rsync. If rsync gets to be approved component in RHEL-5, I hope the rebase bug 339971 will be the first to be done.
This request was evaluated by Red Hat Product Management for inclusion in the current release of Red Hat Enterprise Linux. Because the affected component is not scheduled to be updated in the current release, Red Hat is unfortunately unable to address this request at this time. Red Hat invites you to ask your support representative to propose this request, if appropriate and relevant, in the next release of Red Hat Enterprise Linux.
This request was erroneously denied for the current release of Red Hat Enterprise Linux. The error has been fixed and this request has been re-proposed for the current release.
An advisory has been issued which should help the problem described in this bug report. This report is therefore being closed with a resolution of ERRATA. For more information on therefore solution and/or where to find the updated files, please follow the link below. You may reopen this bug report if the solution does not work for you. http://rhn.redhat.com/errata/RHSA-2011-0999.html