mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] NFS: Use common error handling code in nfs_alloc_server()
@ 2026-06-10 16:35 Markus Elfring
  2026-06-13 20:24 ` Christophe JAILLET
  0 siblings, 1 reply; 5+ messages in thread
From: Markus Elfring @ 2026-06-10 16:35 UTC (permalink / raw)
  To: linux-nfs, Anna Schumaker, Trond Myklebust
  Cc: LKML, kernel-janitors, Christian Brauner, Christoph Hellwig, Chuck Lever

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 10 Jun 2026 18:28:17 +0200

Use an additional label so that a bit of exception handling can be better
reused at the end of this function implementation.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/nfs/client.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 73b95318ba48..50482257667d 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1063,10 +1063,8 @@ struct nfs_server *nfs_alloc_server(void)
 		return NULL;
 
 	server->s_sysfs_id = ida_alloc(&s_sysfs_ids, GFP_KERNEL);
-	if (server->s_sysfs_id < 0) {
-		kfree(server);
-		return NULL;
-	}
+	if (server->s_sysfs_id < 0)
+		goto free_server;
 
 	server->client = server->client_acl = ERR_PTR(-EINVAL);
 
@@ -1087,10 +1085,8 @@ struct nfs_server *nfs_alloc_server(void)
 	atomic_long_set(&server->nr_active_delegations, 0);
 
 	server->io_stats = nfs_alloc_iostats();
-	if (!server->io_stats) {
-		kfree(server);
-		return NULL;
-	}
+	if (!server->io_stats)
+		goto free_server;
 
 	server->change_attr_type = NFS4_CHANGE_TYPE_IS_UNDEFINED;
 
@@ -1103,6 +1099,10 @@ struct nfs_server *nfs_alloc_server(void)
 	rpc_init_wait_queue(&server->uoc_rpcwaitq, "NFS UOC");
 
 	return server;
+
+free_server:
+	kfree(server);
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(nfs_alloc_server);
 
-- 
2.54.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] NFS: Use common error handling code in nfs_alloc_server()
  2026-06-10 16:35 [PATCH] NFS: Use common error handling code in nfs_alloc_server() Markus Elfring
@ 2026-06-13 20:24 ` Christophe JAILLET
  2026-06-14  8:37   ` [PATCH v2 0/2] NFS: Fix exception handling " Markus Elfring
  0 siblings, 1 reply; 5+ messages in thread
From: Christophe JAILLET @ 2026-06-13 20:24 UTC (permalink / raw)
  To: Markus Elfring, anna, brauner, chuck.lever, hch, trondmy
  Cc: kernel-janitors, linux-kernel, linux-nfs

Le 10/06/2026 à 18:35, Markus Elfring a écrit :
> From: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> Date: Wed, 10 Jun 2026 18:28:17 +0200
> 
> Use an additional label so that a bit of exception handling can be better
> reused at the end of this function implementation.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> ---
>   fs/nfs/client.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 73b95318ba48..50482257667d 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -1063,10 +1063,8 @@ struct nfs_server *nfs_alloc_server(void)
>   		return NULL;
>   
>   	server->s_sysfs_id = ida_alloc(&s_sysfs_ids, GFP_KERNEL);

If an error occurs after a successful ida_alloc(), then ida_free() needs 
to be called in the error handling path.

CJ

> -	if (server->s_sysfs_id < 0) {
> -		kfree(server);
> -		return NULL;
> -	}
> +	if (server->s_sysfs_id < 0)
> +		goto free_server;
>   
>   	server->client = server->client_acl = ERR_PTR(-EINVAL);
>   
> @@ -1087,10 +1085,8 @@ struct nfs_server *nfs_alloc_server(void)
>   	atomic_long_set(&server->nr_active_delegations, 0);
>   
>   	server->io_stats = nfs_alloc_iostats();
> -	if (!server->io_stats) {
> -		kfree(server);
> -		return NULL;
> -	}
> +	if (!server->io_stats)
> +		goto free_server;
>   
>   	server->change_attr_type = NFS4_CHANGE_TYPE_IS_UNDEFINED;
>   
> @@ -1103,6 +1099,10 @@ struct nfs_server *nfs_alloc_server(void)
>   	rpc_init_wait_queue(&server->uoc_rpcwaitq, "NFS UOC");
>   
>   	return server;
> +
> +free_server:
> +	kfree(server);
> +	return NULL;
>   }
>   EXPORT_SYMBOL_GPL(nfs_alloc_server);
>   


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 0/2] NFS: Fix exception handling in nfs_alloc_server()
  2026-06-13 20:24 ` Christophe JAILLET
@ 2026-06-14  8:37   ` Markus Elfring
  2026-06-14  8:42     ` [PATCH v2 1/2] NFS: Prevent resource leak " Markus Elfring
  2026-06-14  8:43     ` [PATCH v2 2/2] NFS: Use common error handling code " Markus Elfring
  0 siblings, 2 replies; 5+ messages in thread
From: Markus Elfring @ 2026-06-14  8:37 UTC (permalink / raw)
  To: linux-nfs, Anna Schumaker, Benjamin Coddington,
	Christophe Jaillet, Trond Myklebust
  Cc: kernel-janitors, LKML, Christian Brauner, Christoph Hellwig, Chuck Lever

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 14 Jun 2026 10:28:05 +0200
Subject: [PATCH v2 0/2] NFS: Fix exception handling in nfs_alloc_server()

A few update suggestions were taken into account from source code analysis.

Markus Elfring (2):
  Prevent resource leak
  Use common error handling code


v2:
Christophe Jaillet pointed out that ida_free() should be called in
the error handling path after a successful ida_alloc() call.
Thus corresponding tags were added.

See also:
https://elixir.bootlin.com/linux/v7.1-rc7/source/lib/idr.c#L549-L595


 fs/nfs/client.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] NFS: Prevent resource leak in nfs_alloc_server()
  2026-06-14  8:37   ` [PATCH v2 0/2] NFS: Fix exception handling " Markus Elfring
@ 2026-06-14  8:42     ` Markus Elfring
  2026-06-14  8:43     ` [PATCH v2 2/2] NFS: Use common error handling code " Markus Elfring
  1 sibling, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2026-06-14  8:42 UTC (permalink / raw)
  To: linux-nfs, Anna Schumaker, Benjamin Coddington,
	Christophe Jaillet, Trond Myklebust
  Cc: kernel-janitors, LKML, Christian Brauner, Christoph Hellwig, Chuck Lever

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 14 Jun 2026 09:56:35 +0200

It was overlooked to call ida_free() after a failed nfs_alloc_iostats() call.
Thus add the missed function call in an if branch.

Fixes: 1c7251187dc067a6d460cf33ca67da9c1dd87807 ("NFS: add superblock sysfs entries")
Cc: stable@vger.kernel.org
Reported-by: Christophe Jaillet <christophe.jaillet@wanadoo.fr>
Closes: https://lore.kernel.org/linux-nfs/1c8e10c9-def7-4f0d-8aa1-23c8035a38c8@wanadoo.fr/
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/nfs/client.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 73b95318ba48..e15568e388f8 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1088,6 +1088,7 @@ struct nfs_server *nfs_alloc_server(void)
 
 	server->io_stats = nfs_alloc_iostats();
 	if (!server->io_stats) {
+		ida_free(&s_sysfs_ids, server->s_sysfs_id);
 		kfree(server);
 		return NULL;
 	}
-- 
2.54.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] NFS: Use common error handling code in nfs_alloc_server()
  2026-06-14  8:37   ` [PATCH v2 0/2] NFS: Fix exception handling " Markus Elfring
  2026-06-14  8:42     ` [PATCH v2 1/2] NFS: Prevent resource leak " Markus Elfring
@ 2026-06-14  8:43     ` Markus Elfring
  1 sibling, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2026-06-14  8:43 UTC (permalink / raw)
  To: linux-nfs, Anna Schumaker, Benjamin Coddington,
	Christophe Jaillet, Trond Myklebust
  Cc: kernel-janitors, LKML, Christian Brauner, Christoph Hellwig, Chuck Lever

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 14 Jun 2026 10:06:11 +0200

Use an additional label so that a bit of exception handling can be better
reused at the end of this function implementation.

This issue was detected by using the Coccinelle software.

Cc: stable@vger.kernel.org
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/nfs/client.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index e15568e388f8..4dcb91ab3039 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1063,10 +1063,8 @@ struct nfs_server *nfs_alloc_server(void)
 		return NULL;
 
 	server->s_sysfs_id = ida_alloc(&s_sysfs_ids, GFP_KERNEL);
-	if (server->s_sysfs_id < 0) {
-		kfree(server);
-		return NULL;
-	}
+	if (server->s_sysfs_id < 0)
+		goto free_server;
 
 	server->client = server->client_acl = ERR_PTR(-EINVAL);
 
@@ -1089,8 +1087,7 @@ struct nfs_server *nfs_alloc_server(void)
 	server->io_stats = nfs_alloc_iostats();
 	if (!server->io_stats) {
 		ida_free(&s_sysfs_ids, server->s_sysfs_id);
-		kfree(server);
-		return NULL;
+		goto free_server;
 	}
 
 	server->change_attr_type = NFS4_CHANGE_TYPE_IS_UNDEFINED;
@@ -1104,6 +1101,10 @@ struct nfs_server *nfs_alloc_server(void)
 	rpc_init_wait_queue(&server->uoc_rpcwaitq, "NFS UOC");
 
 	return server;
+
+free_server:
+	kfree(server);
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(nfs_alloc_server);
 
-- 
2.54.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-06-14  8:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-10 16:35 [PATCH] NFS: Use common error handling code in nfs_alloc_server() Markus Elfring
2026-06-13 20:24 ` Christophe JAILLET
2026-06-14  8:37   ` [PATCH v2 0/2] NFS: Fix exception handling " Markus Elfring
2026-06-14  8:42     ` [PATCH v2 1/2] NFS: Prevent resource leak " Markus Elfring
2026-06-14  8:43     ` [PATCH v2 2/2] NFS: Use common error handling code " Markus Elfring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®