Linux s390 Architecture development
 help / color / mirror / Atom feed
* [PATCH net] dibs: Unregister dibs_class after error
@ 2026-09-02 14:34 Alexandra Winter
  2026-09-03 14:35 ` sashiko-bot
  2026-09-04 23:20 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Alexandra Winter @ 2026-09-02 14:34 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
	Andrew Lunn, Julian Ruess
  Cc: netdev, linux-s390, linux-kernel, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	Simon Horman

In case dibs_loopback_init() fails, e.g. because of -ENOMEM, dibs_init()
must unregister dibs_class. Otherwise dibs_class and /sys/class/dibs exist
even though the functionality is not available. A retry to load the module
fails with -EEXIST.

Unregister dibs_class in the error path of dibs_init.

Note that before
commit ad3dfa80be76 ("dibs: change dibs_class to a const struct")
class_destroy(dibs_class) is required instead of
class_unregister(&dibs_class).

Fixes: 804737349813 ("dibs: Create class dibs")
Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>
---
 drivers/dibs/dibs_main.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/dibs/dibs_main.c b/drivers/dibs/dibs_main.c
index 2b53a9d277dc..20c50997a7cf 100644
--- a/drivers/dibs/dibs_main.c
+++ b/drivers/dibs/dibs_main.c
@@ -251,13 +251,19 @@ static int __init dibs_init(void)
 
 	rc = class_register(&dibs_class);
 	if (rc)
-		return rc;
+		goto err;
 
 	rc = dibs_loopback_init();
 	if (rc)
-		pr_err("%s fails with %d\n", __func__, rc);
+		goto err_unregister;
 
 	return rc;
+
+err_unregister:
+	class_unregister(&dibs_class);
+err:
+	pr_err("%s fails with %d\n", __func__, rc);
+	return rc;
 }
 
 static void __exit dibs_exit(void)
-- 
2.53.0


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

* Re: [PATCH net] dibs: Unregister dibs_class after error
  2026-09-02 14:34 [PATCH net] dibs: Unregister dibs_class after error Alexandra Winter
@ 2026-09-03 14:35 ` sashiko-bot
  2026-09-04 23:20 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-03 14:35 UTC (permalink / raw)
  To: Alexandra Winter
  Cc: Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
	Heiko Carstens, linux-s390

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] dibs_unregister_client() can return -EBUSY after partially unregistering a client from some devices, leaving the client in an inconsistent state.
--

commit fa56c806dec36c4e7b30debb7ef4f13581270392
Author: Alexandra Winter <wintera@linux.ibm.com>

dibs: Unregister dibs_class after error

This commit ensures dibs_class is properly unregistered during dibs_init() if
dibs_loopback_init() fails. Previously, a failure would leave the class
registered, causing subsequent module load attempts to fail with -EEXIST.

> diff --git a/drivers/dibs/dibs_main.c b/drivers/dibs/dibs_main.c
> index 2b53a9d277dca..20c50997a7cf2 100644
> --- a/drivers/dibs/dibs_main.c
> +++ b/drivers/dibs/dibs_main.c
> @@ -251,13 +251,19 @@ static int __init dibs_init(void)
>  
>  	rc = class_register(&dibs_class);
>  	if (rc)
> -		return rc;
> +		goto err;
>  
>  	rc = dibs_loopback_init();
>  	if (rc)
> -		pr_err("%s fails with %d\n", __func__, rc);
> +		goto err_unregister;
>  
>  	return rc;
> +
> +err_unregister:
> +	class_unregister(&dibs_class);
> +err:
> +	pr_err("%s fails with %d\n", __func__, rc);
> +	return rc;
>  }

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in
dibs_unregister_client() in this same file:

drivers/dibs/dibs_main.c:dibs_unregister_client() {
...
	list_for_each_entry(dibs, &dibs_dev_list.list, list) {
		spin_lock_irqsave(&dibs->lock, flags);
		max_dmbs = dibs->ops->max_dmbs();
		for (int i = 0; i < max_dmbs; ++i) {
			if (dibs->dmb_clientid_arr[i] == client->id) {
				WARN(1, "%s: attempt to unregister '%s' with registered dmb(s)\n",
				     __func__, client->name);
				rc = -EBUSY;
				goto err_reg_dmb;
			}
		}
		/* Stop forwarding IRQs and events */
		dibs->subs[client->id] = NULL;
		spin_unlock_irqrestore(&dibs->lock, flags);
		clients[client->id]->ops->del_dev(dibs);
		dibs->priv[client->id] = NULL;
	}
...
}

If dibs_unregister_client() successfully unregisters the client from the first
device in dibs_dev_list, but then encounters a device where the client still has
registered DMBs, it returns -EBUSY and aborts.

Since the function clears dibs->subs, calls del_dev(), and clears dibs->priv
for previous devices, does returning -EBUSY without rolling back those changes
leave the client in an inconsistent state?

If the caller retries dibs_unregister_client() later, could this cause
del_dev() to be called a second time on the already processed devices,
potentially leading to a use-after-free or double-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902143438.426664-1-wintera@linux.ibm.com?part=1

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

* Re: [PATCH net] dibs: Unregister dibs_class after error
  2026-09-02 14:34 [PATCH net] dibs: Unregister dibs_class after error Alexandra Winter
  2026-09-03 14:35 ` sashiko-bot
@ 2026-09-04 23:20 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-04 23:20 UTC (permalink / raw)
  To: Alexandra Winter
  Cc: davem, kuba, pabeni, edumazet, andrew+netdev, julianr, netdev,
	linux-s390, linux-kernel, hca, gor, agordeev, borntraeger, svens,
	horms

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  2 Sep 2026 16:34:38 +0200 you wrote:
> In case dibs_loopback_init() fails, e.g. because of -ENOMEM, dibs_init()
> must unregister dibs_class. Otherwise dibs_class and /sys/class/dibs exist
> even though the functionality is not available. A retry to load the module
> fails with -EEXIST.
> 
> Unregister dibs_class in the error path of dibs_init.
> 
> [...]

Here is the summary with links:
  - [net] dibs: Unregister dibs_class after error
    https://git.kernel.org/netdev/net/c/1668a31e3b1a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2026-09-04 23:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 14:34 [PATCH net] dibs: Unregister dibs_class after error Alexandra Winter
2026-09-03 14:35 ` sashiko-bot
2026-09-04 23:20 ` patchwork-bot+netdevbpf

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