Skip to contentSkip to conversation

NFS client broken in Linus' tip

DISCUSSION Russell King - ARM Linux 5 participants 26 messages Jan 30, 2014 – Feb 3, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

I just booted Linus' tip (plus a few other patches to imx-drm and imx code), and stumbled into this interesting scenario:

# touch test
touch: cannot touch `test': Operation not supported

I also tried mkdir and mknod, all result in the same error. Hard and symlinks links are creatable.

However, I can chmod existing files and rename them. Files can also be deleted, and the combination of this has left me without a /etc/mtab !

The machine is a iMX6 based ARM, running root-nfs, which was mounted via ubuntu's initramfs (so not using the kernel's built-in root-nfs.)

/proc/mounts for the root mount gives:
192.168.1.123:/var/boot/ci / nfs rw,relatime,vers=3,rsize=65536,wsize=65536,namlen=255,hard,nolock,proto=tcp,port=2049,timeo=7,retrans=10,sec=sys,local_lock=all,addr=192.168.1.123 0 0

CONFIG_NFS_FS=y
CONFIG_NFS_V2=y
CONFIG_NFS_V3=y
CONFIG_NFS_V3_ACL=y
CONFIG_NFS_V4=y
# CONFIG_NFS_SWAP is not set
# CONFIG_NFS_V4_1 is not set
CONFIG_ROOT_NFS=y
# CONFIG_NFS_USE_LEGACY_DNS is not set
CONFIG_NFS_USE_KERNEL_DNS=y
# CONFIG_NFSD is not set
CONFIG_LOCKD=y
CONFIG_LOCKD_V4=y
CONFIG_NFS_ACL_SUPPORT=y
CONFIG_NFS_COMMON=y
CONFIG_SUNRPC=y
CONFIG_SUNRPC_GSS=y

tcpdumping, I see:

13:59:51.713523 IP 192.168.1.252.1341245608 > 192.168.1.123.2049: 132 lookup fh Unknown/010007011040840000000000CC238FC8FBA0475D9D9F8356B4C44166CDC38700 "test" 13:59:51.714345 IP 192.168.1.123.2049 > 192.168.1.252.1341245608: reply ok 120 lookup ERROR: No such file or directory 13:59:51.751303 IP 192.168.1.252.797 > 192.168.1.123.nfs: . ack 3381 win 2625 <nop,nop,timestamp 474136 3431312924>

which is the only NFS packet(s) I see which mention "test".

and stracing touch:

open("test", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK|O_LARGEFILE, 0666) = -1 EOPNOTSUPP (Operation not supported)
utimensat(AT_FDCWD, "test", NULL, 0) = -1 ENOENT (No such file or directory)
write(2, "touch: ", 7touch: ) = 7
write(2, "cannot touch test'", 19cannot touch test') = 19
write(2, ": Operation not supported", 25: Operation not supported) = 25
write(2, "\n", 1
) = 1

I think it's down to this:

commit 013cdf1088d7235da9477a2375654921d9b9ba9f

Author: Christoph Hellwig <[email protected]>
Date:   Fri Dec 20 05:16:53 2013 -0800

nfs: use generic posix ACL infrastructure for v3 Posix ACLs

This causes a small behaviour change in that we don't bother to set ACLs on file creation if the mode bit can express the access permissions fully, and thus behaving identical to local filesystems.

which adds:

+ status = posix_acl_create(dir, &sattr->ia_mode, &default_acl, &acl);
+ if (status)
+ goto out;

into nfs3_proc_create(), but this ends up calling down into nfs3_get_acl(), which does this:

if (!nfs_server_capable(inode, NFS_CAP_ACLS))
return ERR_PTR(-EOPNOTSUPP);

Signature · 3 lines
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
#1 lore raw
Quoted 5 lines · Russell King - ARM Linux

into nfs3_proc_create(), but this ends up calling down into nfs3_get_acl(), which does this:

if (!nfs_server_capable(inode, NFS_CAP_ACLS))
return ERR_PTR(-EOPNOTSUPP);

Does replacing that return with a

		return NULL;

fix the issue for you?

#2 lore raw
Quoted 4 lines · Christoph Hellwig
Quoted 3 lines · Russell King - ARM Linux

into nfs3_proc_create(), but this ends up calling down into nfs3_get_acl(), which does this:

Quoted 8 lines
Quoted 2 lines

if (!nfs_server_capable(inode, NFS_CAP_ACLS))
return ERR_PTR(-EOPNOTSUPP);

Does replacing that return with a

		return NULL;

fix the issue for you?

Yes and no. I still end up with an empty /etc/mtab, but the file now exists. However, I can create and echo data into /etc/mtab, but it seems that can't happen at boot time.

I also find that kerberos doesn't work - again, it's weird - I can create the file which kerberos is trying to create in /var/tmp (which is also on the root nfs) manually, but when the kerberos daemons try to create it, it fails with -EOPNOTSUPP. The kerberos daemon is running as uid 0 at that point (I don't know about the other uids/gids though since I wasn't able to catch them in the strace.)

Signature · 3 lines
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
#3 lore raw
Quoted 3 lines · Russell King - ARM Linux

Yes and no. I still end up with an empty /etc/mtab, but the file now exists. However, I can create and echo data into /etc/mtab, but it seems that can't happen at boot time.

Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?

I'll be out for a long weekend soon, so I have very limited time to debug things until Monday.

#4 lore raw
Quoted 6 lines · Christoph Hellwig
Quoted 3 lines · Russell King - ARM Linux

Yes and no. I still end up with an empty /etc/mtab, but the file now exists. However, I can create and echo data into /etc/mtab, but it seems that can't happen at boot time.

Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?

Unfortunately, that results in some problem at boot time, which ultimately ends up with the other three CPUs being stopped, and hence the original reason scrolls off the screen before it can be read... even at 1920p.

Signature · 3 lines
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
#5 lore raw
Quoted 6 lines · Russell King - ARM Linux
Quoted 5 lines · Christoph Hellwig
Quoted 3 lines · Russell King - ARM Linux

Yes and no. I still end up with an empty /etc/mtab, but the file now exists. However, I can create and echo data into /etc/mtab, but it seems that can't happen at boot time.

Quoted 7 lines
Quoted 1 line

Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?

Unfortunately, that results in some problem at boot time, which ultimately ends up with the other three CPUs being stopped, and hence the original reason scrolls off the screen before it can be read... even at 1920p.

Hi Russell,

The following patch fixes the issue for me.

Cheers
Trond

8<-------------------------------------------------------------

Quoted 1 line

From 59bc20fe862bd85fcad61427e8669603e789d163 Mon Sep 17 00:00:00 2001

From: Trond Myklebust <[email protected]>
Date: Fri, 31 Jan 2014 14:25:19 -0500
Subject: [PATCH] fs: get_acl() must be allowed to return EOPNOTSUPP

posix_acl_xattr_get requires get_acl() to return EOPNOTSUPP if the filesystem cannot support acls. This is needed for NFS, which can't know whether or not the server supports acls until it tries to get/set one.
This patch converts posix_acl_chmod and posix_acl_create to deal with EOPNOTSUPP return values from get_acl().

Cc Christoph Hellwig <[email protected]>
Cc Al Viro viro at zeniv.linux.org.uk>
---
 fs/posix_acl.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
fs/posix_acl.c +12 -6
index 38bae5a0ea25..11c54fd51e16 100644
@@ -521,8 +521,11 @@ posix_acl_chmod(struct inode *inode, umode_t mode)
 		return -EOPNOTSUPP;
 
 	acl = get_acl(inode, ACL_TYPE_ACCESS);
-	if (IS_ERR_OR_NULL(acl))
+	if (IS_ERR_OR_NULL(acl)) {
+		if (acl == ERR_PTR(-EOPNOTSUPP))
+			return 0;
 		return PTR_ERR(acl);
+	}
 
 	ret = __posix_acl_chmod(&acl, GFP_KERNEL, mode);
 	if (ret)
@@ -544,14 +547,15 @@ posix_acl_create(struct inode *dir, umode_t *mode,
 		goto no_acl;
 
 	p = get_acl(dir, ACL_TYPE_DEFAULT);
-	if (IS_ERR(p))
+	if (IS_ERR(p)) {
+		if (p == ERR_PTR(-EOPNOTSUPP))
+			goto apply_umask;
 		return PTR_ERR(p);
-
-	if (!p) {
-		*mode &= ~current_umask();
-		goto no_acl;
 	}
 
+	if (!p)
+		goto apply_umask;
+
 	*acl = posix_acl_clone(p, GFP_NOFS);
 	if (!*acl)
 		return -ENOMEM;
@@ -575,6 +579,8 @@ posix_acl_create(struct inode *dir, umode_t *mode,
 	}
 	return 0;
 
+apply_umask:
+	*mode &= ~current_umask();
 no_acl:
 	*default_acl = NULL;
 	*acl = NULL;
Signature · 7 lines
1.8.5.3



-- 
Trond Myklebust
Linux NFS client maintainer
#6 lore raw
Quoted 9 lines · Trond Myklebust
Quoted 8 lines · Russell King - ARM Linux
Quoted 6 lines · Christoph Hellwig
Quoted 3 lines · Russell King - ARM Linux

Yes and no. I still end up with an empty /etc/mtab, but the file now exists. However, I can create and echo data into /etc/mtab, but it seems that can't happen at boot time.

Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?

Quoted 8 lines
Quoted 5 lines

Unfortunately, that results in some problem at boot time, which ultimately ends up with the other three CPUs being stopped, and hence the original reason scrolls off the screen before it can be read... even at 1920p.

Hi Russell,

The following patch fixes the issue for me.

It doesn't entirely fix the issue for me, instead we've got even weirder behaviour:

root at cubox-i4:~# ls -al test
ls: cannot access test: No such file or directory
root at cubox-i4:~# touch test
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test
root at cubox-i4:~# echo foo > test
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 4 Feb 1 01:01 test
root at cubox-i4:~# cat test
foo
root at cubox-i4:~# rm test
root at cubox-i4:~# echo foo > test
-bash: test: Operation not supported
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test

Signature · 3 lines
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
#7 lore raw
Quoted 22 lines · Russell King - ARM Linux
Quoted 9 lines · Trond Myklebust
Quoted 8 lines · Russell King - ARM Linux
Quoted 6 lines · Christoph Hellwig
Quoted 3 lines · Russell King - ARM Linux

Yes and no. I still end up with an empty /etc/mtab, but the file now exists. However, I can create and echo data into /etc/mtab, but it seems that can't happen at boot time.

Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?

Quoted 8 lines
Quoted 5 lines

Unfortunately, that results in some problem at boot time, which ultimately ends up with the other three CPUs being stopped, and hence the original reason scrolls off the screen before it can be read... even at 1920p.

Hi Russell,

The following patch fixes the issue for me.

It doesn't entirely fix the issue for me, instead we've got even weirder behaviour:

Quoted 15 lines

root at cubox-i4:~# ls -al test
ls: cannot access test: No such file or directory
root at cubox-i4:~# touch test
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test
root at cubox-i4:~# echo foo > test
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 4 Feb 1 01:01 test
root at cubox-i4:~# cat test
foo
root at cubox-i4:~# rm test
root at cubox-i4:~# echo foo > test
-bash: test: Operation not supported
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test

FYI, I just tested Linus' tip, and NFS is still broken.

Signature · 3 lines
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
#8 lore raw
Quoted 32 lines · Russell King - ARM Linux
Quoted 31 lines · Russell King - ARM Linux
Quoted 9 lines · Trond Myklebust
Quoted 8 lines · Russell King - ARM Linux
Quoted 6 lines · Christoph Hellwig
Quoted 3 lines · Russell King - ARM Linux

Yes and no. I still end up with an empty /etc/mtab, but the file now exists. However, I can create and echo data into /etc/mtab, but it seems that can't happen at boot time.

Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?

Quoted 8 lines
Quoted 5 lines

Unfortunately, that results in some problem at boot time, which ultimately ends up with the other three CPUs being stopped, and hence the original reason scrolls off the screen before it can be read... even at 1920p.

Hi Russell,

The following patch fixes the issue for me.

It doesn't entirely fix the issue for me, instead we've got even weirder behaviour:

root at cubox-i4:~# ls -al test
ls: cannot access test: No such file or directory
root at cubox-i4:~# touch test
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test
root at cubox-i4:~# echo foo > test
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 4 Feb 1 01:01 test
root at cubox-i4:~# cat test

Quoted 9 lines
Quoted 6 lines

foo
root at cubox-i4:~# rm test
root at cubox-i4:~# echo foo > test
-bash: test: Operation not supported
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test

FYI, I just tested Linus' tip, and NFS is still broken.

Hi Russell,

The following patch should fix the above problem. It needs to be applied on top of the one I sent you previously. In addition, you will want to apply Noah Massey's patch from
http://lkml.kernel.org/r/1391135472-9639-1-git-send-email-Noah.Massey at gmail.com

Cheers,
Trond

8<------------------------------------------------------------------------------

Quoted 1 line

From f9ad7a7b43554bc36eccf03d33617c05b8a2c77d Mon Sep 17 00:00:00 2001

From: Trond Myklebust <[email protected]>
Date: Sun, 2 Feb 2014 14:36:42 -0500
Subject: [PATCH] NFSv3: Fix return value of nfs3_proc_setacls

nfs3_proc_setacls is used internally by the NFSv3 create operations to set the acl after the file has been created. If the operation fails because the server doesn't support acls, then it must return '0', not -EOPNOTSUPP.

---
 fs/nfs/nfs3acl.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
fs/nfs/nfs3acl.c +11 -2
index 9271a6bb9a41..871d6eda8dba 100644
@@ -113,7 +113,7 @@ getout:
 	return ERR_PTR(status);
 }
 
-int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
+static int __nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
 		struct posix_acl *dfacl)
 {
 	struct nfs_server *server = NFS_SERVER(inode);
@@ -198,6 +198,15 @@ out:
 	return status;
 }
 
+int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
+		struct posix_acl *dfacl)
+{
+	int ret;
+	ret = __nfs3_proc_setacls(inode, acl, dfacl);
+	return (ret == -EOPNOTSUPP) ? 0 : ret;
+
+}
+
 int nfs3_set_acl(struct inode *inode, struct posix_acl *acl, int type)
 {
 	struct posix_acl *alloc = NULL, *dfacl = NULL;
@@ -225,7 +234,7 @@ int nfs3_set_acl(struct inode *inode, struct posix_acl *acl, int type)
 		if (IS_ERR(alloc))
 			goto fail;
 	}
-	status = nfs3_proc_setacls(inode, acl, dfacl);
+	status = __nfs3_proc_setacls(inode, acl, dfacl);
 	posix_acl_release(alloc);
 	return status;
 
Signature · 6 lines
1.8.5.3


-- 
Trond Myklebust
Linux NFS client maintainer
Tested-by
#9 lore raw

At Sun, 02 Feb 2014 17:04:38 -0500,

Quoted 43 lines · Trond Myklebust
Quoted 32 lines · Russell King - ARM Linux
Quoted 31 lines · Russell King - ARM Linux
Quoted 9 lines · Trond Myklebust
Quoted 8 lines · Russell King - ARM Linux
Quoted 6 lines · Christoph Hellwig
Quoted 3 lines · Russell King - ARM Linux

Yes and no. I still end up with an empty /etc/mtab, but the file now exists. However, I can create and echo data into /etc/mtab, but it seems that can't happen at boot time.

Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?

Quoted 8 lines
Quoted 5 lines

Unfortunately, that results in some problem at boot time, which ultimately ends up with the other three CPUs being stopped, and hence the original reason scrolls off the screen before it can be read... even at 1920p.

Hi Russell,

The following patch fixes the issue for me.

It doesn't entirely fix the issue for me, instead we've got even weirder behaviour:

root at cubox-i4:~# ls -al test
ls: cannot access test: No such file or directory
root at cubox-i4:~# touch test
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test
root at cubox-i4:~# echo foo > test
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 4 Feb 1 01:01 test
root at cubox-i4:~# cat test

Quoted 9 lines
Quoted 6 lines

foo
root at cubox-i4:~# rm test
root at cubox-i4:~# echo foo > test
-bash: test: Operation not supported
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test

FYI, I just tested Linus' tip, and NFS is still broken.

Quoted 4 lines

Hi Russell,

The following patch should fix the above problem. It needs to be applied on top of the one I sent you previously.

I've hit the same problem, and your two patches seem fixing it. I tested them on top of 3.14-rc1. Feel free to take my tested-by tag

Quoted 3 lines

In addition, you will want to
apply Noah Massey's patch from
http://lkml.kernel.org/r/1391135472-9639-1-git-send-email-Noah.Massey at gmail.com

Do I still need to test this one, too?

Thanks!

Takashi

#10 lore raw
Quoted 53 lines · Takashi Iwai

At Sun, 02 Feb 2014 17:04:38 -0500,

Quoted 43 lines · Trond Myklebust
Quoted 32 lines · Russell King - ARM Linux
Quoted 31 lines · Russell King - ARM Linux
Quoted 9 lines · Trond Myklebust
Quoted 8 lines · Russell King - ARM Linux
Quoted 6 lines · Christoph Hellwig
Quoted 3 lines · Russell King - ARM Linux

Yes and no. I still end up with an empty /etc/mtab, but the file now exists. However, I can create and echo data into /etc/mtab, but it seems that can't happen at boot time.

Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?

Quoted 8 lines
Quoted 5 lines

Unfortunately, that results in some problem at boot time, which ultimately ends up with the other three CPUs being stopped, and hence the original reason scrolls off the screen before it can be read... even at 1920p.

Hi Russell,

The following patch fixes the issue for me.

It doesn't entirely fix the issue for me, instead we've got even weirder behaviour:

root at cubox-i4:~# ls -al test
ls: cannot access test: No such file or directory
root at cubox-i4:~# touch test
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test
root at cubox-i4:~# echo foo > test
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 4 Feb 1 01:01 test
root at cubox-i4:~# cat test

Quoted 9 lines
Quoted 6 lines

foo
root at cubox-i4:~# rm test
root at cubox-i4:~# echo foo > test
-bash: test: Operation not supported
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test

FYI, I just tested Linus' tip, and NFS is still broken.

Quoted 4 lines

Hi Russell,

The following patch should fix the above problem. It needs to be applied on top of the one I sent you previously.

I've hit the same problem, and your two patches seem fixing it. I tested them on top of 3.14-rc1. Feel free to take my tested-by tag

Quoted 9 lines
Quoted 3 lines

In addition, you will want to
apply Noah Massey's patch from
http://lkml.kernel.org/r/1391135472-9639-1-git-send-email-Noah.Massey at gmail.com

Do I still need to test this one, too?

Hi Takashi,

Noah?s patch is not a replacement for the above 2 patches; it fixes a different problem. All three patches are therefore required for a complete solution.

Cheers
Trond

--
Trond Myklebust
Linux NFS client maintainer

#11 lore raw

At Mon, 3 Feb 2014 09:21:16 -0500,

Quoted 64 lines · Trond Myklebust
Quoted 60 lines · Takashi Iwai

At Sun, 02 Feb 2014 17:04:38 -0500,

Quoted 43 lines · Trond Myklebust
Quoted 32 lines · Russell King - ARM Linux
Quoted 31 lines · Russell King - ARM Linux
Quoted 9 lines · Trond Myklebust
Quoted 8 lines · Russell King - ARM Linux
Quoted 6 lines · Christoph Hellwig
Quoted 3 lines · Russell King - ARM Linux

Yes and no. I still end up with an empty /etc/mtab, but the file now exists. However, I can create and echo data into /etc/mtab, but it seems that can't happen at boot time.

Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?

Quoted 8 lines
Quoted 5 lines

Unfortunately, that results in some problem at boot time, which ultimately ends up with the other three CPUs being stopped, and hence the original reason scrolls off the screen before it can be read... even at 1920p.

Hi Russell,

The following patch fixes the issue for me.

It doesn't entirely fix the issue for me, instead we've got even weirder behaviour:

root at cubox-i4:~# ls -al test
ls: cannot access test: No such file or directory
root at cubox-i4:~# touch test
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test
root at cubox-i4:~# echo foo > test
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 4 Feb 1 01:01 test
root at cubox-i4:~# cat test

Quoted 9 lines
Quoted 6 lines

foo
root at cubox-i4:~# rm test
root at cubox-i4:~# echo foo > test
-bash: test: Operation not supported
root at cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test

FYI, I just tested Linus' tip, and NFS is still broken.

Quoted 4 lines

Hi Russell,

The following patch should fix the above problem. It needs to be applied on top of the one I sent you previously.

I've hit the same problem, and your two patches seem fixing it. I tested them on top of 3.14-rc1. Feel free to take my tested-by tag

Quoted 3 lines

In addition, you will want to
apply Noah Massey's patch from
http://lkml.kernel.org/r/1391135472-9639-1-git-send-email-Noah.Massey at gmail.com

Quoted 6 lines
Quoted 2 lines

Do I still need to test this one, too?

Hi Takashi,

Noah?s patch is not a replacement for the above 2 patches; it fixes a different problem. All three patches are therefore required for a complete solution.

OK, I'll test with Noah's patch.

thanks,

Takashi

#12 lore raw
In reply to Trond Myklebust
Quoted 6 lines · Trond Myklebust

posix_acl_xattr_get requires get_acl() to return EOPNOTSUPP if the filesystem cannot support acls. This is needed for NFS, which can't know whether or not the server supports acls until it tries to get/set one.
This patch converts posix_acl_chmod and posix_acl_create to deal with EOPNOTSUPP return values from get_acl().

Shouldn't NFS just return a NULL ACL here?

#13 lore raw
Quoted 9 lines · Christoph Hellwig
Quoted 6 lines · Trond Myklebust

posix_acl_xattr_get requires get_acl() to return EOPNOTSUPP if the filesystem cannot support acls. This is needed for NFS, which can't know whether or not the server supports acls until it tries to get/set one.
This patch converts posix_acl_chmod and posix_acl_create to deal with EOPNOTSUPP return values from get_acl().

Shouldn't NFS just return a NULL ACL here?

As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).

--
Trond Myklebust
Linux NFS client maintainer

#14 lore raw
Quoted 1 line · Trond Myklebust

As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).

Is it really the wrong answer? How does userspace care wether this server doesn't support ACLs at all or none is set? The resulting behavior is the same.

If there's a good reason to care we might have to go with your patch, but if we can avoid it I'd prefer to keep things simple.

#15 lore raw
Quoted 6 lines · Christoph Hellwig
Quoted 1 line · Trond Myklebust

As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).

Is it really the wrong answer? How does userspace care wether this server doesn't support ACLs at all or none is set? The resulting behavior is the same.

It will certainly cause acl_get_file() to behave differently than previously. I?ve no idea how that will affect applications, though.

Quoted 2 lines

If there's a good reason to care we might have to go with your patch, but if we can avoid it I'd prefer to keep things simple.

One alternative is to simply wrap posix_acl_xattr_get() in fs/nfs/nfs3acl.c, and have it check the value of nfs_server_capable(inode, NFS_CAP_ACLS) before returning ENODATA. That?s rather ugly too...

--
Trond Myklebust
Linux NFS client maintainer

#16 lore raw
Quoted 9 lines · Trond Myklebust
Quoted 6 lines · Christoph Hellwig
Quoted 1 line · Trond Myklebust

As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).

Is it really the wrong answer? How does userspace care wether this server doesn't support ACLs at all or none is set? The resulting behavior is the same.

Quoted 6 lines

It will certainly cause acl_get_file() to behave differently than previously. I?ve no idea how that will affect applications, though.

Quoted 2 lines

If there's a good reason to care we might have to go with your patch, but if we can avoid it I'd prefer to keep things simple.

One alternative is to simply wrap posix_acl_xattr_get() in fs/nfs/nfs3acl.c, and have it check the value of nfs_server_capable(inode, NFS_CAP_ACLS) before returning ENODATA. That?s rather ugly too...

FWIW, here is the alternative patch. I've tested it, and it seems to work. 8<---------------------------------------------------------------------

Quoted 1 line

From 2e527b12169a67e9cfcf43898ae4d15bcfa1ede9 Mon Sep 17 00:00:00 2001

From: Trond Myklebust <[email protected]>
Date: Mon, 3 Feb 2014 13:07:05 -0500
Subject: [PATCH] NFSv3: Fix return values for get_acl()

Ensure that nfs3_get_acl() returns NULL when the server doesn't support POSIX acls. This ensures that posix_acl_create() does the right thing, and applies the current_umask() to the mode before returning.

Add a wrapper around posix_acl_xattr_get() so that we continue to return EOPNOTSUPP when the server doesn't support POSIX acls. Otherwise, the NULL return from nfs3_get_acl() will cause ENODATA to be returned.

Also add the appropriate exports to posix_acl_xattr_get, posix_acl_xattr_set and posix_acl_xattr_list to enable their use in the wrapper.

Cc Christoph Hellwig <[email protected]>
Cc Al Viro viro at zeniv.linux.org.uk>
---
 fs/nfs/nfs3acl.c                | 42 ++++++++++++++++++++++++++++++++++++-----
 fs/posix_acl.c                  |  9 ++++++---
 include/linux/posix_acl_xattr.h |  8 ++++++++
 3 files changed, 51 insertions(+), 8 deletions(-)
fs/nfs/nfs3acl.c +37 -5
index 871d6eda8dba..d1bc84f22f64 100644
@@ -28,8 +28,10 @@ struct posix_acl *nfs3_get_acl(struct inode *inode, int type)
 	};
 	int status, count;
 
-	if (!nfs_server_capable(inode, NFS_CAP_ACLS))
-		return ERR_PTR(-EOPNOTSUPP);
+	if (!nfs_server_capable(inode, NFS_CAP_ACLS)) {
+		cache_no_acl(inode);
+		return NULL;
+	}
 
 	status = nfs_revalidate_inode(server, inode);
 	if (status < 0)
@@ -70,7 +72,7 @@ struct posix_acl *nfs3_get_acl(struct inode *inode, int type)
 			dprintk("NFS_V3_ACL extension not supported; disabling\n");
 			server->caps &= ~NFS_CAP_ACLS;
 		case -ENOTSUPP:
-			status = -EOPNOTSUPP;
+			status = 0;
 		default:
 			goto getout;
 	}
@@ -242,8 +244,38 @@ fail:
 	return PTR_ERR(alloc);
 }
 
+static int
+nfs_posix_acl_xattr_get(struct dentry *dentry, const char *name,
+		void *value, size_t size, int type)
+{
+	int ret;
+
+	ret = posix_acl_xattr_get(dentry, name, value, size, type);
+	/*
+	 * This check is needed to override the ENODATA error that
+	 * posix_acl_xattr_get will return if the acl probe fails.
+	 */
+	if (!nfs_server_capable(dentry->d_inode, NFS_CAP_ACLS))
+		ret = -EOPNOTSUPP;
+	return ret;
+}
+
+static const struct xattr_handler nfs_posix_acl_access_xattr_handler = {
+	.prefix = POSIX_ACL_XATTR_ACCESS,
+	.flags = ACL_TYPE_ACCESS,
+	.get = nfs_posix_acl_xattr_get,
+	.set = posix_acl_xattr_set,
+};
+
+static const struct xattr_handler nfs_posix_acl_default_xattr_handler = {
+	.prefix = POSIX_ACL_XATTR_DEFAULT,
+	.flags = ACL_TYPE_DEFAULT,
+	.get = nfs_posix_acl_xattr_get,
+	.set = posix_acl_xattr_set,
+};
+
 const struct xattr_handler *nfs3_xattr_handlers[] = {
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
+	&nfs_posix_acl_access_xattr_handler,
+	&nfs_posix_acl_default_xattr_handler,
 	NULL,
 };
fs/posix_acl.c +6 -3
index 38bae5a0ea25..835167f92952 100644
@@ -750,7 +750,7 @@ posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
 }
 EXPORT_SYMBOL (posix_acl_to_xattr);
 
-static int
+int
 posix_acl_xattr_get(struct dentry *dentry, const char *name,
 		void *value, size_t size, int type)
 {
@@ -773,8 +773,9 @@ posix_acl_xattr_get(struct dentry *dentry, const char *name,
 
 	return error;
 }
+EXPORT_SYMBOL_GPL(posix_acl_xattr_get);
 
-static int
+int
 posix_acl_xattr_set(struct dentry *dentry, const char *name,
 		const void *value, size_t size, int flags, int type)
 {
@@ -809,8 +810,9 @@ out:
 	posix_acl_release(acl);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(posix_acl_xattr_set);
 
-static size_t
+size_t
 posix_acl_xattr_list(struct dentry *dentry, char *list, size_t list_size,
 		const char *name, size_t name_len, int type)
 {
@@ -832,6 +834,7 @@ posix_acl_xattr_list(struct dentry *dentry, char *list, size_t list_size,
 		memcpy(list, xname, size);
 	return size;
 }
+EXPORT_SYMBOL_GPL(posix_acl_xattr_list);
 
 const struct xattr_handler posix_acl_access_xattr_handler = {
 	.prefix = POSIX_ACL_XATTR_ACCESS,
include/linux/posix_acl_xattr.h +8 -0
index 6f14ee295822..fc062003a456 100644
@@ -69,6 +69,14 @@ struct posix_acl *posix_acl_from_xattr(struct user_namespace *user_ns,
 int posix_acl_to_xattr(struct user_namespace *user_ns,
 		       const struct posix_acl *acl, void *buffer, size_t size);
 
+int posix_acl_xattr_get(struct dentry *dentry, const char *name,
+			void *value, size_t size, int type);
+int posix_acl_xattr_set(struct dentry *dentry, const char *name,
+			const void *value, size_t size, int flags, int type);
+size_t posix_acl_xattr_list(struct dentry *dentry, char *list,
+			    size_t list_size, const char *name,
+			    size_t name_len, int type);
+
 extern const struct xattr_handler posix_acl_access_xattr_handler;
 extern const struct xattr_handler posix_acl_default_xattr_handler;
 
Signature · 6 lines
1.8.5.3


-- 
Trond Myklebust
Linux NFS client maintainer
#17 lore raw
Quoted 12 lines · Trond Myklebust
Quoted 11 lines · Trond Myklebust
Quoted 6 lines · Christoph Hellwig
Quoted 1 line · Trond Myklebust

As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).

Is it really the wrong answer? How does userspace care wether this server doesn't support ACLs at all or none is set? The resulting behavior is the same.

It will certainly cause acl_get_file() to behave differently than previously. I?ve no idea how that will affect applications, though.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

If there's a good reason to care we might have to go with your patch, but if we can avoid it I'd prefer to keep things simple.

One alternative is to simply wrap posix_acl_xattr_get() in fs/nfs/nfs3acl.c, and have it check the value of nfs_server_capable(inode, NFS_CAP_ACLS) before returning ENODATA. That?s rather ugly too...

FWIW, here is the alternative patch. I've tested it, and it seems to work.

Thanks.

As there's now two fixes, which would you like me to test?

One comment on this patch though:

fs/nfs/nfs3acl.c fail: · line 259

Quoted 13 lines
fs/nfs/nfs3acl.c +13 -0
+static int
+nfs_posix_acl_xattr_get(struct dentry *dentry, const char *name,
+		void *value, size_t size, int type)
+{
+	int ret;
+
+	ret = posix_acl_xattr_get(dentry, name, value, size, type);
+	/*
+	 * This check is needed to override the ENODATA error that
+	 * posix_acl_xattr_get will return if the acl probe fails.
+	 */
+	if (!nfs_server_capable(dentry->d_inode, NFS_CAP_ACLS))
+		ret = -EOPNOTSUPP;

I'm not familiar with this code, but the above looks slightly weird, and a little suspicious - especially with the lack of blank line before the comment. Is the above actually intended?

Signature · 3 lines
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
#18 lore raw
In reply to Trond Myklebust
Quoted 2 lines · Trond Myklebust

FWIW, here is the alternative patch. I've tested it, and it seems to work.

I much prefer the original one. One major point of the series was to get individual filesystems out of the business of providing xattr handlers for ACLs.

#19 lore raw
Quoted 8 lines · Christoph Hellwig
Quoted 2 lines · Trond Myklebust

FWIW, here is the alternative patch. I've tested it, and it seems to work.

I much prefer the original one. One major point of the series was to get individual filesystems out of the business of providing xattr handlers for ACLs.

Then could you and Al please provide an Acked-by? This is a userspace API, so we shouldn?t be changing return values without good reason.

--
Trond Myklebust
Linux NFS client maintainer

#21 lore raw
In reply to Russell King - ARM Linux
Quoted 71 lines · Russell King - ARM Linux

I just booted Linus' tip (plus a few other patches to imx-drm and imx code), and stumbled into this interesting scenario:

# touch test
touch: cannot touch `test': Operation not supported

I also tried mkdir and mknod, all result in the same error. Hard and symlinks links are creatable.

However, I can chmod existing files and rename them. Files can also be deleted, and the combination of this has left me without a /etc/mtab !

The machine is a iMX6 based ARM, running root-nfs, which was mounted via ubuntu's initramfs (so not using the kernel's built-in root-nfs.)

/proc/mounts for the root mount gives:
192.168.1.123:/var/boot/ci / nfs rw,relatime,vers=3,rsize=65536,wsize=65536,namlen=255,hard,nolock,proto=tcp,port=2049,timeo=7,retrans=10,sec=sys,local_lock=all,addr=192.168.1.123 0 0

CONFIG_NFS_FS=y
CONFIG_NFS_V2=y
CONFIG_NFS_V3=y
CONFIG_NFS_V3_ACL=y
CONFIG_NFS_V4=y
# CONFIG_NFS_SWAP is not set
# CONFIG_NFS_V4_1 is not set
CONFIG_ROOT_NFS=y
# CONFIG_NFS_USE_LEGACY_DNS is not set
CONFIG_NFS_USE_KERNEL_DNS=y
# CONFIG_NFSD is not set
CONFIG_LOCKD=y
CONFIG_LOCKD_V4=y
CONFIG_NFS_ACL_SUPPORT=y
CONFIG_NFS_COMMON=y
CONFIG_SUNRPC=y
CONFIG_SUNRPC_GSS=y

tcpdumping, I see:

13:59:51.713523 IP 192.168.1.252.1341245608 > 192.168.1.123.2049: 132 lookup fh Unknown/010007011040840000000000CC238FC8FBA0475D9D9F8356B4C44166CDC38700 "test" 13:59:51.714345 IP 192.168.1.123.2049 > 192.168.1.252.1341245608: reply ok 120 lookup ERROR: No such file or directory 13:59:51.751303 IP 192.168.1.252.797 > 192.168.1.123.nfs: . ack 3381 win 2625 <nop,nop,timestamp 474136 3431312924>

which is the only NFS packet(s) I see which mention "test".

and stracing touch:

open("test", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK|O_LARGEFILE, 0666) = -1 EOPNOTSUPP (Operation not supported)
utimensat(AT_FDCWD, "test", NULL, 0) = -1 ENOENT (No such file or directory)
write(2, "touch: ", 7touch: ) = 7
write(2, "cannot touch test'", 19cannot touch test') = 19
write(2, ": Operation not supported", 25: Operation not supported) = 25
write(2, "\n", 1
) = 1

I think it's down to this:

commit 013cdf1088d7235da9477a2375654921d9b9ba9f

Author: Christoph Hellwig <[email protected]>
Date:   Fri Dec 20 05:16:53 2013 -0800

nfs: use generic posix ACL infrastructure for v3 Posix ACLs

This causes a small behaviour change in that we don't bother to set ACLs on file creation if the mode bit can express the access permissions fully, and thus behaving identical to local filesystems.

which adds:

Review

Quoted 3 lines
(quoted diff) +3 -0
+       status = posix_acl_create(dir, &sattr->ia_mode, &default_acl, &acl);
+       if (status)
+               goto out;

Right, this should clearly not cause nfs4_proc_create to fail if it returns EOPNOTSUPP.

Quoted 5 lines

into nfs3_proc_create(), but this ends up calling down into nfs3_get_acl(), which does this:

if (!nfs_server_capable(inode, NFS_CAP_ACLS))
return ERR_PTR(-EOPNOTSUPP);

Just for completeness sake: is the server you were running against supposed to support POSIX acls?

--
Trond Myklebust
Linux NFS client maintainer

#22 lore raw
Quoted 72 lines · Trond Myklebust
Quoted 69 lines · Russell King - ARM Linux

I just booted Linus' tip (plus a few other patches to imx-drm and imx code), and stumbled into this interesting scenario:

# touch test
touch: cannot touch `test': Operation not supported

I also tried mkdir and mknod, all result in the same error. Hard and symlinks links are creatable.

However, I can chmod existing files and rename them. Files can also be deleted, and the combination of this has left me without a /etc/mtab !

The machine is a iMX6 based ARM, running root-nfs, which was mounted via ubuntu's initramfs (so not using the kernel's built-in root-nfs.)

/proc/mounts for the root mount gives:
192.168.1.123:/var/boot/ci / nfs rw,relatime,vers=3,rsize=65536,wsize=65536,namlen=255,hard,nolock,proto=tcp,port=2049,timeo=7,retrans=10,sec=sys,local_lock=all,addr=192.168.1.123 0 0

CONFIG_NFS_FS=y
CONFIG_NFS_V2=y
CONFIG_NFS_V3=y
CONFIG_NFS_V3_ACL=y
CONFIG_NFS_V4=y
# CONFIG_NFS_SWAP is not set
# CONFIG_NFS_V4_1 is not set
CONFIG_ROOT_NFS=y
# CONFIG_NFS_USE_LEGACY_DNS is not set
CONFIG_NFS_USE_KERNEL_DNS=y
# CONFIG_NFSD is not set
CONFIG_LOCKD=y
CONFIG_LOCKD_V4=y
CONFIG_NFS_ACL_SUPPORT=y
CONFIG_NFS_COMMON=y
CONFIG_SUNRPC=y
CONFIG_SUNRPC_GSS=y

tcpdumping, I see:

13:59:51.713523 IP 192.168.1.252.1341245608 > 192.168.1.123.2049: 132 lookup fh Unknown/010007011040840000000000CC238FC8FBA0475D9D9F8356B4C44166CDC38700 "test" 13:59:51.714345 IP 192.168.1.123.2049 > 192.168.1.252.1341245608: reply ok 120 lookup ERROR: No such file or directory 13:59:51.751303 IP 192.168.1.252.797 > 192.168.1.123.nfs: . ack 3381 win 2625 <nop,nop,timestamp 474136 3431312924>

which is the only NFS packet(s) I see which mention "test".

and stracing touch:

open("test", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK|O_LARGEFILE, 0666) = -1 EOPNOTSUPP (Operation not supported)
utimensat(AT_FDCWD, "test", NULL, 0) = -1 ENOENT (No such file or directory)
write(2, "touch: ", 7touch: ) = 7
write(2, "cannot touch test'", 19cannot touch test') = 19
write(2, ": Operation not supported", 25: Operation not supported) = 25
write(2, "\n", 1
) = 1

I think it's down to this:

commit 013cdf1088d7235da9477a2375654921d9b9ba9f

Author: Christoph Hellwig <[email protected]>
Date:   Fri Dec 20 05:16:53 2013 -0800

nfs: use generic posix ACL infrastructure for v3 Posix ACLs

This causes a small behaviour change in that we don't bother to set ACLs on file creation if the mode bit can express the access permissions fully, and thus behaving identical to local filesystems.

Quoted 8 lines

Review

Quoted 5 lines

which adds:

(quoted diff) +3 -0
+       status = posix_acl_create(dir, &sattr->ia_mode, &default_acl, &acl);
+       if (status)
+               goto out;

Right, this should clearly not cause nfs4_proc_create to fail if it returns EOPNOTSUPP.

NFS3 :)

Quoted 7 lines
Quoted 5 lines

into nfs3_proc_create(), but this ends up calling down into nfs3_get_acl(), which does this:

if (!nfs_server_capable(inode, NFS_CAP_ACLS))
return ERR_PTR(-EOPNOTSUPP);

Just for completeness sake: is the server you were running against supposed to support POSIX acls?

The server is an old 3.1.8 kernel with this NFS config:

CONFIG_NFS_FS=m
CONFIG_NFS_V3=y
# CONFIG_NFS_V3_ACL is not set
# CONFIG_NFS_V4 is not set
# CONFIG_NFS_FSCACHE is not set
CONFIG_NFSD=m
CONFIG_NFSD_V3=y
# CONFIG_NFSD_V3_ACL is not set
# CONFIG_NFSD_V4 is not set
CONFIG_LOCKD=m
CONFIG_LOCKD_V4=y
CONFIG_NFS_COMMON=y

which has worked fine with NFS clients for the last 1800 odd days... until now.

Signature · 3 lines
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
#23 lore raw
Quoted 75 lines · Russell King - ARM Linux
Quoted 74 lines · Trond Myklebust
Quoted 71 lines · Russell King - ARM Linux

I just booted Linus' tip (plus a few other patches to imx-drm and imx code), and stumbled into this interesting scenario:

# touch test
touch: cannot touch `test': Operation not supported

I also tried mkdir and mknod, all result in the same error. Hard and symlinks links are creatable.

However, I can chmod existing files and rename them. Files can also be deleted, and the combination of this has left me without a /etc/mtab !

The machine is a iMX6 based ARM, running root-nfs, which was mounted via ubuntu's initramfs (so not using the kernel's built-in root-nfs.)

/proc/mounts for the root mount gives:
192.168.1.123:/var/boot/ci / nfs rw,relatime,vers=3,rsize=65536,wsize=65536,namlen=255,hard,nolock,proto=tcp,port=2049,timeo=7,retrans=10,sec=sys,local_lock=all,addr=192.168.1.123 0 0

CONFIG_NFS_FS=y
CONFIG_NFS_V2=y
CONFIG_NFS_V3=y
CONFIG_NFS_V3_ACL=y
CONFIG_NFS_V4=y
# CONFIG_NFS_SWAP is not set
# CONFIG_NFS_V4_1 is not set
CONFIG_ROOT_NFS=y
# CONFIG_NFS_USE_LEGACY_DNS is not set
CONFIG_NFS_USE_KERNEL_DNS=y
# CONFIG_NFSD is not set
CONFIG_LOCKD=y
CONFIG_LOCKD_V4=y
CONFIG_NFS_ACL_SUPPORT=y
CONFIG_NFS_COMMON=y
CONFIG_SUNRPC=y
CONFIG_SUNRPC_GSS=y

tcpdumping, I see:

13:59:51.713523 IP 192.168.1.252.1341245608 > 192.168.1.123.2049: 132 lookup fh Unknown/010007011040840000000000CC238FC8FBA0475D9D9F8356B4C44166CDC38700 "test" 13:59:51.714345 IP 192.168.1.123.2049 > 192.168.1.252.1341245608: reply ok 120 lookup ERROR: No such file or directory 13:59:51.751303 IP 192.168.1.252.797 > 192.168.1.123.nfs: . ack 3381 win 2625 <nop,nop,timestamp 474136 3431312924>

which is the only NFS packet(s) I see which mention "test".

and stracing touch:

open("test", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK|O_LARGEFILE, 0666) = -1 EOPNOTSUPP (Operation not supported)
utimensat(AT_FDCWD, "test", NULL, 0) = -1 ENOENT (No such file or directory)
write(2, "touch: ", 7touch: ) = 7
write(2, "cannot touch test'", 19cannot touch test') = 19
write(2, ": Operation not supported", 25: Operation not supported) = 25
write(2, "\n", 1
) = 1

I think it's down to this:

commit 013cdf1088d7235da9477a2375654921d9b9ba9f

Author: Christoph Hellwig <[email protected]>
Date:   Fri Dec 20 05:16:53 2013 -0800

nfs: use generic posix ACL infrastructure for v3 Posix ACLs

This causes a small behaviour change in that we don't bother to set ACLs on file creation if the mode bit can express the access permissions fully, and thus behaving identical to local filesystems.

which adds:

Quoted 8 lines
Quoted 6 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+       status = posix_acl_create(dir, &sattr->ia_mode, &default_acl, &acl);
+       if (status)
+               goto out;

Right, this should clearly not cause nfs4_proc_create to fail if it returns EOPNOTSUPP.

NFS3 :)

Sorry. I fat fingered that one. I intended to write nfs3_...

Quoted 23 lines
Quoted 7 lines
Quoted 5 lines

into nfs3_proc_create(), but this ends up calling down into nfs3_get_acl(), which does this:

if (!nfs_server_capable(inode, NFS_CAP_ACLS))
return ERR_PTR(-EOPNOTSUPP);

Just for completeness sake: is the server you were running against supposed to support POSIX acls?

The server is an old 3.1.8 kernel with this NFS config:

CONFIG_NFS_FS=m
CONFIG_NFS_V3=y
# CONFIG_NFS_V3_ACL is not set
# CONFIG_NFS_V4 is not set
# CONFIG_NFS_FSCACHE is not set
CONFIG_NFSD=m
CONFIG_NFSD_V3=y
# CONFIG_NFSD_V3_ACL is not set
# CONFIG_NFSD_V4 is not set
CONFIG_LOCKD=m
CONFIG_LOCKD_V4=y
CONFIG_NFS_COMMON=y

Quoted 3 lines

which has worked fine with NFS clients for the last 1800 odd days... until now.

OK. I?m guessing that you?re hitting the auto-probing code further down in nfs3_get_acl(), which also returns EOPNOTSUPP in those cases. Those probably need to return NULL too, then?

However, there seems to be an inconsistency in the whole API here: posix_acl_create() and posix_acl_chmod() seem to want to return ?0? both when acls are not supported and when they are not set, however posix_acl_xattr_get() wants to return EOPNOTSUPP in the first case, and ENODATA in the second. How is the filesystem supposed to know what to return?

--
Trond Myklebust
Linux NFS client maintainer

#24 lore raw

Root NFS panicing on Linus' tip (Re: NFS client broken in Linus' tip)

In reply to Russell King - ARM Linux

Hi Russell, Trond:

Quoted 3 lines · Russell King - ARM Linux

I just booted Linus' tip (plus a few other patches to imx-drm and imx code), and stumbled into this interesting scenario:

[..]

Quoted 4 lines

CONFIG_NFS_FS=y
CONFIG_NFS_V2=y
CONFIG_NFS_V3=y
CONFIG_NFS_V3_ACL=y

Just came across another issue, but a bit more problematic, as my kernel (Linus' tip as well) panics, after mounting the rootfs:

IP-Config: Complete:

     device=eth0, hwaddr=00:50:43:50:1c:15, ipaddr=192.168.0.159, mask=255.255.255.0, gw=192.168.0.1
     host=develboard, domain=, nis-domain=(none)
     bootserver=192.168.0.45, rootserver=192.168.0.45, rootpath=
VFS: Mounted root (nfs filesystem) on device 0:11.
devtmpfs: mounted
Freeing unused kernel memory: 136K (c0465000 - c0487000)
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = c0004000
[00000000] *pgd=00000000
Internal error: Oops: 5 [#1] ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Tainted: G        W    3.13.0-10094-g9b0cd30 #276
task: ed839a40 ti: ed83a000 task.ti: ed83a000
PC is at xattr_resolve_name+0x14/0x94
LR is at generic_getxattr+0x2c/0x64
pc : [<c00a7ab0>]    lr : [<c00a7b5c>]    psr: a0000113

sp : ed83be5c ip : ed83be74 fp : ed10ebc0
r10: ed83a000 r9 : ed43d980 r8 : ed81b800
r7 : c034dad8 r6 : 00000000 r5 : c03f3dcc r4 : ed43d980 r3 : 00000014 r2 : ed83be8c r1 : ed83be74 r0 : 00000000
Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 00004059 DAC: 00000015 Process swapper (pid: 1, stack limit = 0xed83a238)

Stack: (0xed83be5c to 0xed83c000)

be40: ed43d980
be60: 00000014 ed83be8c 00000000 00000000 c04bc22c c03f3dcc ed83bf14 ed43f340
be80: ed43d980 c01115cc 00000000 00000041 c04bba6c 00000000 00000000 002040d0
bea0: ed81bc00 ed10ebc0 ed81bc30 c01116f8 00000000 000004d0 ed8172d0 ed43d980
bec0: 45878fd4 00000007 bfe01007 ef7f8fc0 c04bba6c ed43d6d8 c04bba6c 00000101
bee0: 00000000 ed809fd0 ed809fc0 ed809f50 ed809f40 00000000 edb045d8 c0078bcc
bf00: ed0e5dc0 edb045d8 00000000 bf000000 ed0e5dc0 00000000 00000000 00000000
bf20: 00000000 00000000 bf000000 ed10ebc0 ed0e5dc0 00000001 edb045d8 c04926d0
bf40: ed83a000 c0492758 ed10ebc0 c008fc54 00000001 ed0e5dc0 00000002 c0090cec
bf60: c03ec85c ed0e5df4 00000000 ed839c00 c0487000 c04bcec0 c03e4f08 00000000
bf80: 00000000 00000000 00000000 00000000 00000000 c00086a8 00000000 c04bcec0
bfa0: c0344f5c c0345004 00000000 c000e398 00000000 00000000 00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000 [<c00a7ab0>] (xattr_resolve_name) from [<00000000>] ( (null))

Code: e1a06000 e5915000 e3550000 0a00001d (e5900000) 
---[ end trace 15c15b4afa9eff90 ]---
swapper (1) used greatest stack depth: 5104 bytes left
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

Adding a little hack, and could produce a better strack trace. See the diff and the stack trace below:

fs/xattr.c +4 -0
index 3377dff..bd2b173 100644
@@ -740,6 +740,10 @@ xattr_resolve_name(const struct xattr_handler **handlers, const char **name)
 
 	if (!*name)
 		return NULL;
+	if(!handlers) {
+		dump_stack();
+		panic("ouch");
+	}
 
 	for_each_xattr_handler(handlers, handler) {
 		const char *n = strcmp_prefix(*name, handler->prefix);
CPU: 0 PID: 1 Comm: swapper Tainted: G        W    3.13.0-10094-g9b0cd30-dirty #279
[<c0012f40>] (unwind_backtrace) from [<c00107b8>] (show_stack+0x10/0x14)
[<c00107b8>] (show_stack) from [<c00a8160>] (xattr_resolve_name+0x9c/0xa8)
[<c00a8160>] (xattr_resolve_name) from [<c00a8274>] (generic_getxattr+0x2c/0x64)
[<c00a8274>] (generic_getxattr) from [<c01115e0>] (get_vfs_caps_from_disk+0x4c/0xf4)
[<c01115e0>] (get_vfs_caps_from_disk) from [<c011170c>] (cap_bprm_set_creds+0x84/0x408)
[<c011170c>] (cap_bprm_set_creds) from [<c008fc54>] (prepare_binprm+0x80/0x11c)
[<c008fc54>] (prepare_binprm) from [<c0090cec>] (do_execve+0x33c/0x46c)
[<c0090cec>] (do_execve) from [<c00086a8>] (try_to_run_init_process+0x1c/0x50)
[<c00086a8>] (try_to_run_init_process) from [<c0345024>] (kernel_init+0xa8/0x110)
[<c0345024>] (kernel_init) from [<c000e398>] (ret_from_fork+0x14/0x3c)
Kernel panic - not syncing: ouch

FWIW, here's my piece of NFS config:

CONFIG_NFS_FS=y
CONFIG_NFS_V2=y
CONFIG_NFS_V3=y
# CONFIG_NFS_V3_ACL is not set
# CONFIG_NFS_V4 is not set
# CONFIG_NFS_SWAP is not set
CONFIG_ROOT_NFS=y
# CONFIG_NFSD is not set
CONFIG_LOCKD=y
CONFIG_LOCKD_V4=y
CONFIG_NFS_COMMON=y
CONFIG_SUNRPC=y

Quoted 6 lines

I think it's down to this:

commit 013cdf1088d7235da9477a2375654921d9b9ba9f

Author: Christoph Hellwig <[email protected]>
Date:   Fri Dec 20 05:16:53 2013 -0800
Quoted 8 lines

nfs: use generic posix ACL infrastructure for v3 Posix ACLs

This causes a small behaviour change in that we don't bother to set ACLs on file creation if the mode bit can express the access permissions fully, and thus behaving identical to local filesystems.

And also here, reverting the above seem to fix the panic.

Ideas?

Signature · 3 lines
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
#25 lore raw

Re: Root NFS panicing on Linus' tip (Re: NFS client broken in Linus' tip)

Quoted 116 lines · Ezequiel Garcia

Hi Russell, Trond:

Quoted 3 lines · Russell King - ARM Linux

I just booted Linus' tip (plus a few other patches to imx-drm and imx code), and stumbled into this interesting scenario:

[..]

Quoted 4 lines

CONFIG_NFS_FS=y
CONFIG_NFS_V2=y
CONFIG_NFS_V3=y
CONFIG_NFS_V3_ACL=y

Just came across another issue, but a bit more problematic, as my kernel (Linus' tip as well) panics, after mounting the rootfs:

IP-Config: Complete:

     device=eth0, hwaddr=00:50:43:50:1c:15, ipaddr=192.168.0.159, mask=255.255.255.0, gw=192.168.0.1
     host=develboard, domain=, nis-domain=(none)
     bootserver=192.168.0.45, rootserver=192.168.0.45, rootpath=
VFS: Mounted root (nfs filesystem) on device 0:11.
devtmpfs: mounted
Freeing unused kernel memory: 136K (c0465000 - c0487000)
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = c0004000
[00000000] *pgd=00000000
Internal error: Oops: 5 [#1] ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Tainted: G        W    3.13.0-10094-g9b0cd30 #276
task: ed839a40 ti: ed83a000 task.ti: ed83a000
PC is at xattr_resolve_name+0x14/0x94
LR is at generic_getxattr+0x2c/0x64
pc : [<c00a7ab0>]    lr : [<c00a7b5c>]    psr: a0000113

sp : ed83be5c ip : ed83be74 fp : ed10ebc0
r10: ed83a000 r9 : ed43d980 r8 : ed81b800
r7 : c034dad8 r6 : 00000000 r5 : c03f3dcc r4 : ed43d980 r3 : 00000014 r2 : ed83be8c r1 : ed83be74 r0 : 00000000
Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 00004059 DAC: 00000015 Process swapper (pid: 1, stack limit = 0xed83a238)

Stack: (0xed83be5c to 0xed83c000)

be40: ed43d980
be60: 00000014 ed83be8c 00000000 00000000 c04bc22c c03f3dcc ed83bf14 ed43f340
be80: ed43d980 c01115cc 00000000 00000041 c04bba6c 00000000 00000000 002040d0
bea0: ed81bc00 ed10ebc0 ed81bc30 c01116f8 00000000 000004d0 ed8172d0 ed43d980
bec0: 45878fd4 00000007 bfe01007 ef7f8fc0 c04bba6c ed43d6d8 c04bba6c 00000101
bee0: 00000000 ed809fd0 ed809fc0 ed809f50 ed809f40 00000000 edb045d8 c0078bcc
bf00: ed0e5dc0 edb045d8 00000000 bf000000 ed0e5dc0 00000000 00000000 00000000
bf20: 00000000 00000000 bf000000 ed10ebc0 ed0e5dc0 00000001 edb045d8 c04926d0
bf40: ed83a000 c0492758 ed10ebc0 c008fc54 00000001 ed0e5dc0 00000002 c0090cec
bf60: c03ec85c ed0e5df4 00000000 ed839c00 c0487000 c04bcec0 c03e4f08 00000000
bf80: 00000000 00000000 00000000 00000000 00000000 c00086a8 00000000 c04bcec0
bfa0: c0344f5c c0345004 00000000 c000e398 00000000 00000000 00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000 [<c00a7ab0>] (xattr_resolve_name) from [<00000000>] ( (null))

Code: e1a06000 e5915000 e3550000 0a00001d (e5900000) 
---[ end trace 15c15b4afa9eff90 ]---
swapper (1) used greatest stack depth: 5104 bytes left
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

Adding a little hack, and could produce a better strack trace. See the diff and the stack trace below:

fs/xattr.c +4 -0
index 3377dff..bd2b173 100644
@@ -740,6 +740,10 @@ xattr_resolve_name(const struct xattr_handler **handlers, const char **name)
 
 	if (!*name)
 		return NULL;
+	if(!handlers) {
+		dump_stack();
+		panic("ouch");
+	}
 
 	for_each_xattr_handler(handlers, handler) {
 		const char *n = strcmp_prefix(*name, handler->prefix);
CPU: 0 PID: 1 Comm: swapper Tainted: G        W    3.13.0-10094-g9b0cd30-dirty #279
[<c0012f40>] (unwind_backtrace) from [<c00107b8>] (show_stack+0x10/0x14)
[<c00107b8>] (show_stack) from [<c00a8160>] (xattr_resolve_name+0x9c/0xa8)
[<c00a8160>] (xattr_resolve_name) from [<c00a8274>] (generic_getxattr+0x2c/0x64)
[<c00a8274>] (generic_getxattr) from [<c01115e0>] (get_vfs_caps_from_disk+0x4c/0xf4)
[<c01115e0>] (get_vfs_caps_from_disk) from [<c011170c>] (cap_bprm_set_creds+0x84/0x408)
[<c011170c>] (cap_bprm_set_creds) from [<c008fc54>] (prepare_binprm+0x80/0x11c)
[<c008fc54>] (prepare_binprm) from [<c0090cec>] (do_execve+0x33c/0x46c)
[<c0090cec>] (do_execve) from [<c00086a8>] (try_to_run_init_process+0x1c/0x50)
[<c00086a8>] (try_to_run_init_process) from [<c0345024>] (kernel_init+0xa8/0x110)
[<c0345024>] (kernel_init) from [<c000e398>] (ret_from_fork+0x14/0x3c)
Kernel panic - not syncing: ouch

FWIW, here's my piece of NFS config:

CONFIG_NFS_FS=y
CONFIG_NFS_V2=y
CONFIG_NFS_V3=y
# CONFIG_NFS_V3_ACL is not set
# CONFIG_NFS_V4 is not set
# CONFIG_NFS_SWAP is not set
CONFIG_ROOT_NFS=y
# CONFIG_NFSD is not set
CONFIG_LOCKD=y
CONFIG_LOCKD_V4=y
CONFIG_NFS_COMMON=y
CONFIG_SUNRPC=y

Quoted 8 lines

I think it's down to this:

commit 013cdf1088d7235da9477a2375654921d9b9ba9f

Author: Christoph Hellwig <[email protected]>
Date:   Fri Dec 20 05:16:53 2013 -0800

nfs: use generic posix ACL infrastructure for v3 Posix ACLs

Quoted 8 lines
Quoted 6 lines

This causes a small behaviour change in that we don't bother to set ACLs on file creation if the mode bit can express the access permissions fully, and thus behaving identical to local filesystems.

And also here, reverting the above seem to fix the panic.

Reverting this commit with NFS3 ACLs enabled also fixes the problems I reported.

Signature · 3 lines
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".