| Message ID | 20240823155108.72516-1-okorniev@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] nfsd: prevent panic for nfsv4.0 closed files in nfs4_show_open | expand |
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@redhat.com> > --- > 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@kernel.org>
On Fri, 23 Aug 2024 11:51:08 -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. > > [...] Applied to nfsd-fixes for v6.11-rc, thanks! [1/1] nfsd: prevent panic for nfsv4.0 closed files in nfs4_show_open commit: a204501e1743d695ca2930ed25a2be9f8ced96d3
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");
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@redhat.com> --- fs/nfsd/nfs4state.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)