On Fri, 2024-08-23 at 11:51 -0400, Olga Kornievskaia wrote:
> Prior to commit 3f29cc82a84c ("nfsd: split sc_status out of
> sc_type") states_show() relied on sc_type field to be of valid
> type before calling into a subfunction to show content of a
> particular stateid. From that commit, we split the validity of
> the stateid into sc_status and no longer changed sc_type to 0
> while unhashing the stateid. This resulted in kernel oopsing
> for nfsv4.0 opens that stay around and in nfs4_show_open()
> would derefence sc_file which was NULL.
>
> Instead, for closed open stateids forgo displaying information
> that relies of having a valid sc_file.
>
> To reproduce: mount the server with 4.0, read and close
> a file and then on the server cat /proc/fs/nfsd/clients/2/states
>
> [ 513.590804] Call trace:
> [ 513.590925] _raw_spin_lock+0xcc/0x160
> [ 513.591119] nfs4_show_open+0x78/0x2c0 [nfsd]
> [ 513.591412] states_show+0x44c/0x488 [nfsd]
> [ 513.591681] seq_read_iter+0x5d8/0x760
> [ 513.591896] seq_read+0x188/0x208
> [ 513.592075] vfs_read+0x148/0x470
> [ 513.592241] ksys_read+0xcc/0x178
>
> Fixes: 3f29cc82a84c ("nfsd: split sc_status out of sc_type")
> Signed-off-by: Olga Kornievskaia <okorniev@xxxxxxxxxx>
> ---
> fs/nfsd/nfs4state.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c3def49074a4..2b1f8053b9c1 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2786,15 +2786,18 @@ static int nfs4_show_open(struct seq_file *s,
> struct nfs4_stid *st)
> deny & NFS4_SHARE_ACCESS_READ ? "r" : "-",
> deny & NFS4_SHARE_ACCESS_WRITE ? "w" : "-");
>
> - spin_lock(&nf->fi_lock);
> - file = find_any_file_locked(nf);
> - if (file) {
> - nfs4_show_superblock(s, file);
> - seq_puts(s, ", ");
> - nfs4_show_fname(s, file);
> - seq_puts(s, ", ");
> - }
> - spin_unlock(&nf->fi_lock);
> + if (nf) {
> + spin_lock(&nf->fi_lock);
> + file = find_any_file_locked(nf);
> + if (file) {
> + nfs4_show_superblock(s, file);
> + seq_puts(s, ", ");
> + nfs4_show_fname(s, file);
> + seq_puts(s, ", ");
> + }
> + spin_unlock(&nf->fi_lock);
> + } else
> + seq_puts(s, "closed, ");
> nfs4_show_owner(s, oo);
> if (st->sc_status & SC_STATUS_ADMIN_REVOKED)
> seq_puts(s, ", admin-revoked");
Thanks, Olga!
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
