Re: nfsd: delegated timestamps get written to frozen filesystems

[Jeff Layton <jlayton@kernel.org>]

On Sat, 2026-11-14 at 10:35 +0100, Christian Brauner wrote:
> Hey,
> 
> I'm working on a freeze protection series. This fell out of it and I
> can't just fix it myself.
> 
> Can I leave you with fixing this?
> 

Yes. I'm currently working on a patchset to rework how delegated
timestamps are handled. I'll plan to roll some freeze protection into
the series (or in a trailing one if that's more suitable). No promises
on the timeframe though.

Cheers!
Jeff

> cb_getattr_update_times() and nfsd_update_cmtime_attr() call
> notify_change() under inode_lock() but nothing takes freeze protection
> for the export. So they modify filesystems that are frozen. On ext4 that
> trips the WARN_ON() in ext4_journal_check_start().
> 
> Reproduced on v7.4-rc1 with two NFSv4.2 clients, the second one in its
> own network namespace:
> 
> - client A opens a file O_RDWR and gets a write delegation
> - the export gets frozen
> - client A writes 64k into the file and keeps it open, so the data
>   stays in its page cache
> - client B runs stat on the file
> 
> nfsd sends CB_GETATTR to client A, sees the file was modified and
> updates the timestamps on the frozen filesystem:
> 
>   WARNING: fs/ext4/ext4_jbd2.c:90 at ext4_journal_check_start+0x1f6/0x2d0, CPU#0: nfsd/444
>   Call Trace:
>    __ext4_journal_start_sb+0x7a/0x470
>    ext4_dirty_inode+0xa1/0x120
>    __mark_inode_dirty+0x154/0x1110
>    ext4_setattr+0x2bd/0x24e0
>    notify_change+0x81c/0xee0
>    nfsd4_deleg_getattr_conflict+0x7f0/0x1370
>    nfsd4_encode_fattr4+0x3c3/0x1290
>    nfsd4_encode_getattr+0xd1/0x1d0
>    nfsd4_encode_operation+0x285/0xf80
>    nfsd4_proc_compound+0x5e2/0x23c0
>    nfsd_dispatch+0x26d/0x870
>    svc_process_common+0xfed/0x19a0
>    svc_process+0x600/0xb90
>    svc_recv+0x1969/0x2650
>    nfsd+0x2f8/0xcd0
> 
> The inode's mtime and ctime on the server change while it is frozen.
> That was with delegated timestamps turned off, the
> cb_getattr_update_times() path with them on is the same.
> 
> I think nfsd_update_cmtime_attr() has the same problem
> but I haven't reproduced that variant. It runs from:
> 
> - nfsd4_copy(), nfsd4_do_async_copy(), nfsd4_clone()
> - nfsd4_finalize_deleg_timestamps() <- nfs4_unlock_deleg_lease() <-
>   destroy_unhashed_deleg(), which is reached from DELEGRETURN,
>   nfs4_open_delegation(), the laundromat via revoke_delegation(), admin
>   revocation, client teardown and nfs4_state_shutdown_net()
> 
> mnt_want_write_file() on the delegation's file before inode_lock() would
> do for GETATTR, COPY, CLONE and DELEGRETURN.
> 
> But the laundromat and nfs4_state_shutdown_net() shouldn't block on a
> frozen export, I think? Lease expiry for the whole net would stall and
> nfsd shutdown would hang until the thaw. Those probably want
> sb_start_write_trylock() and skip the timestamp update while frozen.
> 
> The OPEN with truncate path in nfsd4_truncate() had the same problem.
> I've got a fix for that one in my series.
> 
> This goes back to 7e8ae8486e44 ("fs/nfsd: fix update of inode attrs in
> CB_GETATTR") and e5e9b24ab8fa ("nfsd: freeze c/mtime updates with
> outstanding WRITE_ATTRS delegation").

-- 
Jeff Layton <jlayton@kernel.org>