A memory-leak problem of unix_open()

A memory-leak problem of unix_open()

[Li Xi]

[-- Attachment #1: Type: text/plain, Size: 209 bytes --]

Hi all,

I think I found a memory-leak problem of e2fsprofgs while using
valgrind to testing a tool. 'log.txt' is the output. It is a simple
problem. I wrote a patch, and it works on my server.

Thanks

Li Xi

[-- Attachment #2: log.txt --]
[-- Type: text/plain, Size: 1730 bytes --]

[root@mds01 mmp]# valgrind --leak-check=full ./mmpstatus /dev/sd 
==19790== Memcheck, a memory error detector.
==19790== Copyright (C) 2002-2006, and GNU GPL'd, by Julian Seward et al.
==19790== Using LibVEX rev 1658, a library for dynamic binary translation.
==19790== Copyright (C) 2004-2006, and GNU GPL'd, by OpenWorks LLP.
==19790== Using valgrind-3.2.1, a dynamic binary instrumentation framework.
==19790== Copyright (C) 2000-2006, and GNU GPL'd, by Julian Seward et al.
==19790== For more details, rerun with: -v
==19790== 
ERROR: mmpstatus.c(280) main(): failed to open filesystem [/dev/sd], ret = 2
==19790== 
==19790== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 5 from 1)
==19790== malloc/free: in use at exit: 8 bytes in 1 blocks.
==19790== malloc/free: 5 allocs, 4 frees, 760 bytes allocated.
==19790== For counts of detected errors, rerun with: -v
==19790== searching for pointers to 1 not-freed blocks.
==19790== checked 100,712 bytes.
==19790== 
==19790== 8 bytes in 1 blocks are definitely lost in loss record 1 of 1
==19790==    at 0x4A05809: malloc (vg_replace_malloc.c:149)
==19790==    by 0x4C498FC: unix_open (ext2fs.h:1502)
==19790==    by 0x4C3F30B: ext2fs_open2 (openfs.c:139)
==19790==    by 0x4C3FA18: ext2fs_open (openfs.c:75)
==19790==    by 0x40116C: main (in /home/lixi/mmp/mmpstatus)
==19790== 
==19790== LEAK SUMMARY:
==19790==    definitely lost: 8 bytes in 1 blocks.
==19790==      possibly lost: 0 bytes in 0 blocks.
==19790==    still reachable: 0 bytes in 0 blocks.
==19790==         suppressed: 0 bytes in 0 blocks.
==19790== Reachable blocks (those to which a pointer was found) are not shown.
==19790== To see them, rerun with: --show-reachable=yes

[-- Attachment #3: bug_unix_open_memleak_e2fsprogs-1.42.6.patch --]
[-- Type: application/octet-stream, Size: 476 bytes --]

Index: e2fsprogs-1.42.6/lib/ext2fs/unix_io.c
===================================================================
--- e2fsprogs-1.42.6.orig/lib/ext2fs/unix_io.c	2012-12-17 06:46:30.000000000 +0800
+++ e2fsprogs-1.42.6/lib/ext2fs/unix_io.c	2012-12-17 07:11:00.000000000 +0800
@@ -624,8 +624,12 @@
 		free_cache(data);
 		ext2fs_free_mem(&data);
 	}
-	if (io)
+	if (io) {
+		if (io->name) {
+			ext2fs_free_mem(&io->name);
+		}
 		ext2fs_free_mem(&io);
+	}
 	return retval;
 }
 

Re: A memory-leak problem of unix_open()

[Theodore Ts'o]

On Mon, Dec 17, 2012 at 12:34:56AM +0800, Li Xi wrote:
> I think I found a memory-leak problem of e2fsprofgs while using
> valgrind to testing a tool. 'log.txt' is the output. It is a simple
> problem. I wrote a patch, and it works on my server.

Thank you very much for reporting the problem!  While I was looking at
your patch, I found some some potential fd leaks that should also be
fixed.  This is what I have checked into the e2fsprogs tree.

Regards,

							- Ted

commit 4e0bb5eb745009decac4c5836671ff4bef21ce2a
Author: Theodore Ts'o <tytso@mit.edu>
Date:   Sun Dec 16 20:14:20 2012 -0500

    libext2fs: fix memory and fd leak in error path of unix_open()
    
    Fix a potential memory leak reported by Li Xi.  In addition, there
    were possible error cases where the file descriptor would not be
    properly closed, so fix those as well while we're at it.
    
    Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
    Reported-by:  Li Xi <pkuelelixi@gmail.com>

diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
index 02570f0..7371654 100644
--- a/lib/ext2fs/unix_io.c
+++ b/lib/ext2fs/unix_io.c
@@ -505,6 +505,7 @@ static errcode_t unix_open(const char *name, int flags, io_channel *channel)
 	memset(data, 0, sizeof(struct unix_private_data));
 	data->magic = EXT2_ET_MAGIC_UNIX_IO_CHANNEL;
 	data->io_stats.num_fields = 2;
+	data->dev = -1;
 
 	open_flags = (flags & IO_FLAG_RW) ? O_RDWR : O_RDONLY;
 	if (flags & IO_FLAG_EXCLUSIVE)
@@ -575,7 +576,6 @@ static errcode_t unix_open(const char *name, int flags, io_channel *channel)
 		/* Is the block device actually writable? */
 		error = ioctl(data->dev, BLKROGET, &readonly);
 		if (!error && readonly) {
-			close(data->dev);
 			retval = EPERM;
 			goto cleanup;
 		}
@@ -621,11 +621,17 @@ static errcode_t unix_open(const char *name, int flags, io_channel *channel)
 
 cleanup:
 	if (data) {
+		if (data->dev >= 0)
+			close(data->dev);
 		free_cache(data);
 		ext2fs_free_mem(&data);
 	}
-	if (io)
+	if (io) {
+		if (io->name) {
+			ext2fs_free_mem(&io->name);
+		}
 		ext2fs_free_mem(&io);
+	}
 	return retval;
 }