Re: [PATCH] staging: dgap: fix memory leak in dgap_parsefile()

2015-10-01 Thread Ronit Halder 
> Why not just reject it earlier instead of allocating more data?
I agree on your point.
But we have to make different mechanism for that and my patch is only
to fix the current code.

regards
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: dgap: fix memory leak in dgap_parsefile()

2015-10-01 Thread Dan Carpenter 
On Thu, Oct 01, 2015 at 09:11:31AM +0530, Ronit Halder wrote:
> The existing dgap_parsefile() rejects invalid config file.
> But before we know that config file is invalid a lot of memory leak
> can happen. Removing the chances of memory leak won't heart anyone.
> 

Why not just reject it earlier instead of allocating more data?

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: dgap: fix memory leak in dgap_parsefile()

2015-09-30 Thread Ronit Halder 
In dgap_parsefile() char pointers are set with kstrdup()
without checking that some string is allocated to that
char pointer before. This patch frees the memory if already allocated
and then set the poniter with kstrdup().

Signed-off-by: Ronit halder 
---
 drivers/staging/dgap/dgap.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index e17bde7..64f6149 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -672,6 +672,7 @@ static int dgap_parsefile(char **in)
pr_err("unexpected end of file");
return -1;
}
+   kfree(p->u.board.portstr);
p->u.board.portstr = kstrdup(s, GFP_KERNEL);
if (kstrtol(s, 0, >u.board.port)) {
pr_err("bad number for IO port");
@@ -690,6 +691,7 @@ static int dgap_parsefile(char **in)
pr_err("unexpected end of file");
return -1;
}
+   kfree(p->u.board.addrstr);
p->u.board.addrstr = kstrdup(s, GFP_KERNEL);
if (kstrtoul(s, 0, >u.board.addr)) {
pr_err("bad number for memory address");
@@ -708,6 +710,7 @@ static int dgap_parsefile(char **in)
pr_err("unexpected end of file");
return -1;
}
+   kfree(p->u.board.pcibusstr);
p->u.board.pcibusstr = kstrdup(s, GFP_KERNEL);
if (kstrtoul(s, 0, >u.board.pcibus)) {
pr_err("bad number for pci bus");
@@ -719,6 +722,7 @@ static int dgap_parsefile(char **in)
pr_err("unexpected end of file");
return -1;
}
+   kfree(p->u.board.pcislotstr);
p->u.board.pcislotstr = kstrdup(s, GFP_KERNEL);
if (kstrtoul(s, 0, >u.board.pcislot)) {
pr_err("bad number for pci slot");
@@ -737,6 +741,7 @@ static int dgap_parsefile(char **in)
pr_err("unexpected end of file");
return -1;
}
+   kfree(p->u.board.method);
p->u.board.method = kstrdup(s, GFP_KERNEL);
p->u.board.v_method = 1;
break;
@@ -751,6 +756,7 @@ static int dgap_parsefile(char **in)
pr_err("unexpected end of file");
return -1;
}
+   kfree(p->u.board.status);
p->u.board.status = kstrdup(s, GFP_KERNEL);
break;
 
@@ -800,13 +806,15 @@ static int dgap_parsefile(char **in)
pr_err("unexpected end of file");
return -1;
}
-
+   kfree(p->u.board.status);
p->u.board.status = kstrdup(s, GFP_KERNEL);
 
if (p->type == CNODE) {
+   kfree(p->u.conc.id);
p->u.conc.id = kstrdup(s, GFP_KERNEL);
p->u.conc.v_id = 1;
} else if (p->type == MNODE) {
+   kfree(p->u.module.id);
p->u.module.id = kstrdup(s, GFP_KERNEL);
p->u.module.v_id = 1;
} else {
@@ -1003,6 +1011,7 @@ static int dgap_parsefile(char **in)
pr_err("unexpected end of file");
return -1;
}
+   kfree(p->u.line.cable);
p->u.line.cable = kstrdup(s, GFP_KERNEL);
p->u.line.v_cable = 1;
}
@@ -1044,6 +1053,7 @@ static int dgap_parsefile(char **in)
pr_err("unexpected end of file");
return -1;
}
+   kfree(p->u.conc.connect);
p->u.conc.connect = kstrdup(s, GFP_KERNEL);
p->u.conc.v_connect = 1;
}
-- 
2.4.0.GIT

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: dgap: fix memory leak in dgap_parsefile()

2015-09-30 Thread Dan Carpenter 
On Wed, Sep 30, 2015 at 11:39:45AM +0530, Ronit Halder wrote:
> In dgap_parsefile() char pointers are set with kstrdup()
> without checking that some string is allocated to that
> char pointer before.

Why would this happen?  Wouldn't it be better to reject the invalid
config file?

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: dgap: fix memory leak in dgap_parsefile()

2015-09-30 Thread Ronit Halder 
The existing dgap_parsefile() rejects invalid config file.
But before we know that config file is invalid a lot of memory leak
can happen. Removing the chances of memory leak won't heart anyone.

regards,
Ronit Halder
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel