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

Commit Message

Ferruh Yigit July 3, 2013, 1:12 p.m. UTC

On 07/03/2013 12:44 AM, Dan Carpenter wrote:
> 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").
>
> Signed-off-by: Dan Carpenter <[e-mail]>
>
xfer_buf still may not be freed, I propose following one instead:

From: Ferruh Yigit <[e-mail]>
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

Signed-off-by: Ferruh Yigit <[e-mail]>
---
  drivers/input/touchscreen/cyttsp4_core.c |    4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

      /* 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Dan Carpenter July 3, 2013, 1:50 p.m. UTC | #1

On Wed, Jul 03, 2013 at 04:12:48PM +0300, Ferruh Yigit wrote:
> On 07/03/2013 12:44 AM, Dan Carpenter wrote:
> >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").
> >
> >Signed-off-by: Dan Carpenter <[e-mail]>
> >
> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Ferruh Yigit July 3, 2013, 1:53 p.m. UTC | #2

On 07/03/2013 04:50 PM, Dan Carpenter wrote:
> On Wed, Jul 03, 2013 at 04:12:48PM +0300, Ferruh Yigit wrote:
>> On 07/03/2013 12:44 AM, Dan Carpenter wrote:
>>> 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").
>>>
>>> Signed-off-by: Dan Carpenter <[e-mail]>
>>>
>> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/input/touchscreen/cyttsp4_core.c
b/drivers/input/touchscreen/cyttsp4_core.c
index 69c9cff..219e32d 100644
--- a/drivers/input/touchscreen/cyttsp4_core.c
+++ b/drivers/input/touchscreen/cyttsp4_core.c
@@ -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;
      }