Message ID | CAH2r5msswZg_NvY9qMNknh2FWht1jMgwW1s2umtaBj3mHreDAw@mail.gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Fri, Nov 15, 2013 at 08:53:12PM -0600, Steve French wrote: > From: Steve French <smfrench@gmail.com> > Date: Fri, 15 Nov 2013 20:41:32 -0600 > Subject: [PATCH] [CIFS] setfacl removes part of ACL when setting POSIX ACLs to > Samba > > setfacl over cifs mounts can remove the default ACL when setting the > (non-default part of) the ACL and vice versa (we were leaving at 0 > rather than setting to -1 the count field for the unaffected > half of the ACL. For example notice the setfacl removed > the default ACL in this sequence: Looks good to me. Removing the default acl when the number of ACE entries is set to zero is by design in the Samba server, to allow setfacl -k to work (remove the default ACL). Jeremy. -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 15, 2013 at 08:53:12PM -0600, Steve French wrote: > From: Steve French <smfrench@gmail.com> > Date: Fri, 15 Nov 2013 20:41:32 -0600 > Subject: [PATCH] [CIFS] setfacl removes part of ACL when setting POSIX ACLs to > Samba > > setfacl over cifs mounts can remove the default ACL when setting the > (non-default part of) the ACL and vice versa (we were leaving at 0 > rather than setting to -1 the count field for the unaffected > half of the ACL. For example notice the setfacl removed > the default ACL in this sequence: Can you send a patch to xfstests to make sure we regression test for this issue on all filesystems? -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > From: Steve French <smfrench@gmail.com> > > Date: Fri, 15 Nov 2013 20:41:32 -0600 > > Subject: [PATCH] [CIFS] setfacl removes part of ACL when setting POSIX ACLs to > > Samba > > > > setfacl over cifs mounts can remove the default ACL Hi cifs-utils 6.1 Samba 4.1.0 I don't understand the '...over cifs mounts...' bit. Does it mean that if I have a share mounted on my Linux box and do a setfacl on a file in the share then the default acl will be removed? Sorry for a non dev question. Cheers, Steve -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Makes sense to add a setfacl/getfacl test to xfstest and was trying to build updated xfstests and look at what has changed but ran into a strange error building xfstests and didn't see an obvious answer when googling for it. Any idea how to workaround the build failure? Building src [DEP] [CC] dirstress gcc: error: /lib64/libhandle.so: Too many levels of symbolic links These are the steps I went through from a fairly clean Fedora 19 64 system before the make failure: git clone git://oss.sgi.com/xfs/cmds/xfstests git clone git://oss.sgi.com/xfs/cmds/xfsprogs sudo yum install uuid-devel e2fsprogs-devel libuuid-devel libattr-devel libacl-devel cd xfsprogs make sudo make install-qa cd ../xfstests ./configure make (which failed with the symlink error above) On Sat, Nov 16, 2013 at 8:55 AM, Christoph Hellwig <hch@infradead.org> wrote: > On Fri, Nov 15, 2013 at 08:53:12PM -0600, Steve French wrote: >> From: Steve French <smfrench@gmail.com> >> Date: Fri, 15 Nov 2013 20:41:32 -0600 >> Subject: [PATCH] [CIFS] setfacl removes part of ACL when setting POSIX ACLs to >> Samba >> >> setfacl over cifs mounts can remove the default ACL when setting the >> (non-default part of) the ACL and vice versa (we were leaving at 0 >> rather than setting to -1 the count field for the unaffected >> half of the ACL. For example notice the setfacl removed >> the default ACL in this sequence: > > Can you send a patch to xfstests to make sure we regression test for > this issue on all filesystems? >
On Sat, Nov 16, 2013 at 9:15 AM, steve <steve@steve-ss.com> wrote: > >> > From: Steve French <smfrench@gmail.com> >> > Date: Fri, 15 Nov 2013 20:41:32 -0600 >> > Subject: [PATCH] [CIFS] setfacl removes part of ACL when setting POSIX ACLs to >> > Samba >> > >> > setfacl over cifs mounts can remove the default ACL > > Hi > cifs-utils 6.1 Samba 4.1.0 > > I don't understand the '...over cifs mounts...' bit. > > Does it mean that if I have a share mounted on my Linux box and do a > setfacl on a file in the share then the default acl will be removed? > > Sorry for a non dev question. I was surprised too. Over a cifs mount (to Samba, as Windows does not support the POSIX style ACLs) it looks like many users paid attention to only half the POSIX ACL, adding additional users or groups to the (non-default) ACL and ignoring the default ACL or vice versa. A user reported the problem (with part of the ACL getting removed on setfacl over a cifs mount) a couple days ago, and I found one older reference to a similar problem.
Hi, I can show you this bug. A share mounted from a Samba server 3.6.3 (Ubuntu 12.04 Server) on Ubuntu 12.04 with kernel 3.2, cifs 1.76. $ getfacl test # file: test # owner: parzerpeter # group: domänen-benutzer user::rwx group::--- group:kjp-admins:rwx mask::rwx other::--- default:user::rwx default:group::--- default:group:kjp-admins:rwx default:mask::rwx default:other::--- $ setfacl -m u:kjptest:rx test $ getfacl test # file: test # owner: parzerpeter # group: domänen-benutzer user::rwx user:kjptest:r-x group::--- group:kjp-admins:rwx mask::rwx other::--- $ setfacl -m d:g:kjp-admins:rwx test $ getfacl test # file: test # owner: parzerpeter # group: domänen-benutzer user::rwx group::--- other::--- default:user::rwx default:group::--- default:group:kjp-admins:rwx default:mask::rwx default:other::--- When changing the ACLs, the defaults are removed, and when changing the defaults the ACLs are removed. Peter
On Sat, Nov 16, 2013 at 03:38:09PM -0600, Steve French wrote: > Makes sense to add a setfacl/getfacl test to xfstest and was trying to > build updated xfstests and look at what has changed but ran into a > strange error building xfstests and didn't see an obvious answer when > googling for it. Any idea how to workaround the build failure? > > Building src > [DEP] > [CC] dirstress > gcc: error: /lib64/libhandle.so: Too many levels of symbolic links > > > These are the steps I went through from a fairly clean Fedora 19 64 > system before the make failure: No idea. Maybe some of the RedHat people on the xfs list have more experience with Fedora than I have. > > git clone git://oss.sgi.com/xfs/cmds/xfstests > git clone git://oss.sgi.com/xfs/cmds/xfsprogs > sudo yum install uuid-devel e2fsprogs-devel libuuid-devel > libattr-devel libacl-devel > cd xfsprogs > make > sudo make install-qa > cd ../xfstests > ./configure > make (which failed with the symlink error above) -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 18, 2013 at 07:27:23AM -0800, Christoph Hellwig wrote: > On Sat, Nov 16, 2013 at 03:38:09PM -0600, Steve French wrote: > > Makes sense to add a setfacl/getfacl test to xfstest and was trying to > > build updated xfstests and look at what has changed but ran into a > > strange error building xfstests and didn't see an obvious answer when > > googling for it. Any idea how to workaround the build failure? > > > > Building src > > [DEP] > > [CC] dirstress > > gcc: error: /lib64/libhandle.so: Too many levels of symbolic links What's the circular link chain? > > These are the steps I went through from a fairly clean Fedora 19 64 > > system before the make failure: > > No idea. Maybe some of the RedHat people on the xfs list have more > experience with Fedora than I have. Don't look at me - I use Debian for all my upstream stuff.... ;) > > git clone git://oss.sgi.com/xfs/cmds/xfstests > > git clone git://oss.sgi.com/xfs/cmds/xfsprogs > > sudo yum install uuid-devel e2fsprogs-devel libuuid-devel > > libattr-devel libacl-devel > > cd xfsprogs > > make > > sudo make install-qa > > cd ../xfstests > > ./configure > > make (which failed with the symlink error above) Having the output of all the steps, especially the xfsprogs install-qa step is probably going to be necessary to debug this. You probably want to run "make Q= install-qa" so that it runs verbosely. Cheers, Dave.
On Nov 18, 2013, at 9:27 AM, Christoph Hellwig <hch@infradead.org> wrote: > >> On Sat, Nov 16, 2013 at 03:38:09PM -0600, Steve French wrote: >> Makes sense to add a setfacl/getfacl test to xfstest and was trying to >> build updated xfstests and look at what has changed but ran into a >> strange error building xfstests and didn't see an obvious answer when >> googling for it. Any idea how to workaround the build failure? >> >> Building src >> [DEP] >> [CC] dirstress >> gcc: error: /lib64/libhandle.so: Too many levels of symbolic links >> >> >> These are the steps I went through from a fairly clean Fedora 19 64 >> system before the make failure: > > No idea. Maybe some of the RedHat people on the xfs list have more > experience with Fedora than I have. > Just FWIW fedora has xfsprogs-devel and xfsprogs-qa-devel rpms which should satisfy xfstests for that part of the deps. I've not done a qa-install manually on fedora since forever... Eric-- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From 5e8b4b2934093b1e8ec6e8ef30f110cd57e0dbdf Mon Sep 17 00:00:00 2001 From: Steve French <smfrench@gmail.com> Date: Fri, 15 Nov 2013 20:41:32 -0600 Subject: [PATCH] [CIFS] setfacl removes part of ACL when setting POSIX ACLs to Samba setfacl over cifs mounts can remove the default ACL when setting the (non-default part of) the ACL and vice versa (we were leaving at 0 rather than setting to -1 the count field for the unaffected half of the ACL. For example notice the setfacl removed the default ACL in this sequence: steven@steven-GA-970A-DS3:~/cifs-2.6$ getfacl /mnt/test-dir ; setfacl -m default:user:test:rwx,user:test:rwx /mnt/test-dir getfacl: Removing leading '/' from absolute path names user::rwx group::r-x other::r-x default:user::rwx default:user:test:rwx default:group::r-x default:mask::rwx default:other::r-x steven@steven-GA-970A-DS3:~/cifs-2.6$ getfacl /mnt/test-dir getfacl: Removing leading '/' from absolute path names user::rwx user:test:rwx group::r-x mask::rwx other::r-x CC: Stable <stable@kernel.org> Signed-off-by: Steve French <smfrench@gmail.com> Acked-by: Jeremy Allison <jra@samba.org> --- fs/cifs/cifssmb.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 93b2947..124aa02 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -3369,11 +3369,13 @@ static __u16 ACL_to_cifs_posix(char *parm_data, const char *pACL, return 0; } cifs_acl->version = cpu_to_le16(1); - if (acl_type == ACL_TYPE_ACCESS) + if (acl_type == ACL_TYPE_ACCESS) { cifs_acl->access_entry_count = cpu_to_le16(count); - else if (acl_type == ACL_TYPE_DEFAULT) + cifs_acl->default_entry_count = __constant_cpu_to_le16(0xFFFF); + } else if (acl_type == ACL_TYPE_DEFAULT) { cifs_acl->default_entry_count = cpu_to_le16(count); - else { + cifs_acl->access_entry_count = __constant_cpu_to_le16(0xFFFF); + } else { cifs_dbg(FYI, "unknown ACL type %d\n", acl_type); return 0; } -- 1.8.3.1