[PATCH 1/1] clocksource: sh_mtu2: Fix irq leaks when sh_mtu2_setup() fails

[Takeshi Yoshimura]

My static checker detected irq leaks in sh_mtu2_setup(). The problem
happens when the first request_irq() succeeds but the later ones fail.
This situation also results that the clockevent for the first channel
is registered although the driver fails. So I introduce
sh_mtu2_setup_channels() to ensure the consistency of all the clockevent
and irqs even if the error occurs.

Signed-off-by: Takeshi Yoshimura <yos@xxxxxxxxxxxxxxxxxxxx>
---
 drivers/clocksource/sh_mtu2.c | 65 ++++++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 26 deletions(-)

diff --git a/drivers/clocksource/sh_mtu2.c b/drivers/clocksource/sh_mtu2.c
index 3d88698..5ddc433 100644
--- a/drivers/clocksource/sh_mtu2.c
+++ b/drivers/clocksource/sh_mtu2.c
@@ -344,38 +344,54 @@ static int sh_mtu2_register(struct sh_mtu2_channel *ch, const char *name)
 	return 0;
 }
 
-static int sh_mtu2_setup_channel(struct sh_mtu2_channel *ch, unsigned int index,
-				 struct sh_mtu2_device *mtu)
+static int sh_mtu2_setup_channels(struct sh_mtu2_device *mtu)
 {
 	static const unsigned int channel_offsets[] = {
 		0x300, 0x380, 0x000,
 	};
 	char name[6];
-	int irq;
+	int irq[3];
 	int ret;
+	int i;
 
-	ch->mtu = mtu;
-
-	sprintf(name, "tgi%ua", index);
-	irq = platform_get_irq_byname(mtu->pdev, name);
-	if (irq < 0) {
-		/* Skip channels with no declared interrupt. */
-		return 0;
+	for (i = 0; i < mtu->num_channels; ++i) {
+		struct sh_mtu2_channel *ch = &mtu->channels[i];
+
+		ch->mtu = mtu;
+		sprintf(name, "tgi%ua", i);
+		irq[i] = platform_get_irq_byname(mtu->pdev, name);
+		if (irq[i] < 0) {
+			/* Skip channels with no declared interrupt. */
+			continue;
+		}
+
+		ret = request_irq(irq[i], sh_mtu2_interrupt,
+				  IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
+				  dev_name(&ch->mtu->pdev->dev), ch);
+		if (ret) {
+			dev_err(&ch->mtu->pdev->dev, "ch%u: failed to request irq %d\n",
+				i, irq[i]);
+			goto err;
+		}
 	}
 
-	ret = request_irq(irq, sh_mtu2_interrupt,
-			  IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
-			  dev_name(&ch->mtu->pdev->dev), ch);
-	if (ret) {
-		dev_err(&ch->mtu->pdev->dev, "ch%u: failed to request irq %d\n",
-			index, irq);
-		return ret;
+	for (i = 0; i < mtu->num_channels; ++i) {
+		struct sh_mtu2_channel *ch = &mtu->channels[i];
+
+		ch->base = mtu->mapbase + channel_offsets[i];
+		ch->index = i;
+		sh_mtu2_register(ch, dev_name(&mtu->pdev->dev));
 	}
 
-	ch->base = mtu->mapbase + channel_offsets[index];
-	ch->index = index;
+	return 0;
 
-	return sh_mtu2_register(ch, dev_name(&mtu->pdev->dev));
+err:
+	--i;
+	for (; i >= 0; --i) {
+		if (irq[i] >= 0)
+			free_irq(irq[i], &mtu->channels[i]);
+	}
+	return ret;
 }
 
 static int sh_mtu2_map_memory(struct sh_mtu2_device *mtu)
@@ -398,7 +414,6 @@ static int sh_mtu2_map_memory(struct sh_mtu2_device *mtu)
 static int sh_mtu2_setup(struct sh_mtu2_device *mtu,
 			 struct platform_device *pdev)
 {
-	unsigned int i;
 	int ret;
 
 	mtu->pdev = pdev;
@@ -433,11 +448,9 @@ static int sh_mtu2_setup(struct sh_mtu2_device *mtu,
 		goto err_unmap;
 	}
 
-	for (i = 0; i < mtu->num_channels; ++i) {
-		ret = sh_mtu2_setup_channel(&mtu->channels[i], i, mtu);
-		if (ret < 0)
-			goto err_unmap;
-	}
+	ret = sh_mtu2_setup_channels(mtu);
+	if (ret < 0)
+		goto err_unmap;
 
 	platform_set_drvdata(pdev, mtu);
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/