2014-07-15 03:06:34

by Daeseok Youn

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

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 <[email protected]>
---
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


2014-07-15 06:51:26

by Dan Carpenter

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

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

2014-07-15 09:05:40

by Daeseok Youn

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

2014-07-15 15:51 GMT+09:00 Dan Carpenter <[email protected]>:
> 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
>

2014-07-15 09:21:34

by Dan Carpenter

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

On Tue, Jul 15, 2014 at 06:05:35PM +0900, DaeSeok Youn wrote:
> 2014-07-15 15:51 GMT+09:00 Dan Carpenter <[email protected]>:
> > 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

2014-07-15 09:45:20

by Daeseok Youn

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

2014-07-15 18:21 GMT+09:00 Dan Carpenter <[email protected]>:
> On Tue, Jul 15, 2014 at 06:05:35PM +0900, DaeSeok Youn wrote:
>> 2014-07-15 15:51 GMT+09:00 Dan Carpenter <[email protected]>:
>> > 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
>