Skip to contentSkip to conversation

Input: cyttsp4 - leak on error path in probe()

PATCH Applied Dan Carpenter 2 participants 5 messages Jul 2, 2013 – Jul 3, 2013

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 3/3 Input: cyttsp4 - leak on error path in probe()

We leak "cd" if the cd->xfer_buf allocation fails. It was weird to "goto error_gpio_irq" so I changed the label name. (Label names should reflect the label location not the goto location otherwise you get an "all roads lead to Rome problem").

drivers/input/touchscreen/cyttsp4_core.c +3 -3
index 851e3ff..a7987e1 100644
@@ -2025,7 +2025,7 @@ struct cyttsp4 *cyttsp4_probe(const struct cyttsp4_bus_ops *ops,
 	if (!cd->xfer_buf) {
 		dev_err(dev, "%s: Error, kzalloc\n", __func__);
 		rc = -ENOMEM;
-		goto error_alloc_data;
+		goto error_free_cd;
 	}
 
 	/* Initialize device info */
@@ -2049,7 +2049,7 @@ struct cyttsp4 *cyttsp4_probe(const struct cyttsp4_bus_ops *ops,
 	cd->irq = gpio_to_irq(cd->cpdata->irq_gpio);
 	if (cd->irq < 0) {
 		rc = -EINVAL;
-		goto error_gpio_irq;
+		goto error_free_cd;
 	}
 
 	dev_set_drvdata(dev, cd);
@@ -2117,7 +2117,7 @@ error_request_irq:
 	if (cd->cpdata->init)
 		cd->cpdata->init(cd->cpdata, 0, dev);
 	dev_set_drvdata(dev, NULL);
-error_gpio_irq:
+error_free_cd:
 	kfree(cd);
 error_alloc_data:
 error_no_pdata:
#1 lore raw
Quoted 7 lines · Dan Carpenter

We leak "cd" if the cd->xfer_buf allocation fails. It was weird to "goto error_gpio_irq" so I changed the label name. (Label names should reflect the label location not the goto location otherwise you get an "all roads lead to Rome problem").

xfer_buf still may not be freed, I propose following one instead:

From: Ferruh Yigit <[email protected]>
Date: Wed, 3 Jul 2013 15:59:54 +0300
Subject: [PATCH] Input: cyttsp4 - free xfer_buf in error path

Label for xfer_buf error is wrong and causing mem leak for cd. Updated label for xfer_buf alloc failure and add kfree for xfer_buf

---
  drivers/input/touchscreen/cyttsp4_core.c |    4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
(quoted diff) +0 -0

b/drivers/input/touchscreen/cyttsp4_core.c
index 69c9cff..219e32d 100644

drivers/input/touchscreen/cyttsp4_core.c +0 -0
@@ -2034,7 +2034,7 @@ struct cyttsp4 *cyttsp4_probe(const struct

cyttsp4_bus_ops *ops,

      if (!cd->xfer_buf) {
          dev_err(dev, "%s: Error, kzalloc\n", __func__);
          rc = -ENOMEM;
-        goto error_alloc_data;
+        goto error_alloc_xfer;
      }

      /* Initialize device info */
@@ -2127,6 +2127,8 @@ error_request_irq:
          cd->cpdata->init(cd->cpdata, 0, dev);
      dev_set_drvdata(dev, NULL);
  error_gpio_irq:
+    kfree(cd->xfer_buf);
+error_alloc_xfer:
      kfree(cd);

error_alloc_data:
error_no_pdata:
--
1.7.9.5
This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

#2 lore raw
Quoted 10 lines · Ferruh Yigit
Quoted 7 lines · Dan Carpenter

We leak "cd" if the cd->xfer_buf allocation fails. It was weird to "goto error_gpio_irq" so I changed the label name. (Label names should reflect the label location not the goto location otherwise you get an "all roads lead to Rome problem").

xfer_buf still may not be freed, I propose following one instead:

That's a good point. I missed that leak. The problem is that my patch was already applied so this patch would have to be re-written on top of that. Also this patch is whitespace dammaged and won't apply.

regards,
dan carpenter

#3 lore raw
Quoted 7 lines · Dan Carpenter
Quoted 6 lines · Ferruh Yigit
Quoted 5 lines · Dan Carpenter

We leak "cd" if the cd->xfer_buf allocation fails. It was weird to "goto error_gpio_irq" so I changed the label name. (Label names should reflect the label location not the goto location otherwise you get an "all roads lead to Rome problem").

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

xfer_buf still may not be freed, I propose following one instead:

That's a good point. I missed that leak. The problem is that my patch was already applied so this patch would have to be re-written on top of that. Also this patch is whitespace dammaged and won't apply.

OK, I am preparing the patch over previous one.

Thanks,
ferruh
This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

#4 lore raw

PATCH 1/1 Input: cyttsp4 - kfree xfer_buf on error path in probe()

If probe() fails after cd->xfer_buf allocated, it will not freed. Added kfree(cd->xfer_buf) with and error label.

---
 drivers/input/touchscreen/cyttsp4_core.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/input/touchscreen/cyttsp4_core.c +3 -1
index a7987e1..edcf799 100644
@@ -2049,7 +2049,7 @@ struct cyttsp4 *cyttsp4_probe(const struct cyttsp4_bus_ops *ops,
 	cd->irq = gpio_to_irq(cd->cpdata->irq_gpio);
 	if (cd->irq < 0) {
 		rc = -EINVAL;
-		goto error_free_cd;
+		goto error_free_xfer;
 	}
 
 	dev_set_drvdata(dev, cd);
@@ -2117,6 +2117,8 @@ error_request_irq:
 	if (cd->cpdata->init)
 		cd->cpdata->init(cd->cpdata, 0, dev);
 	dev_set_drvdata(dev, NULL);
+error_free_xfer:
+	kfree(cd->xfer_buf);
 error_free_cd:
 	kfree(cd);
 error_alloc_data:
Signature · 1 line
1.7.9.5