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


Groups > linux.kernel > #1235794 > unrolled thread

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

Started byRonit Halder <ronit.crj@gmail.com>
First post2015-09-30 08:20 +0200
Last post2015-10-01 17:20 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] staging: dgap: fix memory leak in dgap_parsefile() Ronit Halder <ronit.crj@gmail.com> - 2015-09-30 08:20 +0200
    Re: [PATCH] staging: dgap: fix memory leak in dgap_parsefile() Dan Carpenter <dan.carpenter@oracle.com> - 2015-09-30 13:50 +0200
      Re: [PATCH] staging: dgap: fix memory leak in dgap_parsefile() Ronit Halder <ronit.crj@gmail.com> - 2015-10-01 05:50 +0200
        Re: [PATCH] staging: dgap: fix memory leak in dgap_parsefile() Dan Carpenter <dan.carpenter@oracle.com> - 2015-10-01 17:10 +0200
          Re: [PATCH] staging: dgap: fix memory leak in dgap_parsefile() Ronit Halder <ronit.crj@gmail.com> - 2015-10-01 17:20 +0200

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

FromRonit Halder <ronit.crj@gmail.com>
Date2015-09-30 08:20 +0200
Subject[PATCH] staging: dgap: fix memory leak in dgap_parsefile()
Message-ID<qeiUa-53l-15@gated-at.bofh.it>
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 <ronit.crj@gmail.com>
---
 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, &p->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, &p->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, &p->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, &p->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

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

[toc] | [next] | [standalone]


#1236206

FromDan Carpenter <dan.carpenter@oracle.com>
Date2015-09-30 13:50 +0200
Message-ID<qeo3w-3UA-15@gated-at.bofh.it>
In reply to#1235794
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

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

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


#1236924

FromRonit Halder <ronit.crj@gmail.com>
Date2015-10-01 05:50 +0200
Message-ID<qeD2x-h5-5@gated-at.bofh.it>
In reply to#1236206
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1237487

FromDan Carpenter <dan.carpenter@oracle.com>
Date2015-10-01 17:10 +0200
Message-ID<qeNEB-8aM-5@gated-at.bofh.it>
In reply to#1236924
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

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

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


#1237494

FromRonit Halder <ronit.crj@gmail.com>
Date2015-10-01 17:20 +0200
Message-ID<qeNOi-8mD-5@gated-at.bofh.it>
In reply to#1237487
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web