[Bug 208627] net/rsync: Rsync compiled with ACL option will always consider ACLs to be different even if they are not.

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=208627

            Bug ID: 208627
           Summary: net/rsync: Rsync compiled with ACL option will always
                    consider ACLs to be different even if they are not.
           Product: Ports & Packages
           Version: Latest
          Hardware: amd64
                OS: Any
            Status: New
          Severity: Affects Some People
          Priority: ---
         Component: Individual Port(s)
          Assignee: ehaupt at FreeBSD.org
          Reporter: chen at starworm.com
          Assignee: ehaupt at FreeBSD.org
             Flags: maintainer-feedback?(ehaupt at FreeBSD.org)

I'm seeing the following results when using rsync 3.1.2 that comes with FreeNAS
9.10:

$ touch a.txt

$ rsync -i -aX --acls a.txt b.txt
>f+++++++++ a.txt

$ rsync -i -aX --acls a.txt b.txt
.f.......a. a.txt

$ rsync -i -aX --acls a.txt b.txt
.f.......a. a.txt

$ getfacl a.txt
# file: a.txt
# owner: root
# group: user
            owner@:rw-p--aARWcCos:------:allow
            group@:r-----a-R-c--s:------:allow
         everyone@:r-----a-R-c--s:------:allow

$ getfacl b.txt
# file: b.txt
# owner: root
# group: user
            owner@:rw-p--aARWcCos:------:allow
            group@:r-----a-R-c--s:------:allow
         everyone@:r-----a-R-c--s:------:allow

Each run of with "--acls" considers the ACLs to have changed even if they
haven't. Rsync built from ports using the standard options (i.e. with the ACL
option unset) doesn't have this bug. Code inspection suggests that the problem
is likely caused by "extrapatch-acl" making "set_acl" unconditionally return
"changed == 1".

Compare the code snippet from the patch:

    if (sxp->brand == SMB_ACL_BRAND_NFS4) {
        ndx = F_ACL(file);
        if (ndx >= 0 && (size_t)ndx < nfs4_acl_list.count) {
            nfs4_duo *duo_item = nfs4_acl_list.items;
            duo_item += ndx;
            changed = 1;

            if (!duo_item->sacl) {
                duo_item->sacl = acl_from_text(duo_item->nacl.nfs4_acl_text);
                if (!duo_item->sacl)
                    return -1;
            }

            if (!dry_run && fname) {
                if (sys_acl_set_file(fname, SMB_ACL_TYPE_NFS4, duo_item->sacl)
< 0) {
                    rsyserr(FERROR_XFER, errno, "set_acl: sys_acl_set_file(%s,
%s)",
                        fname, str_acl_type(SMB_ACL_TYPE_NFS4));
                    return -1;
                }

                return changed;
            }
        }
    }

With a code snippet from the rest of the code:

    ndx = F_ACL(file);
    if (ndx >= 0 && (size_t)ndx < access_acl_list.count) {
        acl_duo *duo_item = access_acl_list.items;
        duo_item += ndx;
        eq = sxp->acc_acl
          && rsync_acl_equal_enough(sxp->acc_acl, &duo_item->racl, new_mode);
        if (!eq) {
            changed = 1;
            if (!dry_run && fname
             && set_rsync_acl(fname, duo_item, SMB_ACL_TYPE_ACCESS,
                      sxp, new_mode) < 0)
                return -1;
        }
    }

Today's ACL patch code simply sets "changed", but it should probably check
"nfs4_acl_equal(sxp->nfs4_acl, duo_item->nacl)" first. It might also be
necessary to move the return out of the inner if-statement for things to behave
nicely (it doesn't make sense to only terminate early for non-dry runs).

-- 
You are receiving this mail because:
You are the assignee for the bug.