diff mbox series

[v2] nfsd: prevent panic for nfsv4.0 closed files in nfs4_show_open

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

Commit Message

Olga Kornievskaia Aug. 23, 2024, 3:51 p.m. UTC
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(-)

Comments

Jeff Layton Aug. 23, 2024, 4:02 p.m. UTC | #1
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>
Chuck Lever Aug. 23, 2024, 4:30 p.m. UTC | #2
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 mbox series

Patch

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