NFSD: Clear cached acl after setting a zero-length default posix acl:

PATCH In review Kinglong Mee 3 participants 11 messages Apr 17, 2014 – May 8, 2014

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Kinglong Mee

author

2014-04-17 12:46 UTC #0 lore raw

PATCH 1/1 NFSD: Clear cached acl after setting a zero-length default posix acl:

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 <[email protected]>

---
 fs/nfsd/nfs4acl.c | 2 ++
 1 file changed, 2 insertions(+)

fs/nfsd/nfs4acl.c +2 -0

index de5d66b..4eb0a9f 100644
@@ -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:

Signature · 1 line

1.9.0

Christoph Hellwig

2014-04-17 14:36 UTC #1 lore raw

Quoted 9 lines · Kinglong Mee

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.

Kinglong Mee

author

2014-04-18 03:49 UTC #2 lore raw

PATCH v2 1/1 NFSD: Don't set default ACL if there are no ACE entries

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 <[email protected]>

---
 fs/nfsd/nfs4acl.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

fs/nfsd/nfs4acl.c +9 -9

index de5d66b..fdd259e 100644
@@ -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);
 	}

Signature · 1 line

1.9.0

J. Bruce Fields

2014-04-18 12:19 UTC #3 lore raw

Re: [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries

Quoted 10 lines · Kinglong Mee

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.

Quoted 140 lines

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 <[email protected]>

---
 fs/nfsd/nfs4acl.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

fs/nfsd/nfs4acl.c +8 -8

index de5d66b..fdd259e 100644
@@ -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

fs/nfsd/nfs4acl.c +1 -1

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

}

Signature · 1 line

1.9.0

Kinglong Mee

author

2014-04-18 12:49 UTC #4 lore raw

PATCH v3 1/1 NFSD: Call ->set_acl with a NULL ACL structure if no entries

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 <[email protected]>

---
 fs/nfsd/nfs4acl.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

fs/nfsd/nfs4acl.c +9 -8

index de5d66b..babcc3c 100644
@@ -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

Signature · 1 line

1.9.0

J. Bruce Fields

2014-05-08 16:41 UTC #5 lore raw

Re: [PATCH v3] NFSD: Call ->set_acl with a NULL ACL structure if no entries

Applying this version for 3.15 and stable, thanks.

--b.

Quoted 136 lines · Kinglong Mee

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 <[email protected]>

---
 fs/nfsd/nfs4acl.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

fs/nfsd/nfs4acl.c +9 -8

index de5d66b..babcc3c 100644
@@ -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

Signature · 1 line

1.9.0

Kinglong Mee

author

2014-04-18 12:57 UTC #6 lore raw

Re: [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries

In reply to J. Bruce Fields

Quoted 7 lines · J. Bruce Fields

Quoted 6 lines · Kinglong Mee
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.

Quoted 7 lines

Quoted 4 lines
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

Quoted 141 lines

Quoted 140 lines

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 <[email protected]>

---
  fs/nfsd/nfs4acl.c | 18 +++++++++---------
  1 file changed, 9 insertions(+), 9 deletions(-)

fs/nfsd/nfs4acl.c +8 -8

index de5d66b..fdd259e 100644
@@ -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

fs/nfsd/nfs4acl.c +1 -1

@@ -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

Christoph Hellwig

2014-04-18 13:47 UTC #7 lore raw

Re: [PATCH v2] NFSD: Don't set default ACL if there are no ACE entries

In reply to J. Bruce Fields

Quoted 2 lines · J. Bruce Fields

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.

Kinglong Mee

author

2014-04-18 04:03 UTC #8 lore raw

In reply to Christoph Hellwig

Quoted 7 lines · Christoph Hellwig

Quoted 6 lines · Kinglong Mee
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.

Quoted 7 lines

Quoted 3 lines
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.

Quoted 4 lines

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 <[email protected]>
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 <[email protected]>

---
 runtests | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

runtests +8 -8

index 25d4a9d..d0eeedd 100755
@@ -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

Signature · 1 line

1.9.0

J. Bruce Fields

2014-04-18 12:13 UTC #9 lore raw

In reply to Christoph Hellwig

Quoted 11 lines · Christoph Hellwig

Quoted 9 lines · Kinglong Mee
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 ...........

Quoted 7 lines

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.

Kinglong Mee

author

2014-04-18 12:26 UTC #10 lore raw

Quoted 16 lines · J. Bruce Fields

Quoted 15 lines · Christoph Hellwig
Quoted 9 lines · Kinglong Mee
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.

Quoted 9 lines

Quoted 3 lines
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