Linux-NFS Archive mirror
 help / color / mirror / Atom feed
From: Trond Myklebust <trondmy@hammerspace.com>
To: "anna@kernel.org" <anna@kernel.org>,
	"hch@infradead.org" <hch@infradead.org>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] NFS: Fix directory delegation verifier checks
Date: Wed, 31 Dec 2025 21:52:35 +0000	[thread overview]
Message-ID: <53d40f4781783f9b79196bb30975b788be8bb969.camel@hammerspace.com> (raw)
In-Reply-To: <aUnq_d93Wo9e-oUD@infradead.org>

Hi Christoph,

On Mon, 2025-12-22 at 17:06 -0800, Christoph Hellwig wrote:
> On Mon, Dec 22, 2025 at 02:35:10PM -0800, Christoph Hellwig wrote:
> > I wish I could actually review this, but I don't actually
> > understand
> > the lookup revalidation logic enough for that.  But it does fix the
> > problem I saw, so at least:
> 
> Actually - I have to take this back.  This patch makes generic/786
> when run on NFS v4.2 go from just failing with:
> 
> "Server reported failure (1)"
> 
> to actually crashing the kernel:
> 
> [   30.195133] run fstests generic/786 at 2025-12-23 01:04:02
> [   36.956689] Oops: general protection fault, probably for non-
> canonical address 0xcccccccccccccd0c: 0000 [#1] SMP NOPTI
> [   36.958741] CPU: 0 UID: 0 PID: 3837 Comm: locktest Tainted:
> G                 N  6.19.0-rc2+ #4523 PREEMPT(full) 
> [   36.960855] Tainted: [N]=TEST
> [   36.961443] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [   36.963167] RIP: 0010:nfs_end_delegation_return+0xda/0x390
> [   36.964321] Code: 49 89 ce e8 18 c1 bf ff 48 8b 85 60 ff ff ff 4c
> 8d 68 80 49 39 c6 74 50 4c 8b 7c 24 08 48 8b 1c 24 4d 8b 65 60 4d 85
> e4 74 2e <49> 8b 44 24 40 a8 02 74 25 49 8b 44 24 40 f6 c4 02 75 1b
> 41 8b 47
> [   36.967856] RSP: 0018:ffffc900018e7cc0 EFLAGS: 00010286
> [   36.968862] RAX: ffff88810efc5e20 RBX: ffff888104fa6970 RCX:
> ffff888112570170
> [   36.970200] RDX: ffff888105780040 RSI: ffff8881189120e8 RDI:
> ffff888112570210
> [   36.971676] RBP: ffff888112570210 R08: 0000000000000000 R09:
> 0000000000000000
> [   36.973041] R10: ffffc900018e7d78 R11: fefefefefefefeff R12:
> cccccccccccccccc
> [   36.974313] R13: ffff88810efc5da0 R14: ffff888112570170 R15:
> ffff888104fa6940
> [   36.975635] FS:  00007f0df5dc4740(0000) GS:ffff8882b3544000(0000)
> knlGS:0000000000000000
> [   36.977106] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   36.978187] CR2: 00007f0df5e26450 CR3: 0000000118e1b001 CR4:
> 0000000000772ef0
> [   36.979629] PKRU: 55555554
> [   36.980205] Call Trace:
> [   36.980706]  <TASK>
> [   36.981131]  ? nfs_clear_verifier_delegated+0x50/0x70
> [   36.982109]  nfs4_proc_setattr+0xff/0x110
> [   36.982894]  nfs_setattr+0x1c8/0x410
> [   36.983634]  notify_change+0x373/0x510
> [   36.984376]  ? chmod_common+0xad/0x160
> [   36.985055]  chmod_common+0xad/0x160
> [   36.985702]  __x64_sys_chmod+0x56/0xb0
> [   36.986388]  do_syscall_64+0x50/0xf80
> [   36.987097]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [   36.988171] RIP: 0033:0x7f0df5ec6707
> [   36.988880] Code: 73 01 c3 48 8b 0d f1 76 0e 00 f7 d8 64 89 01 48
> 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 5a 00 00
> 00 0f 05 <48> 3d 00 f0 ff ff 77 01 c3 48 8b 15 c1 76 0e 00 f7 d8 64
> 89 02 b8
> [   36.992424] RSP: 002b:00007fff3cd5bad8 EFLAGS: 00000206 ORIG_RAX:
> 000000000000005a
> [   36.993747] RAX: ffffffffffffffda RBX: 0000558071f19b90 RCX:
> 00007f0df5ec6707
> [   36.995045] RDX: 0000000000000008 RSI: 00000000000001fd RDI:
> 00007fff3cd5cbc0
> [   36.996499] RBP: 0000000000000000 R08: 00000000000001fd R09:
> 0000000000000000
> [   36.997855] R10: 0000000000000003 R11: 0000000000000206 R12:
> 0000000000000035
> [   36.999218] R13: 0000000000000007 R14: 0000000000000000 R15:
> 0000558071f1ebc0
> [   37.000618]  </TASK>
> [   37.001039] Modules linked in: kvm_intel kvm irqbypass
> [   37.002029] ---[ end trace 0000000000000000 ]---
> [   37.003342] RIP: 0010:nfs_end_delegation_return+0xda/0x390
> [   37.004372] Code: 49 89 ce e8 18 c1 bf ff 48 8b 85 60 ff ff ff 4c
> 8d 68 80 49 39 c6 74 50 4c 8b 7c 24 08 48 8b 1c 24 4d 8b 65 60 4d 85
> e4 74 2e <49> 8b 44 24 40 a8 02 74 25 49 8b 44 24 40 f6 c4 02 75 1b
> 41 8b 47
> [   37.007875] RSP: 0018:ffffc900018e7cc0 EFLAGS: 00010286
> [   37.008902] RAX: ffff88810efc5e20 RBX: ffff888104fa6970 RCX:
> ffff888112570170
> [   37.010307] RDX: ffff888105780040 RSI: ffff8881189120e8 RDI:
> ffff888112570210
> [   37.011702] RBP: ffff888112570210 R08: 0000000000000000 R09:
> 0000000000000000
> [   37.012975] R10: ffffc900018e7d78 R11: fefefefefefefeff R12:
> cccccccccccccccc
> [   37.014271] R13: ffff88810efc5da0 R14: ffff888112570170 R15:
> ffff888104fa6940
> [   37.015600] FS:  00007f0df5dc4740(0000) GS:ffff8882b3544000(0000)
> knlGS:0000000000000000
> [   37.017070] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   37.018190] CR2: 00007f0df5e26450 CR3: 0000000118e1b002 CR4:
> 0000000000772ef0
> [   37.019525] PKRU: 55555554
> [   37.020028] BUG: sleeping function called from invalid context at
> kernel/locking/mutex.c:287
> [   37.021516] in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid:
> 3837, name: locktest
> [   37.022851] preempt_count: 0, expected: 0
> [   37.023565] RCU nest depth: 1, expected: 0
> [   37.024235] CPU: 0 UID: 0 PID: 3837 Comm: locktest Tainted: G     
> D          N  6.19.0-rc2+ #4523 PREEMPT(full) 
> [   37.024240] Tainted: [D]=DIE, [N]=TEST
> [   37.024241] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [   37.024242] Call Trace:
> [   37.024243]  <TASK>
> [   37.024244]  dump_stack_lvl+0x4b/0x70
> [   37.024250]  __might_resched.cold+0xd3/0x10e
> [   37.024255]  mutex_lock+0x19/0x80
> [   37.024259]  sched_mm_cid_exit+0x51/0x1e0
> [   37.024263]  do_exit+0xb0/0xa40
> [   37.024265]  ? __x64_sys_chmod+0x56/0xb0
> [   37.024269]  make_task_dead+0x87/0x90
> [   37.024271]  rewind_stack_and_make_dead+0x16/0x20
> [   37.024275] RIP: 0033:0x7f0df5ec6707
> [   37.024277] Code: 73 01 c3 48 8b 0d f1 76 0e 00 f7 d8 64 89 01 48
> 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 5a 00 00
> 00 0f 05 <48> 3d 00 f0 ff ff 77 01 c3 48 8b 15 c1 76 0e 00 f7 d8 64
> 89 02 b8
> [   37.024278] RSP: 002b:00007fff3cd5bad8 EFLAGS: 00000206 ORIG_RAX:
> 000000000000005a
> [   37.024280] RAX: ffffffffffffffda RBX: 0000558071f19b90 RCX:
> 00007f0df5ec6707
> [   37.024281] RDX: 0000000000000008 RSI: 00000000000001fd RDI:
> 00007fff3cd5cbc0
> [   37.024282] RBP: 0000000000000000 R08: 00000000000001fd R09:
> 0000000000000000
> [   37.024283] R10: 0000000000000003 R11: 0000000000000206 R12:
> 0000000000000035
> [   37.024284] R13: 0000000000000007 R14: 0000000000000000 R15:
> 0000558071f1ebc0
> [   37.024286]  </TASK>
> [   37.024761] ------------[ cut here ]------------
> [   37.041739] Voluntary context switch within RCU read-side critical
> section!
> [   37.041740] WARNING: kernel/rcu/tree_plugin.h:332 at
> rcu_note_context_switch+0x39b/0x5e0, CPU#0: locktest/3837
> [   37.043557] Modules linked in: kvm_intel kvm irqbypass
> [   37.044135] CPU: 0 UID: 0 PID: 3837 Comm: locktest Tainted: G     
> D W        N  6.19.0-rc2+ #4523 PREEMPT(full) 
> [   37.045192] Tainted: [D]=DIE, [W]=WARN, [N]=TEST
> [   37.045680] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [   37.046655] RIP: 0010:rcu_note_context_switch+0x39b/0x5e0
> [   37.047211] Code: ef e8 f9 a7 34 01 c6 45 11 00 48 8b 55 28 4c 89
> ef 48 89 c6 b9 01 00 00 00 e8 91 97 ff ff e9 d6 fc ff ff 48 8d 3d 25
> 4f 86 02 <67> 48 0f b9 3a e9 a6 fc ff ff 4c 8b 6d 20 4c 89 ef e8 bf
> a7 34 01
> [   37.049166] RSP: 0018:ffffc900018e7c18 EFLAGS: 00010002
> [   37.049714] RAX: 0000000000000001 RBX: ffff888105780040 RCX:
> 0000000000000000
> [   37.050451] RDX: 0000000000000001 RSI: ffffffff8304f78e RDI:
> ffffffff83c34c20
> [   37.051193] RBP: ffff888237c2a300 R08: ffff88810c4208f7 R09:
> 0000000000000002
> [   37.051957] R10: 0000000000000001 R11: 0000000000000001 R12:
> 0000000000000000
> [   37.052702] R13: ffffffff846e55c0 R14: ffff888105780040 R15:
> ffffffff82608cb0
> [   37.053437] FS:  0000000000000000(0000) GS:ffff8882b3544000(0000)
> knlGS:0000000000000000
> [   37.054272] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   37.054876] CR2: 00007f0df5e26450 CR3: 000000000325a003 CR4:
> 0000000000772ef0
> [   37.055627] PKRU: 55555554
> [   37.055892] Call Trace:
> [   37.056138]  <TASK>
> [   37.056349]  ? _raw_spin_unlock_irqrestore+0x1d/0x40
> [   37.056819]  ? __pfx_rpc_wait_bit_killable+0x10/0x10
> [   37.057291]  __schedule+0xa1/0xe30
> [   37.057622]  ? preempt_count_add+0x73/0xb0
> [   37.058013]  ? __pfx_rpc_wait_bit_killable+0x10/0x10
> [   37.058494]  schedule+0x29/0xe0
> [   37.058839]  rpc_wait_bit_killable+0xc/0x60
> [   37.059285]  __wait_on_bit+0x2c/0x90
> [   37.059740]  out_of_line_wait_on_bit+0x8e/0xb0
> [   37.060210]  ? __pfx_wake_bit_function+0x10/0x10
> [   37.060700]  nfs4_do_close+0x289/0x310
> [   37.061101]  __put_nfs_open_context+0xc9/0x140
> [   37.061572]  nfs_file_release+0x38/0x50
> [   37.061980]  ? security_file_release+0x17/0x30
> [   37.062442]  __fput+0xf2/0x2b0
> [   37.062773]  task_work_run+0x57/0xa0
> [   37.063153]  do_exit+0x273/0xa40
> [   37.063524]  ? __x64_sys_chmod+0x56/0xb0
> [   37.063937]  make_task_dead+0x87/0x90
> [   37.064331]  rewind_stack_and_make_dead+0x16/0x20
> [   37.064819] RIP: 0033:0x7f0df5ec6707
> [   37.065200] Code: Unable to access opcode bytes at 0x7f0df5ec66dd.
> [   37.065828] RSP: 002b:00007fff3cd5bad8 EFLAGS: 00000206 ORIG_RAX:
> 000000000000005a
> [   37.066601] RAX: ffffffffffffffda RBX: 0000558071f19b90 RCX:
> 00007f0df5ec6707
> [   37.067328] RDX: 0000000000000008 RSI: 00000000000001fd RDI:
> 00007fff3cd5cbc0
> [   37.068095] RBP: 0000000000000000 R08: 00000000000001fd R09:
> 0000000000000000
> [   37.068848] R10: 0000000000000003 R11: 0000000000000206 R12:
> 0000000000000035
> [   37.069585] R13: 0000000000000007 R14: 0000000000000000 R15:
> 0000558071f1ebc0
> [   37.070319]  </TASK>
> [   37.070560] ---[ end trace 0000000000000000 ]---
> 

Does applying the following on top of Anna's patch fix the Oops?

Cheers
  Trond

8<--------------------------------------------------------------
From 3485794e16f68659b26918ccb1c57b6210bd0048 Mon Sep 17 00:00:00 2001
Message-ID: <3485794e16f68659b26918ccb1c57b6210bd0048.1767217813.git.trond.myklebust@hammerspace.com>
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Date: Wed, 31 Dec 2025 16:41:15 -0500
Subject: [PATCH] NFSv4: Fix nfs_clear_verifier_delegated() for delegated
 directories

If the client returns a directory delegation, then look up all the child
dentries, and clear their 'verifier delegated' bit, unless subject to a
file delegation.

Similarly, if a file delegation is being returned, check if there is a
directory delegation before clearing a 'verifier delegated' bit.

Reported-by: Christoph Hellwig <hch@lst.de>
Fixes: 156b09482933 ("NFS: Request a directory delegation on ACCESS, CREATE, and UNLINK")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/dir.c | 59 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 51 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 23a78a742b61..d6222c6c3165 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1440,7 +1440,8 @@ static void nfs_set_verifier_locked(struct dentry *dentry, unsigned long verf)
 
 	if (!dir || !nfs_verify_change_attribute(dir, verf))
 		return;
-	if (inode && NFS_PROTO(inode)->have_delegation(inode, FMODE_READ, 0))
+	if (NFS_PROTO(dir)->have_delegation(dir, FMODE_READ, 0) ||
+	    (inode && NFS_PROTO(inode)->have_delegation(inode, FMODE_READ, 0)))
 		nfs_set_verifier_delegated(&verf);
 	dentry->d_time = verf;
 }
@@ -1465,6 +1466,49 @@ void nfs_set_verifier(struct dentry *dentry, unsigned long verf)
 EXPORT_SYMBOL_GPL(nfs_set_verifier);
 
 #if IS_ENABLED(CONFIG_NFS_V4)
+static void nfs_clear_verifier_file(struct inode *inode)
+{
+	struct dentry *alias;
+	struct inode *dir;
+
+	hlist_for_each_entry(alias, &inode->i_dentry, d_u.d_alias) {
+		spin_lock(&alias->d_lock);
+		dir = d_inode_rcu(alias->d_parent);
+		if (!dir ||
+		    !NFS_PROTO(dir)->have_delegation(dir, FMODE_READ, 0))
+			nfs_unset_verifier_delegated(&alias->d_time);
+		spin_unlock(&alias->d_lock);
+	}
+}
+
+static void nfs_clear_verifier_directory(struct inode *dir)
+{
+	struct dentry *this_parent;
+	struct dentry *dentry;
+	struct inode *inode;
+
+	if (hlist_empty(&dir->i_dentry))
+		return;
+	this_parent =
+		hlist_entry(dir->i_dentry.first, struct dentry, d_u.d_alias);
+
+	spin_lock(&this_parent->d_lock);
+	nfs_unset_verifier_delegated(&this_parent->d_time);
+	dentry = d_first_child(this_parent);
+	hlist_for_each_entry_from(dentry, d_sib) {
+		if (unlikely(dentry->d_flags & DCACHE_DENTRY_CURSOR))
+			continue;
+		inode = d_inode_rcu(dentry);
+		if (inode &&
+		    NFS_PROTO(inode)->have_delegation(inode, FMODE_READ, 0))
+			continue;
+		spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
+		nfs_unset_verifier_delegated(&dentry->d_time);
+		spin_unlock(&dentry->d_lock);
+	}
+	spin_unlock(&this_parent->d_lock);
+}
+
 /**
  * nfs_clear_verifier_delegated - clear the dir verifier delegation tag
  * @inode: pointer to inode
@@ -1477,17 +1521,16 @@ EXPORT_SYMBOL_GPL(nfs_set_verifier);
  */
 void nfs_clear_verifier_delegated(struct inode *inode)
 {
-	struct dentry *alias;
-
 	if (!inode)
 		return;
+	rcu_read_lock();
 	spin_lock(&inode->i_lock);
-	hlist_for_each_entry(alias, &inode->i_dentry, d_u.d_alias) {
-		spin_lock(&alias->d_lock);
-		nfs_unset_verifier_delegated(&alias->d_time);
-		spin_unlock(&alias->d_lock);
-	}
+	if (S_ISREG(inode->i_mode))
+		nfs_clear_verifier_file(inode);
+	else if (S_ISDIR(inode->i_mode))
+		nfs_clear_verifier_directory(inode);
 	spin_unlock(&inode->i_lock);
+	rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(nfs_clear_verifier_delegated);
 #endif /* IS_ENABLED(CONFIG_NFS_V4) */
-- 
2.52.0


  reply	other threads:[~2025-12-31 21:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-19 20:13 [PATCH] NFS: Fix directory delegation verifier checks Anna Schumaker
2025-12-22 22:35 ` Christoph Hellwig
2025-12-23  1:06   ` Christoph Hellwig
2025-12-31 21:52     ` Trond Myklebust [this message]
2026-01-06  6:21       ` hch@infradead.ori
2026-01-06 18:32         ` Trond Myklebust
2026-01-07  5:23           ` hch@infradead.ori
2026-01-07 15:07             ` Trond Myklebust
2026-01-07 15:30               ` hch@infradead.ori
2026-01-08  9:23                 ` hch@infradead.ori
2026-01-07 16:52               ` Anna Schumaker
2026-04-04 18:32       ` Al Viro
2026-04-04 19:07         ` Al Viro
2026-04-05  2:39         ` Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53d40f4781783f9b79196bb30975b788be8bb969.camel@hammerspace.com \
    --to=trondmy@hammerspace.com \
    --cc=anna@kernel.org \
    --cc=hch@infradead.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).