In fruit_fget_nt_acl() the 3 extra ACE entries corresponding to mode/uid/gid are always added to the returned ACL as virtual ACE entries that are not expected to be stored in the file ACL on disk. In fruit_fset_nt_acl() the client-sent ACL is applied to the file, then the mode entry (if sent) is used to do the CHMOD - but the client-sent ACL is applied without removing any virtual ACE entries. If a naive client just round-trips get/set/get/set, on every set the 'virtual' ACE entries will be stored in the xattr. I think the correct action on fruit_fset_nt_acl() is to check for - then *remove* any virtual ACE entries sent by the client before passing down to the underlying SET_NT_ACL call. (Discovered by code review when designing a similar mechanism for SMB2 UNIX extensions). Jeremy.
Created attachment 14020 [details] Proposed git-am fix for master.
Created attachment 14027 [details] git-am fix for 4.8.0rcX, 4.7.next, 4.6.next. Cherry-picked from master.
Comment on attachment 14027 [details] git-am fix for 4.8.0rcX, 4.7.next, 4.6.next. Damn. There is a remaining bug in this patchset. In the set_acl path I only remove the ACE corresponding to the mode already on the file. I need to remove the ACE entry that matches global_sid_Unix_NFS_Mode+new_permissions as well as global_sid_Unix_NFS_Mode+existing permissions. Follow-up patch to come on samba-technical. Jeremy.
(In reply to Jeremy Allison from comment #3) Can't you just filter out any aces related to global_sid_Unix_NFS sids, with some like: + cmp = dom_sid_compare_domain(&global_sid_Unix_NFS, + &user_info_dc->sids[i]); + if (cmp == 0) { I'm using some similar here https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=35603dd8ef03b87a581d1dd4f9ad4964beb13336
Yes, that's what I'm planning to do in a subsequent patch. I need to make sure they don't get stored in the ACL on disk as they are 'virtual' ACE's created on demand on read. Patch to follow :-).
FYI, the reason the patch is taking a little while is I'm adding a regression test for this...
Created attachment 14058 [details] git-am fix for 4.8.next.
Comment on attachment 14058 [details] git-am fix for 4.8.next. Cherry-picked from master.
Created attachment 14059 [details] git-am fix for 4.7.next. Back-ported from master.
Comment on attachment 14058 [details] git-am fix for 4.8.next. Argh. Error in patch again (addressing on list).
Comment on attachment 14059 [details] git-am fix for 4.7.next. Argh. Error in patch again (addressing on list).
Created attachment 14068 [details] (final) git-am fix for 4.8.next. Cherry-picked from master.
Created attachment 14069 [details] (final) git-am fix for 4.7.next. Back-ported from master.
Reassigning to Karolin for inclusion in 4.8 and 4.7.
(In reply to Ralph Böhme from comment #14) Pushed to autobuild-v4-[8,7]-test
(In reply to Karolin Seeger from comment #15) Pushed to both branches. Closing out bug report. Thanks!