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

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

[Darren Jenkins\]

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));

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

[OGAWA Hirofumi]

"Darren Jenkins\" <darrenrjenkins@gmail.com> 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 <hirofumi@mail.parknet.co.jp>

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

[OGAWA Hirofumi]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

> "Darren Jenkins\" <darrenrjenkins@gmail.com> 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 <hirofumi@mail.parknet.co.jp>

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

[Darren Jenkins]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 660 bytes --]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:
> BTW, email address of Signed-off-by line is typo?
Yes, my mistake.
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>


> 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Ç+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

[OGAWA Hirofumi]

"Darren Jenkins" <darrenrjenkins@gmail.com> 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 <hirofumi@mail.parknet.co.jp>