Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1535059 > unrolled thread

[PATCH] ppdev: don't print a free'd string

Back to article view | Back to linux.kernel

Contents

  [PATCH] ppdev: don't print a free'd string Colin King <colin.king@canonical.com> - 2016-12-02 17:30 +0100
    Re: [PATCH] ppdev: don't print a free'd string Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-12-06 12:00 +0100
      Re: [PATCH] ppdev: don't print a free'd string Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-12-08 23:20 +0100

#1535059 — [PATCH] ppdev: don't print a free'd string

From: Colin Ian King <colin.king@canonical.com>

A previous fix of a memory leak now prints the string 'name'
that was previously free'd.  Fix this by free'ing the string
at the end of the function and adding an error exit path for
the error conditions.

CoverityScan CID#1384523 ("Use after free")

Fixes: 2bd362d5f45c1 ("ppdev: fix memory leak")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/char/ppdev.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index 02819e0..4cf6d68 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -290,6 +290,7 @@ static int register_device(int minor, struct pp_struct *pp)
 	struct pardevice *pdev = NULL;
 	char *name;
 	struct pardev_cb ppdev_cb;
+	int rc = 0;
 
 	name = kasprintf(GFP_KERNEL, CHRDEV "%x", minor);
 	if (name == NULL)
@@ -298,8 +299,8 @@ static int register_device(int minor, struct pp_struct *pp)
 	port = parport_find_number(minor);
 	if (!port) {
 		pr_warn("%s: no associated port!\n", name);
-		kfree(name);
-		return -ENXIO;
+		rc = -ENXIO;
+		goto err;
 	}
 
 	memset(&ppdev_cb, 0, sizeof(ppdev_cb));
@@ -308,16 +309,18 @@ static int register_device(int minor, struct pp_struct *pp)
 	ppdev_cb.private = pp;
 	pdev = parport_register_dev_model(port, name, &ppdev_cb, minor);
 	parport_put_port(port);
-	kfree(name);
 
 	if (!pdev) {
 		pr_warn("%s: failed to register device!\n", name);
-		return -ENXIO;
+		rc = -ENXIO;
+		goto err;
 	}
 
 	pp->pdev = pdev;
 	dev_dbg(&pdev->dev, "registered pardevice\n");
-	return 0;
+err:
+	kfree(name);
+	return rc;
 }
 
 static enum ieee1284_phase init_phase(int mode)
-- 
2.10.2

[toc] | [next] | [standalone]

#1536853

On Fri, Dec 02, 2016 at 04:23:55PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> A previous fix of a memory leak now prints the string 'name'
> that was previously free'd.  Fix this by free'ing the string
> at the end of the function and adding an error exit path for
> the error conditions.
> 
> CoverityScan CID#1384523 ("Use after free")
> 
> Fixes: 2bd362d5f45c1 ("ppdev: fix memory leak")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/char/ppdev.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)

Sudip, any thoughts?

[toc] | [prev] | [next] | [standalone]

#1538917

On Tuesday 06 December 2016 10:51 AM, Greg Kroah-Hartman wrote:
> On Fri, Dec 02, 2016 at 04:23:55PM +0000, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> A previous fix of a memory leak now prints the string 'name'
>> that was previously free'd.  Fix this by free'ing the string
>> at the end of the function and adding an error exit path for
>> the error conditions.
>>
>> CoverityScan CID#1384523 ("Use after free")
>>
>> Fixes: 2bd362d5f45c1 ("ppdev: fix memory leak")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>   drivers/char/ppdev.c | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
>
> Sudip, any thoughts?
>

Hi Greg,

Sorry for the late reply. It looks good.

Acked-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>


Regards
Sudip

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web