Hi Trond & Christoph,
It's still broken, but in a different way.
The phantom attrs are gone, but the attr/acl interaction is still
uncertain.
I have tested vanilla 3.14.5 + this patch on x86_64.
Mount options are the same as last time (NFSv3).
This is what I see on the client:
nfsv3client% mkdir x
nfsv3client% cd x
nfsv3client% getfattr -m '.*' .
nfsv3client% getfacl .
# file: .
# owner: phil
# group: phil
user::rwx
group::rwx
other::r-x
OK so far: no more phantom attrs.
This is where things get dodgy:
nfsv3client% setfacl -m u:root:r .
nfsv3client% getfacl .
# file: .
# owner: phil
# group: phil
user::rwx
user:root:r--
group::rwx
mask::rwx
other::r-x
nfsv3client% getfattr -m '.*' .
[1] 2123 segmentation fault getfattr -m '.*' .
% strace getfattr -m '.*' . 2>&1 | tail -n 20
fstat(3, {st_mode=S_IFREG|0644, st_size=26254, ...}) = 0
mmap(NULL, 26254, PROT_READ, MAP_SHARED, 3, 0) = 0x7f46a1450000
close(3) = 0
getrlimit(RLIMIT_NOFILE, {rlim_cur=1024, rlim_max=4*1024}) = 0
lstat(".", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0
listxattr(".", NULL, 0) = 23
listxattr(".", "system.posix_acl_access", 256) = 23
brk(0) = 0x1138000
brk(0x1178000) = 0x1178000
brk(0) = 0x1178000
brk(0) = 0x1178000
brk(0x1159000) = 0x1159000
brk(0) = 0x1159000
mmap(NULL, 266240, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f46a140f000
brk(0) = 0x1159000
brk(0) = 0x1159000
brk(0x1139000) = 0x1139000
brk(0) = 0x1139000
--- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x11586e8} ---
+++ killed by SIGSEGV +++
[1] 2311 segmentation fault strace getfattr -m '.*' . 2>&1
|
2312 done tail -n 20
I have no idea get getfattr crashes right after the listxattr() syscall,
but it surely doesn't on the NFSv3 server nor with 3.13.
A quick check on the NFS server shows the the ACLs are correctly set:
nfsv3server% cd /path/to/x
nfsv3server% getfacl .
# file: .
# owner: phil
# group: phil
user::rwx
user:root:r--
group::rwx
mask::rwx
other::r-x
nfsv3server% getfattr -m '.*' .
# file: .
system.posix_acl_access
Back on the client, clearing the ACL confuses the client further:
nfsv3client% setfacl -b .
nfsv3client% getfacl .
# file: .
# owner: phil
# group: phil
user::rwx
group::rwx
other::r-x
nfsv3client% strace getfattr -m '.*' . 2>&1 | tail -n 20
fstat(3, {st_mode=S_IFREG|0644, st_size=26254, ...}) = 0
mmap(NULL, 26254, PROT_READ, MAP_SHARED, 3, 0) = 0x7fc7e3f9a000
close(3) = 0
getrlimit(RLIMIT_NOFILE, {rlim_cur=1024, rlim_max=4*1024}) = 0
lstat(".", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0
listxattr(".", NULL, 0) = 23
listxattr(".", "system.posix_acl_access", 256) = 23
brk(0) = 0x1655000
brk(0x1695000) = 0x1695000
brk(0) = 0x1695000
brk(0) = 0x1695000
brk(0x1676000) = 0x1676000
brk(0) = 0x1676000
mmap(NULL, 266240, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fc7e3f59000
brk(0) = 0x1676000
brk(0) = 0x1676000
brk(0x1656000) = 0x1656000
brk(0) = 0x1656000
--- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x16756e8} ---
+++ killed by SIGSEGV +++
[1] 2353 segmentation fault strace getfattr -m '.*' . 2>&1
|
2354 done tail -n 20
nfsv3client% getfattr -n system.posix_acl_access .
# file: .
system.posix_acl_access=0sAgAAAAEABwD/////BAAHAP////8gAAUA/////w==
See how:
* getfacl says there's no ACLs
* getfattr says there's still a system.posix_acl_access attr.
Interestingly, the server says otherwise:
nfsv3server% getfattr -m '.*' .
nfsv3server% getfattr -n system.posix_acl_access .
.: system.posix_acl_access: No such attribute
[1] 2233 exit 1 getfattr -n system.posix_acl_access .
nfsv3server% getfacl .
# file: .
# owner: phil
# group: phil
user::rwx
group::rwx
other::r-x
Phil.
On Sat, 2014-06-07 at 07:04 -0700, Christoph Hellwig wrote:
> On Fri, Jun 06, 2014 at 04:37:03PM -0400, Trond Myklebust wrote:
> > Christoph, what is the intended interface for telling
> > posix_acl_xattr_list() that there are no acls on a particular file?
> > Should there perhaps be a call to get_acl()?
>
> The interface is to not call posix_acl_xattr_list unless you have ACLs.
> Every implementation does this, except for generic_listxattr which is
> only used by NFS.
>
> Philippe, can you test the patch below?
>
>
> diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
> index 871d6ed..e083827 100644
> --- a/fs/nfs/nfs3acl.c
> +++ b/fs/nfs/nfs3acl.c
> @@ -247,3 +247,45 @@ const struct xattr_handler *nfs3_xattr_handlers[] = {
> &posix_acl_default_xattr_handler,
> NULL,
> };
> +
> +static int
> +nfs3_list_one_acl(struct inode *inode, int type, const char *name, void *data,
> + size_t size, ssize_t *result)
> +{
> + struct posix_acl *acl;
> + char *p = data + *result;
> +
> + acl = get_acl(inode, type);
> + if (!acl)
> + return 0;
> +
> + posix_acl_release(acl);
> +
> + *result += strlen(name);
> + if (!size)
> + return 0;
> + if (*result > size)
> + return -ERANGE;
> +
> + strcpy(p, name);
> + return 0;
> +}
> +
> +ssize_t
> +nfs3_listxattr(struct dentry *dentry, char *data, size_t size)
> +{
> + struct inode *inode = dentry->d_inode;
> + ssize_t result = 0;
> + int error;
> +
> + error = nfs3_list_one_acl(inode, ACL_TYPE_ACCESS,
> + POSIX_ACL_XATTR_ACCESS, data, size, &result);
> + if (error)
> + return error;
> +
> + error = nfs3_list_one_acl(inode, ACL_TYPE_DEFAULT,
> + POSIX_ACL_XATTR_DEFAULT, data, size, &result);
> + if (error)
> + return error;
> + return result;
> +}
> diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
> index db60149..0e2bb26 100644
> --- a/fs/nfs/nfs3proc.c
> +++ b/fs/nfs/nfs3proc.c
> @@ -891,7 +891,7 @@ static const struct inode_operations nfs3_dir_inode_operations = {
> .getattr = nfs_getattr,
> .setattr = nfs_setattr,
> #ifdef CONFIG_NFS_V3_ACL
> - .listxattr = generic_listxattr,
> + .listxattr = nfs3_listxattr,
> .getxattr = generic_getxattr,
> .setxattr = generic_setxattr,
> .removexattr = generic_removexattr,
> @@ -905,7 +905,7 @@ static const struct inode_operations nfs3_file_inode_operations = {
> .getattr = nfs_getattr,
> .setattr = nfs_setattr,
> #ifdef CONFIG_NFS_V3_ACL
> - .listxattr = generic_listxattr,
> + .listxattr = nfs3_listxattr,
> .getxattr = generic_getxattr,
> .setxattr = generic_setxattr,
> .removexattr = generic_removexattr,
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
