Skip to contentSkip to conversation

dibs: Unregister dibs_class after error

PATCH Applied Alexandra Winter 3 participants 3 messages Sep 2 – Sep 4

Subsystems (4)
Networking Network drivers s390 Dibs (direct internal buffer sharing)
lore
Share & tools
lore

Alexandra Winter

author maintainer
#0 lore raw

PATCH 1/1 dibs: Unregister dibs_class after error

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 <[email protected]>
---
 drivers/dibs/dibs_main.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/dibs/dibs_main.c +8 -2
index 2b53a9d277dc..20c50997a7cf 100644
@@ -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)
Signature · 1 line
2.53.0
AI review
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

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 <[email protected]>

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.

Quoted 14 lines
drivers/dibs/dibs_main.c +1 -2
index 2b53a9d277dca..20c50997a7cf2 100644
@@ -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);

drivers/dibs/dibs_main.c static int __init dibs_init(void) · line 267

Quoted 10 lines
drivers/dibs/dibs_main.c +7 -0
+		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?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
CI Applied
#2 lore raw
In reply to Alexandra Winter
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 8 lines · 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!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html