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

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

[Daeseok Youn]

The p->u.board.status is allocated and set a string as
"No" once within allocating a node of BNODE type.
But it also set again with kstrdup() in case of "STATUS"
or "ID". If it is not allocated yet, use kstrdup().
If not, use just memcpy().

Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
 drivers/staging/dgap/dgap.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index a207bd7..cedf4b3 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -6598,7 +6598,11 @@ static int dgap_parsefile(char **in)
 				pr_err("dgap: parse: unexpected end of file\n");
 				return -1;
 			}
-			p->u.board.status = kstrdup(s, GFP_KERNEL);
+
+			if (p->u.board.status)
+				memcpy(p->u.board.status, s, strlen(s) + 1);
+			else
+				p->u.board.status = kstrdup(s, GFP_KERNEL);
 			break;
 
 		case NPORTS:	/* number of ports */
@@ -6648,7 +6652,10 @@ static int dgap_parsefile(char **in)
 				return -1;
 			}
 
-			p->u.board.status = kstrdup(s, GFP_KERNEL);
+			if (p->u.board.status)
+				memcpy(p->u.board.status, s, strlen(s) + 1);
+			else
+				p->u.board.status = kstrdup(s, GFP_KERNEL);
 
 			if (p->type == CNODE) {
 				p->u.conc.id = kstrdup(s, GFP_KERNEL);
-- 
1.7.1

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

[Dan Carpenter]

On Tue, Jul 15, 2014 at 12:05:14PM +0900, Daeseok Youn wrote:
> The p->u.board.status is allocated and set a string as
> "No" once within allocating a node of BNODE type.
> But it also set again with kstrdup() in case of "STATUS"
> or "ID". If it is not allocated yet, use kstrdup().
> If not, use just memcpy().

I don't think a 2 char buffer is always large enough to hold the new
strings.

Just free it and allocate again.

regards,
dan carpenter

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

[DaeSeok Youn]

2014-07-15 15:51 GMT+09:00 Dan Carpenter <dan.carpenter@oracle.com>:
> On Tue, Jul 15, 2014 at 12:05:14PM +0900, Daeseok Youn wrote:
>> The p->u.board.status is allocated and set a string as
>> "No" once within allocating a node of BNODE type.
>> But it also set again with kstrdup() in case of "STATUS"
>> or "ID". If it is not allocated yet, use kstrdup().
>> If not, use just memcpy().
>
> I don't think a 2 char buffer is always large enough to hold the new
> strings.
>
> Just free it and allocate again.
Yes, I will send this again.

Thanks.

regards,
Daeseok Youn.
>
> regards,
> dan carpenter
>

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

[Dan Carpenter]

On Tue, Jul 15, 2014 at 06:05:35PM +0900, DaeSeok Youn wrote:
> 2014-07-15 15:51 GMT+09:00 Dan Carpenter <dan.carpenter@oracle.com>:
> > On Tue, Jul 15, 2014 at 12:05:14PM +0900, Daeseok Youn wrote:
> >> The p->u.board.status is allocated and set a string as
> >> "No" once within allocating a node of BNODE type.
> >> But it also set again with kstrdup() in case of "STATUS"
> >> or "ID". If it is not allocated yet, use kstrdup().
> >> If not, use just memcpy().
> >
> > I don't think a 2 char buffer is always large enough to hold the new
> > strings.
> >
> > Just free it and allocate again.
> Yes, I will send this again.
> 

Actually, please just send the whole set again.  I really want Mark on
the CC list so he's reviewing these.

regards,
dan carpenter

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

[DaeSeok Youn]

2014-07-15 18:21 GMT+09:00 Dan Carpenter <dan.carpenter@oracle.com>:
> On Tue, Jul 15, 2014 at 06:05:35PM +0900, DaeSeok Youn wrote:
>> 2014-07-15 15:51 GMT+09:00 Dan Carpenter <dan.carpenter@oracle.com>:
>> > On Tue, Jul 15, 2014 at 12:05:14PM +0900, Daeseok Youn wrote:
>> >> The p->u.board.status is allocated and set a string as
>> >> "No" once within allocating a node of BNODE type.
>> >> But it also set again with kstrdup() in case of "STATUS"
>> >> or "ID". If it is not allocated yet, use kstrdup().
>> >> If not, use just memcpy().
>> >
>> > I don't think a 2 char buffer is always large enough to hold the new
>> > strings.
>> >
>> > Just free it and allocate again.
>> Yes, I will send this again.
>>
>
> Actually, please just send the whole set again.  I really want Mark on
> the CC list so he's reviewing these.
OK. I already sent 6/8, 7/8 and 8/8 again. I just resend rest of them.

thanks.

regards,
Daeseok Youn
>
> regards,
> dan carpenter
>