mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrew Jeffery <andrew@codeconstruct.com.au>
To: minyard@acm.org, openipmi-developer@lists.sourceforge.net
Cc: Andrew Jeffery <andrew@codeconstruct.com.au>,
	linux-kernel@vger.kernel.org, Jonathan.Cameron@Huawei.com,
	aladyshev22@gmail.com, jk@codeconstruct.com.au
Subject: [PATCH 07/10] ipmi: kcs_bmc: Disassociate client from device lifetimes
Date: Fri,  3 Nov 2023 16:45:19 +1030	[thread overview]
Message-ID: <20231103061522.1268637-8-andrew@codeconstruct.com.au> (raw)
In-Reply-To: <20231103061522.1268637-1-andrew@codeconstruct.com.au>

KCS client modules may be removed by actions unrelated to KCS devices.
As usual, removing a KCS client module requires unbinding all client
instances from the underlying devices to prevent further use of the code.

Previously, KCS client resource lifetimes were tied to the underlying
KCS device instance with the use of `devm_k[mz]alloc()` APIs. This
requires the use of `devm_free()` as a consequence. It's necessary to
scrutinise use of explicit `devm_free()`s because they generally
indicate there's a concerning corner-case in play, but that's not really
the case here. Switch to explicit kmalloc()/kfree() to align
expectations with the intent of the code.

Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
---
 drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 28 ++++++++++++++++++---------
 drivers/char/ipmi/kcs_bmc_serio.c     | 20 ++++++++++++-------
 2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
index 45ac930172ec..98f231f24c26 100644
--- a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
+++ b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
@@ -470,7 +470,7 @@ static int kcs_bmc_ipmi_add_device(struct kcs_bmc_device *kcs_bmc)
 	struct kcs_bmc_ipmi *priv;
 	int rc;
 
-	priv = devm_kzalloc(kcs_bmc->dev, sizeof(*priv), GFP_KERNEL);
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
@@ -482,26 +482,35 @@ static int kcs_bmc_ipmi_add_device(struct kcs_bmc_device *kcs_bmc)
 	priv->client.ops = &kcs_bmc_ipmi_client_ops;
 
 	priv->miscdev.minor = MISC_DYNAMIC_MINOR;
-	priv->miscdev.name = devm_kasprintf(kcs_bmc->dev, GFP_KERNEL, "%s%u", DEVICE_NAME,
-					   kcs_bmc->channel);
-	if (!priv->miscdev.name)
-		return -EINVAL;
+	priv->miscdev.name = kasprintf(GFP_KERNEL, "%s%u", DEVICE_NAME, kcs_bmc->channel);
+	if (!priv->miscdev.name) {
+		rc = -ENOMEM;
+		goto cleanup_priv;
+	}
 
 	priv->miscdev.fops = &kcs_bmc_ipmi_fops;
 
 	rc = misc_register(&priv->miscdev);
 	if (rc) {
-		dev_err(kcs_bmc->dev, "Unable to register device: %d\n", rc);
-		return rc;
+		pr_err("Unable to register device: %d\n", rc);
+		goto cleanup_miscdev_name;
 	}
 
 	spin_lock_irq(&kcs_bmc_ipmi_instances_lock);
 	list_add(&priv->entry, &kcs_bmc_ipmi_instances);
 	spin_unlock_irq(&kcs_bmc_ipmi_instances_lock);
 
-	dev_info(kcs_bmc->dev, "Initialised IPMI client for channel %d", kcs_bmc->channel);
+	pr_info("Initialised IPMI client for channel %d\n", kcs_bmc->channel);
 
 	return 0;
+
+cleanup_miscdev_name:
+	kfree(priv->miscdev.name);
+
+cleanup_priv:
+	kfree(priv);
+
+	return rc;
 }
 
 static void kcs_bmc_ipmi_remove_device(struct kcs_bmc_device *kcs_bmc)
@@ -523,7 +532,8 @@ static void kcs_bmc_ipmi_remove_device(struct kcs_bmc_device *kcs_bmc)
 
 	misc_deregister(&priv->miscdev);
 	kcs_bmc_disable_device(&priv->client);
-	devm_kfree(kcs_bmc->dev, priv);
+	kfree(priv->miscdev.name);
+	kfree(priv);
 }
 
 static const struct kcs_bmc_driver_ops kcs_bmc_ipmi_driver_ops = {
diff --git a/drivers/char/ipmi/kcs_bmc_serio.c b/drivers/char/ipmi/kcs_bmc_serio.c
index 713e847bbc4e..0a68c76da955 100644
--- a/drivers/char/ipmi/kcs_bmc_serio.c
+++ b/drivers/char/ipmi/kcs_bmc_serio.c
@@ -71,15 +71,18 @@ static int kcs_bmc_serio_add_device(struct kcs_bmc_device *kcs_bmc)
 {
 	struct kcs_bmc_serio *priv;
 	struct serio *port;
+	int rc;
 
-	priv = devm_kzalloc(kcs_bmc->dev, sizeof(*priv), GFP_KERNEL);
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
 	/* Use kzalloc() as the allocation is cleaned up with kfree() via serio_unregister_port() */
 	port = kzalloc(sizeof(*port), GFP_KERNEL);
-	if (!port)
-		return -ENOMEM;
+	if (!port) {
+		rc = -ENOMEM;
+		goto cleanup_priv;
+	}
 
 	port->id.type = SERIO_8042;
 	port->open = kcs_bmc_serio_open;
@@ -98,9 +101,14 @@ static int kcs_bmc_serio_add_device(struct kcs_bmc_device *kcs_bmc)
 
 	serio_register_port(port);
 
-	dev_info(kcs_bmc->dev, "Initialised serio client for channel %d", kcs_bmc->channel);
+	pr_info("Initialised serio client for channel %d\n", kcs_bmc->channel);
 
 	return 0;
+
+cleanup_priv:
+	kfree(priv);
+
+	return rc;
 }
 
 static void kcs_bmc_serio_remove_device(struct kcs_bmc_device *kcs_bmc)
@@ -122,11 +130,9 @@ static void kcs_bmc_serio_remove_device(struct kcs_bmc_device *kcs_bmc)
 
 	/* kfree()s priv->port via put_device() */
 	serio_unregister_port(priv->port);
-
 	/* Ensure the IBF IRQ is disabled if we were the active client */
 	kcs_bmc_disable_device(&priv->client);
-
-	devm_kfree(priv->client.dev->dev, priv);
+	kfree(priv);
 }
 
 static const struct kcs_bmc_driver_ops kcs_bmc_serio_driver_ops = {
-- 
2.39.2


  parent reply	other threads:[~2023-11-03  6:16 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-03  6:15 [PATCH 00/10] ipmi: kcs_bmc: Miscellaneous cleanups Andrew Jeffery
2023-11-03  6:15 ` [PATCH 01/10] ipmi: kcs_bmc: Update module description Andrew Jeffery
2023-11-03 14:34   ` Jonathan Cameron
2023-11-03  6:15 ` [PATCH 02/10] ipmi: kcs_bmc: Include spinlock.h Andrew Jeffery
2023-11-03 14:36   ` Jonathan Cameron
2023-11-05 22:47     ` Andrew Jeffery
2023-11-03  6:15 ` [PATCH 03/10] ipmi: kcs_bmc: Make kcs_bmc_update_event_mask() static Andrew Jeffery
2023-11-03 14:40   ` Jonathan Cameron
2023-11-05 22:52     ` Andrew Jeffery
2023-11-03  6:15 ` [PATCH 04/10] ipmi: kcs_bmc: Make remove_device() callback return void Andrew Jeffery
2023-11-03 14:43   ` Jonathan Cameron
2023-11-05 22:54     ` Andrew Jeffery
2023-11-03  6:15 ` [PATCH 05/10] ipmi: kcs_bmc: Define client actions in terms of kcs_bmc_client Andrew Jeffery
2023-11-03 15:16   ` Jonathan Cameron
2023-11-07  6:32     ` Andrew Jeffery
2023-11-03  6:15 ` [PATCH 06/10] ipmi: kcs_bmc: Integrate buffers into driver struct Andrew Jeffery
2023-11-03 14:45   ` Jonathan Cameron
2023-11-05 22:55     ` Andrew Jeffery
2023-11-03  6:15 ` Andrew Jeffery [this message]
2023-11-03 14:51   ` [PATCH 07/10] ipmi: kcs_bmc: Disassociate client from device lifetimes Jonathan Cameron
2023-11-05 23:01     ` Andrew Jeffery
2023-11-03  6:15 ` [PATCH 08/10] ipmi: kcs_bmc: Track clients in core Andrew Jeffery
2023-11-03 15:05   ` Jonathan Cameron
2023-11-05 23:56     ` Andrew Jeffery
2023-11-20 12:40       ` Jonathan Cameron
2023-11-27  3:02         ` Andrew Jeffery
2023-11-03  6:15 ` [PATCH 09/10] ipmi: kcs_bmc: Add module_kcs_bmc_driver() Andrew Jeffery
2023-11-03 15:06   ` Jonathan Cameron
2023-11-03  6:15 ` [PATCH 10/10] ipmi: kcs_bmc: Add subsystem kerneldoc Andrew Jeffery
2023-11-03 15:12   ` Jonathan Cameron
2023-11-06  0:05     ` Andrew Jeffery

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231103061522.1268637-8-andrew@codeconstruct.com.au \
    --to=andrew@codeconstruct.com.au \
    --cc=Jonathan.Cameron@Huawei.com \
    --cc=aladyshev22@gmail.com \
    --cc=jk@codeconstruct.com.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=minyard@acm.org \
    --cc=openipmi-developer@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome