setfacl fix

[Steve French]

From: Steve French <smfrench-***@public.gmane.org>
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-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-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-DgEjT+Ai2ygdnm+***@public.gmane.org>
Signed-off-by: Steve French <smfrench-***@public.gmane.org>
Acked-by: Jeremy Allison <jra-***@public.gmane.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;
}

[Jeremy Allison]

Post by Steve French
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

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.

[Christoph Hellwig]

Post by Steve French
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

Can you send a patch to xfstests to make sure we regression test for
this issue on all filesystems?

[steve]

Post by Steve French
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

[Steve French]

Post by steve

Post by Steve French
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.

[Parzer, Peter]

Hi,

I can show you this bug. A share mounted from a Samba server 3.6.3 (Ubu=
ntu 12.04 Server) on Ubuntu 12.04 with kernel 3.2, cifs 1.76.

$ getfacl test
# file: test
# owner: parzerpeter
# group: dom=E4nen-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=E4nen-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=E4nen-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
________________________________________
Von: linux-cifs-owner-***@public.gmane.org [linux-cifs-owner-***@public.gmane.org=
]&quot; im Auftrag von &quot;Steve French [smfrench-***@public.gmane.org]
Gesendet: Samstag, 16. November 2013 22:55
An: steve
Cc: Christoph Hellwig; linux-cifs-***@public.gmane.org; samba-technical
Betreff: Re: setfacl fix

Post by steve

Post by Steve French
Date: Fri, 15 Nov 2013 20:41:32 -0600
Subject: [PATCH] [CIFS] setfacl removes part of ACL when setting P=

OSIX ACLs to

Post by steve

Post by Steve French
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.

--
Thanks,

Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" i=
n
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Steve French]

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)

Post by Christoph Hellwig

Post by Steve French
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

Can you send a patch to xfstests to make sure we regression test for
this issue on all filesystems?

[Christoph Hellwig]

Post by Steve French
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

No idea. Maybe some of the RedHat people on the xfs list have more
experience with Fedora than I have.

Post by Steve French
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)

[Dave Chinner]

Post by Christoph Hellwig

Post by Steve French
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?

Post by Christoph Hellwig

Post by Steve French
These are the steps I went through from a fairly clean Fedora 19 64

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.... ;)

Post by Christoph Hellwig

Post by Steve French
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.

[Eric Sandeen]

Post by Christoph Hellwig

Post by Steve French
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

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