[PATCH] mailbox: omap: fix reference count leak in omap_mbox_probe()

[PATCH] mailbox: omap: fix reference count leak in omap_mbox_probe()

[Weigang He]

of_get_next_available_child() returns a device_node pointer with
refcount incremented. The caller is responsible for calling
of_node_put() to release the reference when done.

In omap_mbox_probe(), when the loop iterates over child nodes using
of_get_next_available_child(), several error paths return directly
without releasing the child node reference. Additionally, when the
loop completes normally, the last child node reference is never
released.

Fix this by:
1. Using goto-based error handling to ensure of_node_put(child) is
   called on all error paths within the loop
2. Adding of_node_put(child) after the loop completes normally to
   release the last child node reference

Fixes: 75288cc66dc4 ("mailbox/omap: add support for parsing dt devices")
Cc: stable@vger.kernel.org
Signed-off-by: Weigang He <geoffreyhe2@gmail.com>
---
 drivers/mailbox/omap-mailbox.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index 17fe6545875d0..3f6af947e29ce 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -508,14 +508,16 @@ static int omap_mbox_probe(struct platform_device *pdev)
 		int rx_id,         rx_usr;
 
 		mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
-		if (!mbox)
-			return -ENOMEM;
+		if (!mbox) {
+			ret = -ENOMEM;
+			goto err_put_child;
+		}
 
 		child = of_get_next_available_child(node, child);
 		ret = of_property_read_u32_array(child, "ti,mbox-tx", tmp,
 						 ARRAY_SIZE(tmp));
 		if (ret)
-			return ret;
+			goto err_put_child;
 		tx_id = tmp[0];
 		tx_irq = tmp[1];
 		tx_usr = tmp[2];
@@ -523,14 +525,16 @@ static int omap_mbox_probe(struct platform_device *pdev)
 		ret = of_property_read_u32_array(child, "ti,mbox-rx", tmp,
 						 ARRAY_SIZE(tmp));
 		if (ret)
-			return ret;
+			goto err_put_child;
 		rx_id = tmp[0];
 		/* rx_irq = tmp[1]; */
 		rx_usr = tmp[2];
 
 		if (tx_id >= num_fifos || rx_id >= num_fifos ||
-		    tx_usr >= num_users || rx_usr >= num_users)
-			return -EINVAL;
+		    tx_usr >= num_users || rx_usr >= num_users) {
+			ret = -EINVAL;
+			goto err_put_child;
+		}
 
 		fifo = &mbox->tx_fifo;
 		fifo->msg = MAILBOX_MESSAGE(tx_id);
@@ -554,11 +558,14 @@ static int omap_mbox_probe(struct platform_device *pdev)
 		mbox->parent = mdev;
 		mbox->name = child->name;
 		mbox->irq = platform_get_irq(pdev, tx_irq);
-		if (mbox->irq < 0)
-			return mbox->irq;
+		if (mbox->irq < 0) {
+			ret = mbox->irq;
+			goto err_put_child;
+		}
 		mbox->chan = &chnls[i];
 		chnls[i].con_priv = mbox;
 	}
+	of_node_put(child);
 
 	mutex_init(&mdev->cfg_lock);
 	mdev->dev = &pdev->dev;
@@ -602,6 +609,10 @@ static int omap_mbox_probe(struct platform_device *pdev)
 		return ret;
 
 	return 0;
+
+err_put_child:
+	of_node_put(child);
+	return ret;
 }
 
 static struct platform_driver omap_mbox_driver = {
-- 
2.34.1

Re: [PATCH] mailbox: omap: fix reference count leak in omap_mbox_probe()

[Markus Elfring]

…
> 1. Using goto-based error handling to ensure of_node_put(child) is
>    called on all error paths within the loop
> 2. Adding of_node_put(child) after the loop completes normally to
>    release the last child node reference

How do you think about to increase the application of scope-based resource management
with another patch?

Regards,
Markus