2008-07-29 11:50:11

by Darren Jenkins

Subject: [PATCH] fs/fat/dir.c fix resource leak

G'day people,

Coverity CID 2332 & 2333 RESOURCE_LEAK


In fat_search_long() if fat_parse_long() returns a -ve value we return
without first freeing unicode and bufname.
This patch free's them on this error path.

Patch against linux-next commit 99be8ca04aa13c0ff10bbb3ba5f90676b050fcc6
Only compile tested

Signed-off-by: Darren Jenkins <darrenrjenkins@gmailcom>

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 34541d0..8a9d7fa 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -346,14 +346,16 @@ parse_record:
if (de->attr == ATTR_EXT) {
int status = fat_parse_long(inode, &cpos, &bh, &de,
&unicode, &nr_slots);
- if (status < 0)
- return status;
- else if (status == PARSE_INVALID)
+ if (status < 0) {
+ err = status;
+ goto EODir;
+ } else if (status == PARSE_INVALID) {
continue;
- else if (status == PARSE_NOT_LONGNAME)
+ } else if (status == PARSE_NOT_LONGNAME) {
goto parse_record;
- else if (status == PARSE_EOF)
+ } else if (status == PARSE_EOF) {
goto EODir;
+ }
}

memcpy(work, de->name, sizeof(de->name));

2008-07-29 12:55:44

by OGAWA Hirofumi

Subject: Re: [PATCH] fs/fat/dir.c fix resource leak

"Darren Jenkins\" <[email protected]> writes:

> Coverity CID 2332 & 2333 RESOURCE_LEAK
>
> In fat_search_long() if fat_parse_long() returns a -ve value we return
> without first freeing unicode and bufname.
> This patch free's them on this error path.
>
> Signed-off-by: Darren Jenkins <darrenrjenkins@gmailcom>

I'll apply to current code. Thanks.

BTW, email address of Signed-off-by line is typo?
--
OGAWA Hirofumi <[email protected]>

2008-07-29 13:06:33

by OGAWA Hirofumi

Subject: Re: [PATCH] fs/fat/dir.c fix resource leak

OGAWA Hirofumi <[email protected]> writes:

> "Darren Jenkins\" <[email protected]> writes:
>
>> Coverity CID 2332 & 2333 RESOURCE_LEAK
>>
>> In fat_search_long() if fat_parse_long() returns a -ve value we return
>> without first freeing unicode and bufname.
>> This patch free's them on this error path.
>>
>> Signed-off-by: Darren Jenkins <darrenrjenkins@gmailcom>
>
> I'll apply to current code. Thanks.

I noticed this is false positive. bufname is not used anymore, and
fat_parse_long() never returns error after allocating unicode.

Instead, I'll apply this as cleanup.
--
OGAWA Hirofumi <[email protected]>

2008-07-30 09:15:56

by Darren Jenkins

Subject: Re: [PATCH] fs/fat/dir.c fix resource leak

OGAWA Hirofumi <[email protected]> writes:
> BTW, email address of Signed-off-by line is typo?
Yes, my mistake.
Signed-off-by: Darren Jenkins <[email protected]>


> I noticed this is false positive. bufname is not used anymore, and> fat_parse_long() never returns error after allocating unicode.
Yes you are right about unicode, I should of picked that up.
bufname is not used anymore ? I'm not sure what you mean? are youtalking about an external tree ?

Darren Jenkins????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2008-07-30 15:04:31

by OGAWA Hirofumi

Subject: Re: [PATCH] fs/fat/dir.c fix resource leak

"Darren Jenkins" <[email protected]> writes:

> bufname is not used anymore ? I'm not sure what you mean? are you
> talking about an external tree ?

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=fs/fat/dir.c;h=cd4a0162e10d6dea7507507cf25a13fd93d93f04;hb=HEAD#l356

No, about current Linus tree. I meant, bufname is used actually, but
it's not allocated dynamically by __getname().

Thanks.
--
OGAWA Hirofumi <[email protected]>