libext2fs: allocate separate memory regions for each inode in the cache

The changes to support metadata checksum allocated a single large
array for all of the inodes in the inode cache.  This is slightly more
efficient, but given that the inode cache is small (only 4 inodes) it
doesn't really have that much benefit.  The problem with doing things
this way is that the memory overruns, such as the one fixed in commit
43c4910371a, do not get detected by valgrind.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 9148d4ec9d9427dfa5032e63f75f4b81a40d9274..7ec189e81142e9809d0015926da2875eee7b0c34 100644 (file)
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -1328,6 +1328,7 @@ extern errcode_t ext2fs_get_memalign(unsigned long size,
                                     unsigned long align, void *ptr);
 
 /* inode.c */
+extern void ext2fs_free_inode_cache(struct ext2_inode_cache *icache);
 extern errcode_t ext2fs_flush_icache(ext2_filsys fs);
 extern errcode_t ext2fs_get_next_inode_full(ext2_inode_scan scan,
                                            ext2_ino_t *ino,

diff --git a/lib/ext2fs/freefs.c b/lib/ext2fs/freefs.c
index 28c4132fd9d9d1177be7c8ce40b28dd27f07c5a8..d3e815cf8ab41dc8d1e286750918e5e3259d48ef 100644 (file)
--- a/lib/ext2fs/freefs.c
+++ b/lib/ext2fs/freefs.c
@@ -18,8 +18,6 @@
 #include "ext2_fs.h"
 #include "ext2fsP.h"
 
-static void ext2fs_free_inode_cache(struct ext2_inode_cache *icache);
-
 void ext2fs_free(ext2_filsys fs)
 {
        if (!fs || (fs->magic != EXT2_ET_MAGIC_EXT2FS_FILSYS))
@@ -64,21 +62,6 @@ void ext2fs_free(ext2_filsys fs)
        ext2fs_free_mem(&fs);
 }
 
-/*
- * Free the inode cache structure
- */
-static void ext2fs_free_inode_cache(struct ext2_inode_cache *icache)
-{
-       if (--icache->refcount)
-               return;
-       if (icache->buffer)
-               ext2fs_free_mem(&icache->buffer);
-       if (icache->cache)
-               ext2fs_free_mem(&icache->cache);
-       icache->buffer_blk = 0;
-       ext2fs_free_mem(&icache);
-}
-
 /*
  * This procedure frees a badblocks list.
  */

diff --git a/lib/ext2fs/inode.c b/lib/ext2fs/inode.c
index e47d664fcaf97bf9f230b12a73e7cfe72a8cae28..f877146d18a04af3b128bc4563bf2ec6a4209fe5 100644 (file)
--- a/lib/ext2fs/inode.c
+++ b/lib/ext2fs/inode.c
@@ -72,6 +72,25 @@ errcode_t ext2fs_flush_icache(ext2_filsys fs)
        return 0;
 }
 
+/*
+ * Free the inode cache structure
+ */
+void ext2fs_free_inode_cache(struct ext2_inode_cache *icache)
+{
+       int i;
+
+       if (--icache->refcount)
+               return;
+       if (icache->buffer)
+               ext2fs_free_mem(&icache->buffer);
+       for (i = 0; i < icache->cache_size; i++)
+               ext2fs_free_mem(&icache->cache[i].inode);
+       if (icache->cache)
+               ext2fs_free_mem(&icache->cache);
+       icache->buffer_blk = 0;
+       ext2fs_free_mem(&icache);
+}
+
 static errcode_t create_icache(ext2_filsys fs)
 {
        int             i;
@@ -86,31 +105,32 @@ static errcode_t create_icache(ext2_filsys fs)
 
        memset(fs->icache, 0, sizeof(struct ext2_inode_cache));
        retval = ext2fs_get_mem(fs->blocksize, &fs->icache->buffer);
-       if (retval) {
-               ext2fs_free_mem(&fs->icache);
-               return retval;
-       }
+       if (retval)
+               goto errout;
+
        fs->icache->buffer_blk = 0;
        fs->icache->cache_last = -1;
        fs->icache->cache_size = 4;
        fs->icache->refcount = 1;
        retval = ext2fs_get_array(fs->icache->cache_size,
-                                 sizeof(struct ext2_inode_cache_ent) +
-                                 EXT2_INODE_SIZE(fs->super),
+                                 sizeof(struct ext2_inode_cache_ent),
                                  &fs->icache->cache);
-       if (retval) {
-               ext2fs_free_mem(&fs->icache->buffer);
-               ext2fs_free_mem(&fs->icache);
-               return retval;
-       }
+       if (retval)
+               goto errout;
 
-       for (i = 0, p = (void *)(fs->icache->cache + fs->icache->cache_size);
-            i < fs->icache->cache_size;
-            i++, p += EXT2_INODE_SIZE(fs->super))
-               fs->icache->cache[i].inode = p;
+       for (i = 0; i < fs->icache->cache_size; i++) {
+               retval = ext2fs_get_mem(EXT2_INODE_SIZE(fs->super),
+                                       &fs->icache->cache[i].inode);
+               if (retval)
+                       goto errout;
+       }
 
        ext2fs_flush_icache(fs);
        return 0;
+errout:
+       ext2fs_free_inode_cache(fs->icache);
+       fs->icache = 0;
+       return retval;
 }
 
 errcode_t ext2fs_open_inode_scan(ext2_filsys fs, int buffer_blocks,