* [PATCH] NFSD: Clear cached acl after setting a zero-length default posix acl:
@ 2014-04-17 12:46 Kinglong Mee
2014-04-17 14:36 ` Christoph Hellwig
0 siblings, 1 reply; 11+ messages in thread
From: Kinglong Mee @ 2014-04-17 12:46 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs, Christoph Hellwig
After setting ACL for directory, I got two problems that caused
by the cached zero-length default posix acl.
This patch just clears the cached zero-length default posix acl
after setting.
First problem:
# nfs4_setfacl -s A::OWNER@:RWX /mnt/123/; touch /mnt/123/test
............ hang ...........
Second problem:
# nfs4_setfacl -s A::OWNER@:RWX /mnt/123/; nfs4_getfacl /mnt/123/
[ 1610.167668] ------------[ cut here ]------------
[ 1610.168320] kernel BUG at /root/nfs/linux/fs/nfsd/nfs4acl.c:239!
[ 1610.168320] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
[ 1610.168320] Modules linked in: nfsv4(OE) nfs(OE) nfsd(OE)
rpcsec_gss_krb5 fscache ip6t_rpfilter ip6t_REJECT cfg80211 xt_conntrack
rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables
ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6
ip6table_mangle ip6table_security ip6table_raw ip6table_filter
ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4
nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw
auth_rpcgss nfs_acl snd_intel8x0 ppdev lockd snd_ac97_codec ac97_bus
snd_pcm snd_timer e1000 pcspkr parport_pc snd parport serio_raw joydev
i2c_piix4 sunrpc(OE) microcode soundcore i2c_core ata_generic pata_acpi
[last unloaded: nfsd]
[ 1610.168320] CPU: 0 PID: 27397 Comm: nfsd Tainted: G OE
3.15.0-rc1+ #15
[ 1610.168320] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS
VirtualBox 12/01/2006
[ 1610.168320] task: ffff88005ab653d0 ti: ffff88005a944000 task.ti:
ffff88005a944000
[ 1610.168320] RIP: 0010:[<ffffffffa034d5ed>] [<ffffffffa034d5ed>]
_posix_to_nfsv4_one+0x3cd/0x3d0 [nfsd]
[ 1610.168320] RSP: 0018:ffff88005a945b00 EFLAGS: 00010293
[ 1610.168320] RAX: 0000000000000001 RBX: ffff88006700bac0 RCX:
0000000000000000
[ 1610.168320] RDX: 0000000000000000 RSI: ffff880067c83f00 RDI:
ffff880068233300
[ 1610.168320] RBP: ffff88005a945b48 R08: ffffffff81c64830 R09:
0000000000000000
[ 1610.168320] R10: ffff88004ea85be0 R11: 000000000000f475 R12:
ffff880068233300
[ 1610.168320] R13: 0000000000000003 R14: 0000000000000002 R15:
ffff880068233300
[ 1610.168320] FS: 0000000000000000(0000) GS:ffff880077800000(0000)
knlGS:0000000000000000
[ 1610.168320] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 1610.168320] CR2: 00007f5bcbd3b0b9 CR3: 0000000001c0f000 CR4:
00000000000006f0
[ 1610.168320] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 1610.168320] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[ 1610.168320] Stack:
[ 1610.168320] ffffffff00000000 0000000b67c83500 000000076700bac0
0000000000000000
[ 1610.168320] ffff88006700bac0 ffff880068233300 ffff88005a945c08
0000000000000002
[ 1610.168320] 0000000000000000 ffff88005a945b88 ffffffffa034e2d5
000000065a945b68
[ 1610.168320] Call Trace:
[ 1610.168320] [<ffffffffa034e2d5>] nfsd4_get_nfs4_acl+0x95/0x150 [nfsd]
[ 1610.168320] [<ffffffffa03400d6>] nfsd4_encode_fattr+0x646/0x1e70 [nfsd]
[ 1610.168320] [<ffffffff816a6e6e>] ? kmemleak_alloc+0x4e/0xb0
[ 1610.168320] [<ffffffffa0327962>] ?
nfsd_setuser_and_check_port+0x52/0x80 [nfsd]
[ 1610.168320] [<ffffffff812cd4bb>] ? selinux_cred_prepare+0x1b/0x30
[ 1610.168320] [<ffffffffa0341caa>] nfsd4_encode_getattr+0x5a/0x60 [nfsd]
[ 1610.168320] [<ffffffffa0341e07>] nfsd4_encode_operation+0x67/0x110
[nfsd]
[ 1610.168320] [<ffffffffa033844d>] nfsd4_proc_compound+0x21d/0x810 [nfsd]
[ 1610.168320] [<ffffffffa0324d9b>] nfsd_dispatch+0xbb/0x200 [nfsd]
[ 1610.168320] [<ffffffffa00850cd>] svc_process_common+0x46d/0x6d0 [sunrpc]
[ 1610.168320] [<ffffffffa0085433>] svc_process+0x103/0x170 [sunrpc]
[ 1610.168320] [<ffffffffa032472f>] nfsd+0xbf/0x130 [nfsd]
[ 1610.168320] [<ffffffffa0324670>] ? nfsd_destroy+0x80/0x80 [nfsd]
[ 1610.168320] [<ffffffff810a5202>] kthread+0xd2/0xf0
[ 1610.168320] [<ffffffff810a5130>] ? insert_kthread_work+0x40/0x40
[ 1610.168320] [<ffffffff816c1ebc>] ret_from_fork+0x7c/0xb0
[ 1610.168320] [<ffffffff810a5130>] ? insert_kthread_work+0x40/0x40
[ 1610.168320] Code: 78 02 e9 e7 fc ff ff 31 c0 31 d2 31 c9 66 89 45 ce
41 8b 04 24 66 89 55 d0 66 89 4d d2 48 8d 04 80 49 8d 5c 84 04 e9 37 fd
ff ff <0f> 0b 90 0f 1f 44 00 00 55 8b 56 08 c7 07 00 00 00 00 8b 46 0c
[ 1610.168320] RIP [<ffffffffa034d5ed>] _posix_to_nfsv4_one+0x3cd/0x3d0
[nfsd]
[ 1610.168320] RSP <ffff88005a945b00>
[ 1610.257313] ---[ end trace 838254e3e352285b ]---
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
fs/nfsd/nfs4acl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
index de5d66b..4eb0a9f 100644
--- a/fs/nfsd/nfs4acl.c
+++ b/fs/nfsd/nfs4acl.c
@@ -834,6 +834,8 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct
svc_fh *fhp,
if (S_ISDIR(inode->i_mode)) {
host_error = inode->i_op->set_acl(inode, dpacl,
ACL_TYPE_DEFAULT);
+ if (!dpacl->a_count)
+ forget_cached_acl(inode, ACL_TYPE_DEFAULT);
}
out_release:
--
1.9.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] NFSD: Clear cached acl after setting a zero-length default posix acl:
2014-04-17 12:46 [PATCH] NFSD: Clear cached acl after setting a zero-length default posix acl: Kinglong Mee
@ 2014-04-17 14:36 ` Christoph Hellwig
2014-04-18 3:48 ` [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries Kinglong Mee
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Christoph Hellwig @ 2014-04-17 14:36 UTC (permalink / raw)
To: Kinglong Mee; +Cc: J. Bruce Fields, linux-nfs, Christoph Hellwig
On Thu, Apr 17, 2014 at 08:46:11PM +0800, Kinglong Mee wrote:
> After setting ACL for directory, I got two problems that caused
> by the cached zero-length default posix acl.
>
> This patch just clears the cached zero-length default posix acl
> after setting.
>
> First problem:
> # nfs4_setfacl -s A::OWNER@:RWX /mnt/123/; touch /mnt/123/test
> ............ hang ...........
Nfsd must not call forget_cached_acl, that's the filesystems job.
I think the right fix is to make sure nfsd4_set_nfs4_acl calls ->set_acl
with a NULL ACL structure if there are no entries.
Btw, it would be really good if we kept tests like this as a regression
test suite. Is there one for NFS already? If not we could add
nfs-specific tests to xfstests as well.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries
2014-04-17 14:36 ` Christoph Hellwig
@ 2014-04-18 3:48 ` Kinglong Mee
2014-04-18 12:19 ` J. Bruce Fields
2014-04-18 4:03 ` [PATCH] NFSD: Clear cached acl after setting a zero-length default posix acl: Kinglong Mee
2014-04-18 12:13 ` J. Bruce Fields
2 siblings, 1 reply; 11+ messages in thread
From: Kinglong Mee @ 2014-04-18 3:48 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: J. Bruce Fields, linux-nfs
After setting ACL for directory, I got two problems that caused
by the cached zero-length default posix acl.
This patch make sure nfsd don't set a zero-length default
posix ACL if there are no entries for the default ACL.
Thanks for Christoph Hellwig's advice.
v2:
drop calling forget_cached_acl(), just not set zero-length ACL
First problem:
# nfs4_setfacl -s A::OWNER@:RWX /mnt/123/; touch /mnt/123/test
............ hang ...........
Second problem:
# nfs4_setfacl -s A::OWNER@:RWX /mnt/123/; nfs4_getfacl /mnt/123/
[ 1610.167668] ------------[ cut here ]------------
[ 1610.168320] kernel BUG at /root/nfs/linux/fs/nfsd/nfs4acl.c:239!
[ 1610.168320] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
[ 1610.168320] Modules linked in: nfsv4(OE) nfs(OE) nfsd(OE)
rpcsec_gss_krb5 fscache ip6t_rpfilter ip6t_REJECT cfg80211 xt_conntrack
rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables
ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6
ip6table_mangle ip6table_security ip6table_raw ip6table_filter
ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4
nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw
auth_rpcgss nfs_acl snd_intel8x0 ppdev lockd snd_ac97_codec ac97_bus
snd_pcm snd_timer e1000 pcspkr parport_pc snd parport serio_raw joydev
i2c_piix4 sunrpc(OE) microcode soundcore i2c_core ata_generic pata_acpi
[last unloaded: nfsd]
[ 1610.168320] CPU: 0 PID: 27397 Comm: nfsd Tainted: G OE
3.15.0-rc1+ #15
[ 1610.168320] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS
VirtualBox 12/01/2006
[ 1610.168320] task: ffff88005ab653d0 ti: ffff88005a944000 task.ti:
ffff88005a944000
[ 1610.168320] RIP: 0010:[<ffffffffa034d5ed>] [<ffffffffa034d5ed>]
_posix_to_nfsv4_one+0x3cd/0x3d0 [nfsd]
[ 1610.168320] RSP: 0018:ffff88005a945b00 EFLAGS: 00010293
[ 1610.168320] RAX: 0000000000000001 RBX: ffff88006700bac0 RCX:
0000000000000000
[ 1610.168320] RDX: 0000000000000000 RSI: ffff880067c83f00 RDI:
ffff880068233300
[ 1610.168320] RBP: ffff88005a945b48 R08: ffffffff81c64830 R09:
0000000000000000
[ 1610.168320] R10: ffff88004ea85be0 R11: 000000000000f475 R12:
ffff880068233300
[ 1610.168320] R13: 0000000000000003 R14: 0000000000000002 R15:
ffff880068233300
[ 1610.168320] FS: 0000000000000000(0000) GS:ffff880077800000(0000)
knlGS:0000000000000000
[ 1610.168320] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 1610.168320] CR2: 00007f5bcbd3b0b9 CR3: 0000000001c0f000 CR4:
00000000000006f0
[ 1610.168320] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 1610.168320] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[ 1610.168320] Stack:
[ 1610.168320] ffffffff00000000 0000000b67c83500 000000076700bac0
0000000000000000
[ 1610.168320] ffff88006700bac0 ffff880068233300 ffff88005a945c08
0000000000000002
[ 1610.168320] 0000000000000000 ffff88005a945b88 ffffffffa034e2d5
000000065a945b68
[ 1610.168320] Call Trace:
[ 1610.168320] [<ffffffffa034e2d5>] nfsd4_get_nfs4_acl+0x95/0x150 [nfsd]
[ 1610.168320] [<ffffffffa03400d6>] nfsd4_encode_fattr+0x646/0x1e70 [nfsd]
[ 1610.168320] [<ffffffff816a6e6e>] ? kmemleak_alloc+0x4e/0xb0
[ 1610.168320] [<ffffffffa0327962>] ?
nfsd_setuser_and_check_port+0x52/0x80 [nfsd]
[ 1610.168320] [<ffffffff812cd4bb>] ? selinux_cred_prepare+0x1b/0x30
[ 1610.168320] [<ffffffffa0341caa>] nfsd4_encode_getattr+0x5a/0x60 [nfsd]
[ 1610.168320] [<ffffffffa0341e07>] nfsd4_encode_operation+0x67/0x110
[nfsd]
[ 1610.168320] [<ffffffffa033844d>] nfsd4_proc_compound+0x21d/0x810 [nfsd]
[ 1610.168320] [<ffffffffa0324d9b>] nfsd_dispatch+0xbb/0x200 [nfsd]
[ 1610.168320] [<ffffffffa00850cd>] svc_process_common+0x46d/0x6d0 [sunrpc]
[ 1610.168320] [<ffffffffa0085433>] svc_process+0x103/0x170 [sunrpc]
[ 1610.168320] [<ffffffffa032472f>] nfsd+0xbf/0x130 [nfsd]
[ 1610.168320] [<ffffffffa0324670>] ? nfsd_destroy+0x80/0x80 [nfsd]
[ 1610.168320] [<ffffffff810a5202>] kthread+0xd2/0xf0
[ 1610.168320] [<ffffffff810a5130>] ? insert_kthread_work+0x40/0x40
[ 1610.168320] [<ffffffff816c1ebc>] ret_from_fork+0x7c/0xb0
[ 1610.168320] [<ffffffff810a5130>] ? insert_kthread_work+0x40/0x40
[ 1610.168320] Code: 78 02 e9 e7 fc ff ff 31 c0 31 d2 31 c9 66 89 45 ce
41 8b 04 24 66 89 55 d0 66 89 4d d2 48 8d 04 80 49 8d 5c 84 04 e9 37 fd
ff ff <0f> 0b 90 0f 1f 44 00 00 55 8b 56 08 c7 07 00 00 00 00 8b 46 0c
[ 1610.168320] RIP [<ffffffffa034d5ed>] _posix_to_nfsv4_one+0x3cd/0x3d0
[nfsd]
[ 1610.168320] RSP <ffff88005a945b00>
[ 1610.257313] ---[ end trace 838254e3e352285b ]---
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
fs/nfsd/nfs4acl.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
index de5d66b..fdd259e 100644
--- a/fs/nfsd/nfs4acl.c
+++ b/fs/nfsd/nfs4acl.c
@@ -395,8 +395,10 @@ sort_pacl(struct posix_acl *pacl)
* by uid/gid. */
int i, j;
- if (pacl->a_count <= 4)
- return; /* no users or groups */
+ /* no users or groups */
+ if (!pacl || pacl->a_count <= 4)
+ return;
+
i = 1;
while (pacl->a_entries[i].e_tag == ACL_USER)
i++;
@@ -523,13 +525,11 @@ posix_state_to_acl(struct posix_acl_state *state,
unsigned int flags)
/*
* ACLs with no ACEs are treated differently in the inheritable
- * and effective cases: when there are no inheritable ACEs, we
- * set a zero-length default posix acl:
+ * and effective cases.
*/
- if (state->empty && (flags & NFS4_ACL_TYPE_DEFAULT)) {
- pacl = posix_acl_alloc(0, GFP_KERNEL);
- return pacl ? pacl : ERR_PTR(-ENOMEM);
- }
+ if (state->empty && (flags & NFS4_ACL_TYPE_DEFAULT))
+ return NULL;
+
/*
* When there are no effective ACEs, the following will end
* up setting a 3-element effective posix ACL with all
@@ -831,7 +831,7 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct
svc_fh *fhp,
if (host_error < 0)
goto out_release;
- if (S_ISDIR(inode->i_mode)) {
+ if (dpacl && S_ISDIR(inode->i_mode)) {
host_error = inode->i_op->set_acl(inode, dpacl,
ACL_TYPE_DEFAULT);
}
--
1.9.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] NFSD: Clear cached acl after setting a zero-length default posix acl:
2014-04-17 14:36 ` Christoph Hellwig
2014-04-18 3:48 ` [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries Kinglong Mee
@ 2014-04-18 4:03 ` Kinglong Mee
2014-04-18 12:13 ` J. Bruce Fields
2 siblings, 0 replies; 11+ messages in thread
From: Kinglong Mee @ 2014-04-18 4:03 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: J. Bruce Fields, linux-nfs
On 2014/4/17 22:36, Christoph Hellwig wrote:
> On Thu, Apr 17, 2014 at 08:46:11PM +0800, Kinglong Mee wrote:
>> After setting ACL for directory, I got two problems that caused
>> by the cached zero-length default posix acl.
>>
>> This patch just clears the cached zero-length default posix acl
>> after setting.
>>
>> First problem:
>> # nfs4_setfacl -s A::OWNER@:RWX /mnt/123/; touch /mnt/123/test
>> ............ hang ...........
>
> Nfsd must not call forget_cached_acl, that's the filesystems job.
> I think the right fix is to make sure nfsd4_set_nfs4_acl calls ->set_acl
> with a NULL ACL structure if there are no entries.
Thanks for your advice.
>
> Btw, it would be really good if we kept tests like this as a regression
> test suite. Is there one for NFS already? If not we could add
> nfs-specific tests to xfstests as well.
I found one at
http://www.citi.umich.edu/projects/asci/icsi-alpha/acl-test-20060929.tar.gz
, but it does not work on Fedora 20 without the following patch.
thanks,
Kinglong Mee
From 6fb72997379ef84fdbcf4938dc68a6ef7c4f1f00 Mon Sep 17 00:00:00 2001
From: Kinglong Mee <kinglongmee@gmail.com>
Date: Thu, 17 Apr 2014 19:57:15 +0800
Subject: [PATCH] nfs4_set_acl has delete l/s/f flags, using r/w/a
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
runtests | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/runtests b/runtests
index 25d4a9d..d0eeedd 100755
--- a/runtests
+++ b/runtests
@@ -79,30 +79,30 @@ $r $u $prog setacl "$a":rwaxdnNtTcoy $file $file
# Create a test directory, give owner all permissions
mkdir $dir
-nfs4_setfacl -s "$a":xlfsDnNtTcCoy $dir
+nfs4_setfacl -s "$a":xrwaDnNtTcCoy $dir
# Remove permissions one at a time, check that they're really gone
-$r $u $prog creat "$a":xlsDnNtTcCoy $dir $dir/$file
-$r $u $prog listdir "$a":xsfDnNtTcCoy $dir $dir
-$r $u $prog mkdir "$a":xlfDnNtTcCoy $dir $dir/$dir
+$r $u $prog creat "$a":xraDnNtTcCoy $dir $dir/$file
+$r $u $prog listdir "$a":xawDnNtTcCoy $dir $dir
+$r $u $prog mkdir "$a":xrwDnNtTcCoy $dir $dir/$dir
# Create a file for lookup and delete tests
-nfs4_setfacl -s A::OWNER@:xlfsDnNtTcCoy $dir
+nfs4_setfacl -s A::OWNER@:xrwaDnNtTcCoy $dir
touch $dir/$file
if (! -f $dir/$file) then
echo "can't create $testdir/$dir/$file for testing"
endif
-$r $u $prog lookup "$a":lfsDnNtTcCoy $dir $dir/$file
+$r $u $prog lookup "$a":rwaDnNtTcCoy $dir $dir/$file
# Delete test: first have to put back lookup ("x") rights,
# then remove "d", then "D"
-nfs4_setfacl -s A::OWNER@:xlfsDnNtTcCoy $dir
+nfs4_setfacl -s A::OWNER@:xrwaDnNtTcCoy $dir
nfs4_setfacl -s "$a":rwaxnNtTcCoy $dir/$file
if ($status != 0) then
exit 1
endif
-$r $u $prog rm "$a":xlfsnNtTcCoy $dir $dir/$file
+$r $u $prog rm "$a":xrwanNtTcCoy $dir $dir/$file
--
1.9.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] NFSD: Clear cached acl after setting a zero-length default posix acl:
2014-04-17 14:36 ` Christoph Hellwig
2014-04-18 3:48 ` [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries Kinglong Mee
2014-04-18 4:03 ` [PATCH] NFSD: Clear cached acl after setting a zero-length default posix acl: Kinglong Mee
@ 2014-04-18 12:13 ` J. Bruce Fields
2014-04-18 12:26 ` Kinglong Mee
2 siblings, 1 reply; 11+ messages in thread
From: J. Bruce Fields @ 2014-04-18 12:13 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Kinglong Mee, linux-nfs
On Thu, Apr 17, 2014 at 07:36:34AM -0700, Christoph Hellwig wrote:
> On Thu, Apr 17, 2014 at 08:46:11PM +0800, Kinglong Mee wrote:
> > After setting ACL for directory, I got two problems that caused
> > by the cached zero-length default posix acl.
> >
> > This patch just clears the cached zero-length default posix acl
> > after setting.
> >
> > First problem:
> > # nfs4_setfacl -s A::OWNER@:RWX /mnt/123/; touch /mnt/123/test
> > ............ hang ...........
>
> Nfsd must not call forget_cached_acl, that's the filesystems job.
> I think the right fix is to make sure nfsd4_set_nfs4_acl calls ->set_acl
> with a NULL ACL structure if there are no entries.
>
> Btw, it would be really good if we kept tests like this as a regression
> test suite. Is there one for NFS already? If not we could add
> nfs-specific tests to xfstests as well.
I'd recommend pynfs for something like this.
(It talks NFSv4 directly to the server, so won't depend on client
acl-caching behavior, etc.)
--b.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries
2014-04-18 3:48 ` [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries Kinglong Mee
@ 2014-04-18 12:19 ` J. Bruce Fields
2014-04-18 12:49 ` [PATCH v3] NFSD: Call ->set_acl with a NULL ACL structure if no entries Kinglong Mee
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: J. Bruce Fields @ 2014-04-18 12:19 UTC (permalink / raw)
To: Kinglong Mee; +Cc: Christoph Hellwig, linux-nfs
On Fri, Apr 18, 2014 at 11:48:50AM +0800, Kinglong Mee wrote:
> After setting ACL for directory, I got two problems that caused
> by the cached zero-length default posix acl.
>
> This patch make sure nfsd don't set a zero-length default
> posix ACL if there are no entries for the default ACL.
>
> Thanks for Christoph Hellwig's advice.
>
> v2:
> drop calling forget_cached_acl(), just not set zero-length ACL
I thought Christoph's suggestion was to actually call ->set_acl with a
NULL ACL? Does that turn out to be unnecessary?
--b.
>
> First problem:
> # nfs4_setfacl -s A::OWNER@:RWX /mnt/123/; touch /mnt/123/test
> ............ hang ...........
>
> Second problem:
> # nfs4_setfacl -s A::OWNER@:RWX /mnt/123/; nfs4_getfacl /mnt/123/
> [ 1610.167668] ------------[ cut here ]------------
> [ 1610.168320] kernel BUG at /root/nfs/linux/fs/nfsd/nfs4acl.c:239!
> [ 1610.168320] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
> [ 1610.168320] Modules linked in: nfsv4(OE) nfs(OE) nfsd(OE)
> rpcsec_gss_krb5 fscache ip6t_rpfilter ip6t_REJECT cfg80211
> xt_conntrack rfkill ebtable_nat ebtable_broute bridge stp llc
> ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6
> nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security
> ip6table_raw ip6table_filter ip6_tables iptable_nat
> nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
> iptable_mangle iptable_security iptable_raw auth_rpcgss nfs_acl
> snd_intel8x0 ppdev lockd snd_ac97_codec ac97_bus snd_pcm snd_timer
> e1000 pcspkr parport_pc snd parport serio_raw joydev i2c_piix4
> sunrpc(OE) microcode soundcore i2c_core ata_generic pata_acpi [last
> unloaded: nfsd]
> [ 1610.168320] CPU: 0 PID: 27397 Comm: nfsd Tainted: G OE
> 3.15.0-rc1+ #15
> [ 1610.168320] Hardware name: innotek GmbH VirtualBox/VirtualBox,
> BIOS VirtualBox 12/01/2006
> [ 1610.168320] task: ffff88005ab653d0 ti: ffff88005a944000 task.ti:
> ffff88005a944000
> [ 1610.168320] RIP: 0010:[<ffffffffa034d5ed>] [<ffffffffa034d5ed>]
> _posix_to_nfsv4_one+0x3cd/0x3d0 [nfsd]
> [ 1610.168320] RSP: 0018:ffff88005a945b00 EFLAGS: 00010293
> [ 1610.168320] RAX: 0000000000000001 RBX: ffff88006700bac0 RCX:
> 0000000000000000
> [ 1610.168320] RDX: 0000000000000000 RSI: ffff880067c83f00 RDI:
> ffff880068233300
> [ 1610.168320] RBP: ffff88005a945b48 R08: ffffffff81c64830 R09:
> 0000000000000000
> [ 1610.168320] R10: ffff88004ea85be0 R11: 000000000000f475 R12:
> ffff880068233300
> [ 1610.168320] R13: 0000000000000003 R14: 0000000000000002 R15:
> ffff880068233300
> [ 1610.168320] FS: 0000000000000000(0000) GS:ffff880077800000(0000)
> knlGS:0000000000000000
> [ 1610.168320] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 1610.168320] CR2: 00007f5bcbd3b0b9 CR3: 0000000001c0f000 CR4:
> 00000000000006f0
> [ 1610.168320] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [ 1610.168320] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> 0000000000000400
> [ 1610.168320] Stack:
> [ 1610.168320] ffffffff00000000 0000000b67c83500 000000076700bac0
> 0000000000000000
> [ 1610.168320] ffff88006700bac0 ffff880068233300 ffff88005a945c08
> 0000000000000002
> [ 1610.168320] 0000000000000000 ffff88005a945b88 ffffffffa034e2d5
> 000000065a945b68
> [ 1610.168320] Call Trace:
> [ 1610.168320] [<ffffffffa034e2d5>] nfsd4_get_nfs4_acl+0x95/0x150 [nfsd]
> [ 1610.168320] [<ffffffffa03400d6>] nfsd4_encode_fattr+0x646/0x1e70 [nfsd]
> [ 1610.168320] [<ffffffff816a6e6e>] ? kmemleak_alloc+0x4e/0xb0
> [ 1610.168320] [<ffffffffa0327962>] ?
> nfsd_setuser_and_check_port+0x52/0x80 [nfsd]
> [ 1610.168320] [<ffffffff812cd4bb>] ? selinux_cred_prepare+0x1b/0x30
> [ 1610.168320] [<ffffffffa0341caa>] nfsd4_encode_getattr+0x5a/0x60 [nfsd]
> [ 1610.168320] [<ffffffffa0341e07>]
> nfsd4_encode_operation+0x67/0x110 [nfsd]
> [ 1610.168320] [<ffffffffa033844d>] nfsd4_proc_compound+0x21d/0x810 [nfsd]
> [ 1610.168320] [<ffffffffa0324d9b>] nfsd_dispatch+0xbb/0x200 [nfsd]
> [ 1610.168320] [<ffffffffa00850cd>] svc_process_common+0x46d/0x6d0 [sunrpc]
> [ 1610.168320] [<ffffffffa0085433>] svc_process+0x103/0x170 [sunrpc]
> [ 1610.168320] [<ffffffffa032472f>] nfsd+0xbf/0x130 [nfsd]
> [ 1610.168320] [<ffffffffa0324670>] ? nfsd_destroy+0x80/0x80 [nfsd]
> [ 1610.168320] [<ffffffff810a5202>] kthread+0xd2/0xf0
> [ 1610.168320] [<ffffffff810a5130>] ? insert_kthread_work+0x40/0x40
> [ 1610.168320] [<ffffffff816c1ebc>] ret_from_fork+0x7c/0xb0
> [ 1610.168320] [<ffffffff810a5130>] ? insert_kthread_work+0x40/0x40
> [ 1610.168320] Code: 78 02 e9 e7 fc ff ff 31 c0 31 d2 31 c9 66 89 45
> ce 41 8b 04 24 66 89 55 d0 66 89 4d d2 48 8d 04 80 49 8d 5c 84 04 e9
> 37 fd ff ff <0f> 0b 90 0f 1f 44 00 00 55 8b 56 08 c7 07 00 00 00 00
> 8b 46 0c
> [ 1610.168320] RIP [<ffffffffa034d5ed>]
> _posix_to_nfsv4_one+0x3cd/0x3d0 [nfsd]
> [ 1610.168320] RSP <ffff88005a945b00>
> [ 1610.257313] ---[ end trace 838254e3e352285b ]---
>
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
> fs/nfsd/nfs4acl.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
> index de5d66b..fdd259e 100644
> --- a/fs/nfsd/nfs4acl.c
> +++ b/fs/nfsd/nfs4acl.c
> @@ -395,8 +395,10 @@ sort_pacl(struct posix_acl *pacl)
> * by uid/gid. */
> int i, j;
>
> - if (pacl->a_count <= 4)
> - return; /* no users or groups */
> + /* no users or groups */
> + if (!pacl || pacl->a_count <= 4)
> + return;
> +
> i = 1;
> while (pacl->a_entries[i].e_tag == ACL_USER)
> i++;
> @@ -523,13 +525,11 @@ posix_state_to_acl(struct posix_acl_state
> *state, unsigned int flags)
>
> /*
> * ACLs with no ACEs are treated differently in the inheritable
> - * and effective cases: when there are no inheritable ACEs, we
> - * set a zero-length default posix acl:
> + * and effective cases.
> */
> - if (state->empty && (flags & NFS4_ACL_TYPE_DEFAULT)) {
> - pacl = posix_acl_alloc(0, GFP_KERNEL);
> - return pacl ? pacl : ERR_PTR(-ENOMEM);
> - }
> + if (state->empty && (flags & NFS4_ACL_TYPE_DEFAULT))
> + return NULL;
> +
> /*
> * When there are no effective ACEs, the following will end
> * up setting a 3-element effective posix ACL with all
> @@ -831,7 +831,7 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp,
> struct svc_fh *fhp,
> if (host_error < 0)
> goto out_release;
>
> - if (S_ISDIR(inode->i_mode)) {
> + if (dpacl && S_ISDIR(inode->i_mode)) {
> host_error = inode->i_op->set_acl(inode, dpacl,
> ACL_TYPE_DEFAULT);
> }
> --
> 1.9.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] NFSD: Clear cached acl after setting a zero-length default posix acl:
2014-04-18 12:13 ` J. Bruce Fields
@ 2014-04-18 12:26 ` Kinglong Mee
0 siblings, 0 replies; 11+ messages in thread
From: Kinglong Mee @ 2014-04-18 12:26 UTC (permalink / raw)
To: J. Bruce Fields, Christoph Hellwig; +Cc: linux-nfs
On 2014/4/18 20:13, J. Bruce Fields wrote:
> On Thu, Apr 17, 2014 at 07:36:34AM -0700, Christoph Hellwig wrote:
>> On Thu, Apr 17, 2014 at 08:46:11PM +0800, Kinglong Mee wrote:
>>> After setting ACL for directory, I got two problems that caused
>>> by the cached zero-length default posix acl.
>>>
>>> This patch just clears the cached zero-length default posix acl
>>> after setting.
>>>
>>> First problem:
>>> # nfs4_setfacl -s A::OWNER@:RWX /mnt/123/; touch /mnt/123/test
>>> ............ hang ...........
>>
>> Nfsd must not call forget_cached_acl, that's the filesystems job.
>> I think the right fix is to make sure nfsd4_set_nfs4_acl calls ->set_acl
>> with a NULL ACL structure if there are no entries.
>>
>> Btw, it would be really good if we kept tests like this as a regression
>> test suite. Is there one for NFS already? If not we could add
>> nfs-specific tests to xfstests as well.
>
> I'd recommend pynfs for something like this.
>
> (It talks NFSv4 directly to the server, so won't depend on client
> acl-caching behavior, etc.)
>
Make sense.
I will try to add some test cases for ACL to pynfs.
Thanks,
Kinglong Mee
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] NFSD: Call ->set_acl with a NULL ACL structure if no entries
2014-04-18 12:19 ` J. Bruce Fields
@ 2014-04-18 12:49 ` Kinglong Mee
2014-05-08 16:41 ` J. Bruce Fields
2014-04-18 12:56 ` [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries Kinglong Mee
2014-04-18 13:47 ` Christoph Hellwig
2 siblings, 1 reply; 11+ messages in thread
From: Kinglong Mee @ 2014-04-18 12:49 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Christoph Hellwig, linux-nfs
After setting ACL for directory, I got two problems that caused
by the cached zero-length default posix acl.
This patch make sure nfsd4_set_nfs4_acl calls ->set_acl
with a NULL ACL structure if there are no entries.
Thanks for Christoph Hellwig's advice.
First problem:
............ hang ...........
Second problem:
[ 1610.167668] ------------[ cut here ]------------
[ 1610.168320] kernel BUG at /root/nfs/linux/fs/nfsd/nfs4acl.c:239!
[ 1610.168320] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
[ 1610.168320] Modules linked in: nfsv4(OE) nfs(OE) nfsd(OE)
rpcsec_gss_krb5 fscache ip6t_rpfilter ip6t_REJECT cfg80211 xt_conntrack
rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables
ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6
ip6table_mangle ip6table_security ip6table_raw ip6table_filter
ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4
nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw
auth_rpcgss nfs_acl snd_intel8x0 ppdev lockd snd_ac97_codec ac97_bus
snd_pcm snd_timer e1000 pcspkr parport_pc snd parport serio_raw joydev
i2c_piix4 sunrpc(OE) microcode soundcore i2c_core ata_generic pata_acpi
[last unloaded: nfsd]
[ 1610.168320] CPU: 0 PID: 27397 Comm: nfsd Tainted: G OE
3.15.0-rc1+ #15
[ 1610.168320] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS
VirtualBox 12/01/2006
[ 1610.168320] task: ffff88005ab653d0 ti: ffff88005a944000 task.ti:
ffff88005a944000
[ 1610.168320] RIP: 0010:[<ffffffffa034d5ed>] [<ffffffffa034d5ed>]
_posix_to_nfsv4_one+0x3cd/0x3d0 [nfsd]
[ 1610.168320] RSP: 0018:ffff88005a945b00 EFLAGS: 00010293
[ 1610.168320] RAX: 0000000000000001 RBX: ffff88006700bac0 RCX:
0000000000000000
[ 1610.168320] RDX: 0000000000000000 RSI: ffff880067c83f00 RDI:
ffff880068233300
[ 1610.168320] RBP: ffff88005a945b48 R08: ffffffff81c64830 R09:
0000000000000000
[ 1610.168320] R10: ffff88004ea85be0 R11: 000000000000f475 R12:
ffff880068233300
[ 1610.168320] R13: 0000000000000003 R14: 0000000000000002 R15:
ffff880068233300
[ 1610.168320] FS: 0000000000000000(0000) GS:ffff880077800000(0000)
knlGS:0000000000000000
[ 1610.168320] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 1610.168320] CR2: 00007f5bcbd3b0b9 CR3: 0000000001c0f000 CR4:
00000000000006f0
[ 1610.168320] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 1610.168320] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[ 1610.168320] Stack:
[ 1610.168320] ffffffff00000000 0000000b67c83500 000000076700bac0
0000000000000000
[ 1610.168320] ffff88006700bac0 ffff880068233300 ffff88005a945c08
0000000000000002
[ 1610.168320] 0000000000000000 ffff88005a945b88 ffffffffa034e2d5
000000065a945b68
[ 1610.168320] Call Trace:
[ 1610.168320] [<ffffffffa034e2d5>] nfsd4_get_nfs4_acl+0x95/0x150 [nfsd]
[ 1610.168320] [<ffffffffa03400d6>] nfsd4_encode_fattr+0x646/0x1e70 [nfsd]
[ 1610.168320] [<ffffffff816a6e6e>] ? kmemleak_alloc+0x4e/0xb0
[ 1610.168320] [<ffffffffa0327962>] ?
nfsd_setuser_and_check_port+0x52/0x80 [nfsd]
[ 1610.168320] [<ffffffff812cd4bb>] ? selinux_cred_prepare+0x1b/0x30
[ 1610.168320] [<ffffffffa0341caa>] nfsd4_encode_getattr+0x5a/0x60 [nfsd]
[ 1610.168320] [<ffffffffa0341e07>] nfsd4_encode_operation+0x67/0x110
[nfsd]
[ 1610.168320] [<ffffffffa033844d>] nfsd4_proc_compound+0x21d/0x810 [nfsd]
[ 1610.168320] [<ffffffffa0324d9b>] nfsd_dispatch+0xbb/0x200 [nfsd]
[ 1610.168320] [<ffffffffa00850cd>] svc_process_common+0x46d/0x6d0 [sunrpc]
[ 1610.168320] [<ffffffffa0085433>] svc_process+0x103/0x170 [sunrpc]
[ 1610.168320] [<ffffffffa032472f>] nfsd+0xbf/0x130 [nfsd]
[ 1610.168320] [<ffffffffa0324670>] ? nfsd_destroy+0x80/0x80 [nfsd]
[ 1610.168320] [<ffffffff810a5202>] kthread+0xd2/0xf0
[ 1610.168320] [<ffffffff810a5130>] ? insert_kthread_work+0x40/0x40
[ 1610.168320] [<ffffffff816c1ebc>] ret_from_fork+0x7c/0xb0
[ 1610.168320] [<ffffffff810a5130>] ? insert_kthread_work+0x40/0x40
[ 1610.168320] Code: 78 02 e9 e7 fc ff ff 31 c0 31 d2 31 c9 66 89 45 ce
41 8b 04 24 66 89 55 d0 66 89 4d d2 48 8d 04 80 49 8d 5c 84 04 e9 37 fd
ff ff <0f> 0b 90 0f 1f 44 00 00 55 8b 56 08 c7 07 00 00 00 00 8b 46 0c
[ 1610.168320] RIP [<ffffffffa034d5ed>] _posix_to_nfsv4_one+0x3cd/0x3d0
[nfsd]
[ 1610.168320] RSP <ffff88005a945b00>
[ 1610.257313] ---[ end trace 838254e3e352285b ]---
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
fs/nfsd/nfs4acl.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
index de5d66b..babcc3c 100644
--- a/fs/nfsd/nfs4acl.c
+++ b/fs/nfsd/nfs4acl.c
@@ -395,8 +395,10 @@ sort_pacl(struct posix_acl *pacl)
* by uid/gid. */
int i, j;
- if (pacl->a_count <= 4)
- return; /* no users or groups */
+ /* no users or groups */
+ if (!pacl || pacl->a_count <= 4)
+ return;
+
i = 1;
while (pacl->a_entries[i].e_tag == ACL_USER)
i++;
@@ -523,13 +525,12 @@ posix_state_to_acl(struct posix_acl_state *state,
unsigned int flags)
/*
* ACLs with no ACEs are treated differently in the inheritable
- * and effective cases: when there are no inheritable ACEs, we
- * set a zero-length default posix acl:
+ * and effective cases: when there are no inheritable ACEs,
+ * calls ->set_acl with a NULL ACL structure.
*/
- if (state->empty && (flags & NFS4_ACL_TYPE_DEFAULT)) {
- pacl = posix_acl_alloc(0, GFP_KERNEL);
- return pacl ? pacl : ERR_PTR(-ENOMEM);
- }
+ if (state->empty && (flags & NFS4_ACL_TYPE_DEFAULT))
+ return NULL;
+
/*
* When there are no effective ACEs, the following will end
* up setting a 3-element effective posix ACL with all
--
1.9.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries
2014-04-18 12:19 ` J. Bruce Fields
2014-04-18 12:49 ` [PATCH v3] NFSD: Call ->set_acl with a NULL ACL structure if no entries Kinglong Mee
@ 2014-04-18 12:56 ` Kinglong Mee
2014-04-18 13:47 ` Christoph Hellwig
2 siblings, 0 replies; 11+ messages in thread
From: Kinglong Mee @ 2014-04-18 12:56 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Christoph Hellwig, linux-nfs
On 2014/4/18 20:19, J. Bruce Fields wrote:
> On Fri, Apr 18, 2014 at 11:48:50AM +0800, Kinglong Mee wrote:
>> After setting ACL for directory, I got two problems that caused
>> by the cached zero-length default posix acl.
>>
>> This patch make sure nfsd don't set a zero-length default
>> posix ACL if there are no entries for the default ACL.
>>
>> Thanks for Christoph Hellwig's advice.
>>
>> v2:
>> drop calling forget_cached_acl(), just not set zero-length ACL
>
> I thought Christoph's suggestion was to actually call ->set_acl with a
> NULL ACL? Does that turn out to be unnecessary?
I have test with two versions(call set_acl with NULL ACL
and not call set_acl), test results are no different.
Maybe calling set_acl with NULL ACL is better.
The new version patch have be sent.
thanks
Kinglong Mee
>>
>> First problem:
>> # nfs4_setfacl -s A::OWNER@:RWX /mnt/123/; touch /mnt/123/test
>> ............ hang ...........
>>
>> Second problem:
>> # nfs4_setfacl -s A::OWNER@:RWX /mnt/123/; nfs4_getfacl /mnt/123/
>> [ 1610.167668] ------------[ cut here ]------------
>> [ 1610.168320] kernel BUG at /root/nfs/linux/fs/nfsd/nfs4acl.c:239!
>> [ 1610.168320] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>> [ 1610.168320] Modules linked in: nfsv4(OE) nfs(OE) nfsd(OE)
>> rpcsec_gss_krb5 fscache ip6t_rpfilter ip6t_REJECT cfg80211
>> xt_conntrack rfkill ebtable_nat ebtable_broute bridge stp llc
>> ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6
>> nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security
>> ip6table_raw ip6table_filter ip6_tables iptable_nat
>> nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
>> iptable_mangle iptable_security iptable_raw auth_rpcgss nfs_acl
>> snd_intel8x0 ppdev lockd snd_ac97_codec ac97_bus snd_pcm snd_timer
>> e1000 pcspkr parport_pc snd parport serio_raw joydev i2c_piix4
>> sunrpc(OE) microcode soundcore i2c_core ata_generic pata_acpi [last
>> unloaded: nfsd]
>> [ 1610.168320] CPU: 0 PID: 27397 Comm: nfsd Tainted: G OE
>> 3.15.0-rc1+ #15
>> [ 1610.168320] Hardware name: innotek GmbH VirtualBox/VirtualBox,
>> BIOS VirtualBox 12/01/2006
>> [ 1610.168320] task: ffff88005ab653d0 ti: ffff88005a944000 task.ti:
>> ffff88005a944000
>> [ 1610.168320] RIP: 0010:[<ffffffffa034d5ed>] [<ffffffffa034d5ed>]
>> _posix_to_nfsv4_one+0x3cd/0x3d0 [nfsd]
>> [ 1610.168320] RSP: 0018:ffff88005a945b00 EFLAGS: 00010293
>> [ 1610.168320] RAX: 0000000000000001 RBX: ffff88006700bac0 RCX:
>> 0000000000000000
>> [ 1610.168320] RDX: 0000000000000000 RSI: ffff880067c83f00 RDI:
>> ffff880068233300
>> [ 1610.168320] RBP: ffff88005a945b48 R08: ffffffff81c64830 R09:
>> 0000000000000000
>> [ 1610.168320] R10: ffff88004ea85be0 R11: 000000000000f475 R12:
>> ffff880068233300
>> [ 1610.168320] R13: 0000000000000003 R14: 0000000000000002 R15:
>> ffff880068233300
>> [ 1610.168320] FS: 0000000000000000(0000) GS:ffff880077800000(0000)
>> knlGS:0000000000000000
>> [ 1610.168320] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>> [ 1610.168320] CR2: 00007f5bcbd3b0b9 CR3: 0000000001c0f000 CR4:
>> 00000000000006f0
>> [ 1610.168320] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
>> 0000000000000000
>> [ 1610.168320] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
>> 0000000000000400
>> [ 1610.168320] Stack:
>> [ 1610.168320] ffffffff00000000 0000000b67c83500 000000076700bac0
>> 0000000000000000
>> [ 1610.168320] ffff88006700bac0 ffff880068233300 ffff88005a945c08
>> 0000000000000002
>> [ 1610.168320] 0000000000000000 ffff88005a945b88 ffffffffa034e2d5
>> 000000065a945b68
>> [ 1610.168320] Call Trace:
>> [ 1610.168320] [<ffffffffa034e2d5>] nfsd4_get_nfs4_acl+0x95/0x150 [nfsd]
>> [ 1610.168320] [<ffffffffa03400d6>] nfsd4_encode_fattr+0x646/0x1e70 [nfsd]
>> [ 1610.168320] [<ffffffff816a6e6e>] ? kmemleak_alloc+0x4e/0xb0
>> [ 1610.168320] [<ffffffffa0327962>] ?
>> nfsd_setuser_and_check_port+0x52/0x80 [nfsd]
>> [ 1610.168320] [<ffffffff812cd4bb>] ? selinux_cred_prepare+0x1b/0x30
>> [ 1610.168320] [<ffffffffa0341caa>] nfsd4_encode_getattr+0x5a/0x60 [nfsd]
>> [ 1610.168320] [<ffffffffa0341e07>]
>> nfsd4_encode_operation+0x67/0x110 [nfsd]
>> [ 1610.168320] [<ffffffffa033844d>] nfsd4_proc_compound+0x21d/0x810 [nfsd]
>> [ 1610.168320] [<ffffffffa0324d9b>] nfsd_dispatch+0xbb/0x200 [nfsd]
>> [ 1610.168320] [<ffffffffa00850cd>] svc_process_common+0x46d/0x6d0 [sunrpc]
>> [ 1610.168320] [<ffffffffa0085433>] svc_process+0x103/0x170 [sunrpc]
>> [ 1610.168320] [<ffffffffa032472f>] nfsd+0xbf/0x130 [nfsd]
>> [ 1610.168320] [<ffffffffa0324670>] ? nfsd_destroy+0x80/0x80 [nfsd]
>> [ 1610.168320] [<ffffffff810a5202>] kthread+0xd2/0xf0
>> [ 1610.168320] [<ffffffff810a5130>] ? insert_kthread_work+0x40/0x40
>> [ 1610.168320] [<ffffffff816c1ebc>] ret_from_fork+0x7c/0xb0
>> [ 1610.168320] [<ffffffff810a5130>] ? insert_kthread_work+0x40/0x40
>> [ 1610.168320] Code: 78 02 e9 e7 fc ff ff 31 c0 31 d2 31 c9 66 89 45
>> ce 41 8b 04 24 66 89 55 d0 66 89 4d d2 48 8d 04 80 49 8d 5c 84 04 e9
>> 37 fd ff ff <0f> 0b 90 0f 1f 44 00 00 55 8b 56 08 c7 07 00 00 00 00
>> 8b 46 0c
>> [ 1610.168320] RIP [<ffffffffa034d5ed>]
>> _posix_to_nfsv4_one+0x3cd/0x3d0 [nfsd]
>> [ 1610.168320] RSP <ffff88005a945b00>
>> [ 1610.257313] ---[ end trace 838254e3e352285b ]---
>>
>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>> ---
>> fs/nfsd/nfs4acl.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
>> index de5d66b..fdd259e 100644
>> --- a/fs/nfsd/nfs4acl.c
>> +++ b/fs/nfsd/nfs4acl.c
>> @@ -395,8 +395,10 @@ sort_pacl(struct posix_acl *pacl)
>> * by uid/gid. */
>> int i, j;
>>
>> - if (pacl->a_count <= 4)
>> - return; /* no users or groups */
>> + /* no users or groups */
>> + if (!pacl || pacl->a_count <= 4)
>> + return;
>> +
>> i = 1;
>> while (pacl->a_entries[i].e_tag == ACL_USER)
>> i++;
>> @@ -523,13 +525,11 @@ posix_state_to_acl(struct posix_acl_state
>> *state, unsigned int flags)
>>
>> /*
>> * ACLs with no ACEs are treated differently in the inheritable
>> - * and effective cases: when there are no inheritable ACEs, we
>> - * set a zero-length default posix acl:
>> + * and effective cases.
>> */
>> - if (state->empty && (flags & NFS4_ACL_TYPE_DEFAULT)) {
>> - pacl = posix_acl_alloc(0, GFP_KERNEL);
>> - return pacl ? pacl : ERR_PTR(-ENOMEM);
>> - }
>> + if (state->empty && (flags & NFS4_ACL_TYPE_DEFAULT))
>> + return NULL;
>> +
>> /*
>> * When there are no effective ACEs, the following will end
>> * up setting a 3-element effective posix ACL with all
>> @@ -831,7 +831,7 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp,
>> struct svc_fh *fhp,
>> if (host_error < 0)
>> goto out_release;
>>
>> - if (S_ISDIR(inode->i_mode)) {
>> + if (dpacl && S_ISDIR(inode->i_mode)) {
>> host_error = inode->i_op->set_acl(inode, dpacl,
>> ACL_TYPE_DEFAULT);
>> }
>> --
>> 1.9.0
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries
2014-04-18 12:19 ` J. Bruce Fields
2014-04-18 12:49 ` [PATCH v3] NFSD: Call ->set_acl with a NULL ACL structure if no entries Kinglong Mee
2014-04-18 12:56 ` [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries Kinglong Mee
@ 2014-04-18 13:47 ` Christoph Hellwig
2 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2014-04-18 13:47 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Kinglong Mee, linux-nfs
On Fri, Apr 18, 2014 at 08:19:20AM -0400, J. Bruce Fields wrote:
> I thought Christoph's suggestion was to actually call ->set_acl with a
> NULL ACL? Does that turn out to be unnecessary?
It's a no-op if there wasn't a previous ACL on the inode, but if there
was it needs to be removed by passing the NULL acl argument.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] NFSD: Call ->set_acl with a NULL ACL structure if no entries
2014-04-18 12:49 ` [PATCH v3] NFSD: Call ->set_acl with a NULL ACL structure if no entries Kinglong Mee
@ 2014-05-08 16:41 ` J. Bruce Fields
0 siblings, 0 replies; 11+ messages in thread
From: J. Bruce Fields @ 2014-05-08 16:41 UTC (permalink / raw)
To: Kinglong Mee; +Cc: Christoph Hellwig, linux-nfs
Applying this version for 3.15 and stable, thanks.
--b.
On Fri, Apr 18, 2014 at 08:49:04PM +0800, Kinglong Mee wrote:
> After setting ACL for directory, I got two problems that caused
> by the cached zero-length default posix acl.
>
> This patch make sure nfsd4_set_nfs4_acl calls ->set_acl
> with a NULL ACL structure if there are no entries.
>
> Thanks for Christoph Hellwig's advice.
>
> First problem:
> ............ hang ...........
>
> Second problem:
> [ 1610.167668] ------------[ cut here ]------------
> [ 1610.168320] kernel BUG at /root/nfs/linux/fs/nfsd/nfs4acl.c:239!
> [ 1610.168320] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
> [ 1610.168320] Modules linked in: nfsv4(OE) nfs(OE) nfsd(OE)
> rpcsec_gss_krb5 fscache ip6t_rpfilter ip6t_REJECT cfg80211
> xt_conntrack rfkill ebtable_nat ebtable_broute bridge stp llc
> ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6
> nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security
> ip6table_raw ip6table_filter ip6_tables iptable_nat
> nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
> iptable_mangle iptable_security iptable_raw auth_rpcgss nfs_acl
> snd_intel8x0 ppdev lockd snd_ac97_codec ac97_bus snd_pcm snd_timer
> e1000 pcspkr parport_pc snd parport serio_raw joydev i2c_piix4
> sunrpc(OE) microcode soundcore i2c_core ata_generic pata_acpi [last
> unloaded: nfsd]
> [ 1610.168320] CPU: 0 PID: 27397 Comm: nfsd Tainted: G OE
> 3.15.0-rc1+ #15
> [ 1610.168320] Hardware name: innotek GmbH VirtualBox/VirtualBox,
> BIOS VirtualBox 12/01/2006
> [ 1610.168320] task: ffff88005ab653d0 ti: ffff88005a944000 task.ti:
> ffff88005a944000
> [ 1610.168320] RIP: 0010:[<ffffffffa034d5ed>] [<ffffffffa034d5ed>]
> _posix_to_nfsv4_one+0x3cd/0x3d0 [nfsd]
> [ 1610.168320] RSP: 0018:ffff88005a945b00 EFLAGS: 00010293
> [ 1610.168320] RAX: 0000000000000001 RBX: ffff88006700bac0 RCX:
> 0000000000000000
> [ 1610.168320] RDX: 0000000000000000 RSI: ffff880067c83f00 RDI:
> ffff880068233300
> [ 1610.168320] RBP: ffff88005a945b48 R08: ffffffff81c64830 R09:
> 0000000000000000
> [ 1610.168320] R10: ffff88004ea85be0 R11: 000000000000f475 R12:
> ffff880068233300
> [ 1610.168320] R13: 0000000000000003 R14: 0000000000000002 R15:
> ffff880068233300
> [ 1610.168320] FS: 0000000000000000(0000) GS:ffff880077800000(0000)
> knlGS:0000000000000000
> [ 1610.168320] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 1610.168320] CR2: 00007f5bcbd3b0b9 CR3: 0000000001c0f000 CR4:
> 00000000000006f0
> [ 1610.168320] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [ 1610.168320] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> 0000000000000400
> [ 1610.168320] Stack:
> [ 1610.168320] ffffffff00000000 0000000b67c83500 000000076700bac0
> 0000000000000000
> [ 1610.168320] ffff88006700bac0 ffff880068233300 ffff88005a945c08
> 0000000000000002
> [ 1610.168320] 0000000000000000 ffff88005a945b88 ffffffffa034e2d5
> 000000065a945b68
> [ 1610.168320] Call Trace:
> [ 1610.168320] [<ffffffffa034e2d5>] nfsd4_get_nfs4_acl+0x95/0x150 [nfsd]
> [ 1610.168320] [<ffffffffa03400d6>] nfsd4_encode_fattr+0x646/0x1e70 [nfsd]
> [ 1610.168320] [<ffffffff816a6e6e>] ? kmemleak_alloc+0x4e/0xb0
> [ 1610.168320] [<ffffffffa0327962>] ?
> nfsd_setuser_and_check_port+0x52/0x80 [nfsd]
> [ 1610.168320] [<ffffffff812cd4bb>] ? selinux_cred_prepare+0x1b/0x30
> [ 1610.168320] [<ffffffffa0341caa>] nfsd4_encode_getattr+0x5a/0x60 [nfsd]
> [ 1610.168320] [<ffffffffa0341e07>]
> nfsd4_encode_operation+0x67/0x110 [nfsd]
> [ 1610.168320] [<ffffffffa033844d>] nfsd4_proc_compound+0x21d/0x810 [nfsd]
> [ 1610.168320] [<ffffffffa0324d9b>] nfsd_dispatch+0xbb/0x200 [nfsd]
> [ 1610.168320] [<ffffffffa00850cd>] svc_process_common+0x46d/0x6d0 [sunrpc]
> [ 1610.168320] [<ffffffffa0085433>] svc_process+0x103/0x170 [sunrpc]
> [ 1610.168320] [<ffffffffa032472f>] nfsd+0xbf/0x130 [nfsd]
> [ 1610.168320] [<ffffffffa0324670>] ? nfsd_destroy+0x80/0x80 [nfsd]
> [ 1610.168320] [<ffffffff810a5202>] kthread+0xd2/0xf0
> [ 1610.168320] [<ffffffff810a5130>] ? insert_kthread_work+0x40/0x40
> [ 1610.168320] [<ffffffff816c1ebc>] ret_from_fork+0x7c/0xb0
> [ 1610.168320] [<ffffffff810a5130>] ? insert_kthread_work+0x40/0x40
> [ 1610.168320] Code: 78 02 e9 e7 fc ff ff 31 c0 31 d2 31 c9 66 89 45
> ce 41 8b 04 24 66 89 55 d0 66 89 4d d2 48 8d 04 80 49 8d 5c 84 04 e9
> 37 fd ff ff <0f> 0b 90 0f 1f 44 00 00 55 8b 56 08 c7 07 00 00 00 00
> 8b 46 0c
> [ 1610.168320] RIP [<ffffffffa034d5ed>]
> _posix_to_nfsv4_one+0x3cd/0x3d0 [nfsd]
> [ 1610.168320] RSP <ffff88005a945b00>
> [ 1610.257313] ---[ end trace 838254e3e352285b ]---
>
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
> fs/nfsd/nfs4acl.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
> index de5d66b..babcc3c 100644
> --- a/fs/nfsd/nfs4acl.c
> +++ b/fs/nfsd/nfs4acl.c
> @@ -395,8 +395,10 @@ sort_pacl(struct posix_acl *pacl)
> * by uid/gid. */
> int i, j;
>
> - if (pacl->a_count <= 4)
> - return; /* no users or groups */
> + /* no users or groups */
> + if (!pacl || pacl->a_count <= 4)
> + return;
> +
> i = 1;
> while (pacl->a_entries[i].e_tag == ACL_USER)
> i++;
> @@ -523,13 +525,12 @@ posix_state_to_acl(struct posix_acl_state
> *state, unsigned int flags)
>
> /*
> * ACLs with no ACEs are treated differently in the inheritable
> - * and effective cases: when there are no inheritable ACEs, we
> - * set a zero-length default posix acl:
> + * and effective cases: when there are no inheritable ACEs,
> + * calls ->set_acl with a NULL ACL structure.
> */
> - if (state->empty && (flags & NFS4_ACL_TYPE_DEFAULT)) {
> - pacl = posix_acl_alloc(0, GFP_KERNEL);
> - return pacl ? pacl : ERR_PTR(-ENOMEM);
> - }
> + if (state->empty && (flags & NFS4_ACL_TYPE_DEFAULT))
> + return NULL;
> +
> /*
> * When there are no effective ACEs, the following will end
> * up setting a 3-element effective posix ACL with all
> --
> 1.9.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-05-08 16:41 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-17 12:46 [PATCH] NFSD: Clear cached acl after setting a zero-length default posix acl: Kinglong Mee
2014-04-17 14:36 ` Christoph Hellwig
2014-04-18 3:48 ` [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries Kinglong Mee
2014-04-18 12:19 ` J. Bruce Fields
2014-04-18 12:49 ` [PATCH v3] NFSD: Call ->set_acl with a NULL ACL structure if no entries Kinglong Mee
2014-05-08 16:41 ` J. Bruce Fields
2014-04-18 12:56 ` [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries Kinglong Mee
2014-04-18 13:47 ` Christoph Hellwig
2014-04-18 4:03 ` [PATCH] NFSD: Clear cached acl after setting a zero-length default posix acl: Kinglong Mee
2014-04-18 12:13 ` J. Bruce Fields
2014-04-18 12:26 ` Kinglong Mee
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).