| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 15 Jul 2009 13:33:36 +0530
From: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To: Catalin Marinas <catalin.marinas@....com>
Cc: Alexey Fisher <bug-track@...her-privat.net>,
Pekka Enberg <penberg@...helsinki.fi>,
Kernel Testers List <kernel-testers@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Sam Ravnborg <sam@...nborg.org>, Ingo Molnar <mingo@...e.hu>,
linux-ext4@...r.kernel.org
Subject: Re: ext4 memory leak (was Re: [PATCH] x86: _edata should include
all .data.* sections on X86_64)
On Tue, Jul 14, 2009 at 01:26:30PM +0100, Catalin Marinas wrote:
> (I cc'ed linux-ext4@...r.kernel.org as well)
>
> On Tue, 2009-07-14 at 12:37 +0200, Alexey Fisher wrote:
> > this is complete trace from debug/kmemleak .
> [...]
> > i will compile now latest linux-arm.org/linux-2.6.git
> > unreferenced object 0xffff880132c48890 (size 1024):
> > comm "exe", pid 1612, jiffies 4294894130
> > backtrace:
> > [<ffffffff810fbaca>] create_object+0x13a/0x2c0
> > [<ffffffff810fbd75>] kmemleak_alloc+0x25/0x60
> > [<ffffffff810f596b>] __kmalloc+0x11b/0x210
> > [<ffffffff811ae061>] ext4_mb_init+0x1b1/0x5c0
> > [<ffffffff8119f1e9>] ext4_fill_super+0x1e29/0x2720
> > [<ffffffff8110111f>] get_sb_bdev+0x16f/0x1b0
> > [<ffffffff81195413>] ext4_get_sb+0x13/0x20
> > [<ffffffff81100bf6>] vfs_kern_mount+0x76/0x180
> > [<ffffffff81100d6d>] do_kern_mount+0x4d/0x120
> > [<ffffffff81118ee7>] do_mount+0x307/0x8b0
> > [<ffffffff8111951f>] sys_mount+0x8f/0xe0
> > [<ffffffff8100b66b>] system_call_fastpath+0x16/0x1b
> > [<ffffffffffffffff>] 0xffffffffffffffff
>
> After some investigation, this looks to me like a real leak.
>
> I managed to reproduce something similar (though the size may differ, I
> think depending on filesystem size - only tried with a 64MB loop
> device):
>
> unreferenced object 0xde468300 (size 32):
> comm "mount", pid 1445, jiffies 4294950074
> backtrace:
> [<c006d473>] __save_stack_trace+0x17/0x1c
> [<c006d545>] create_object+0xcd/0x188
> [<c01efe43>] kmemleak_alloc+0x1b/0x3c
> [<c006c013>] __kmalloc+0xd7/0xe4
> [<c00c1029>] ext4_mb_init+0x14d/0x374
> [<c00b7d7d>] ext4_fill_super+0x1385/0x16b4
> [<c0070891>] get_sb_bdev+0xa9/0xe4
> [<c00b574b>] ext4_get_sb+0xf/0x14
> [<c006fd3f>] vfs_kern_mount+0x33/0x64
> [<c006fda5>] do_kern_mount+0x25/0x8c
> [<c007e11f>] do_mount+0x47f/0x4c4
> [<c007e1b5>] sys_mount+0x51/0x80
> [<c0027c01>] ret_fast_syscall+0x1/0x40
> [<ffffffff>] 0xffffffff
>
> The above block is the meta_group_info allocated in
> ext4_mb_init_backend() and stored in sbi->s_group_info[i] (i = 0 in my
> case). Adding printk's and and inspecting the memory at
> sbi->s_group_info[] shows different value stored, not the pointer
> reported as leak.
>
> About the new pointer at sbi->s_group_info[0], kmemleak has this
> information (via the dump= option in my branch; it isn't a leak report):
>
> kmemleak: Object 0xdfebfa80 (size 128):
> kmemleak: comm "mount", pid 1445, jiffies 4294950075
> kmemleak: min_count = 1
> kmemleak: count = 1
> kmemleak: flags = 0x1
> kmemleak: backtrace:
> [<c006d473>] __save_stack_trace+0x17/0x1c
> [<c006d545>] create_object+0xcd/0x188
> [<c01efe43>] kmemleak_alloc+0x1b/0x3c
> [<c006c013>] __kmalloc+0xd7/0xe4
> [<c00c0df1>] ext4_mb_add_groupinfo+0x29/0x114
> [<c00c107f>] ext4_mb_init+0x1a3/0x374
> [<c00b7d7d>] ext4_fill_super+0x1385/0x16b4
> [<c0070891>] get_sb_bdev+0xa9/0xe4
> [<c00b574b>] ext4_get_sb+0xf/0x14
> [<c006fd3f>] vfs_kern_mount+0x33/0x64
> [<c006fda5>] do_kern_mount+0x25/0x8c
> [<c007e11f>] do_mount+0x47f/0x4c4
> [<c007e1b5>] sys_mount+0x51/0x80
> [<c0027c01>] ret_fast_syscall+0x1/0x40
> [<ffffffff>] 0xffffffff
>
> So, ext4_mb_add_groupinfo() is overriding the pointers stored in
> sbi->s_group_info[] by the ext4_mb_init_backend() function without
> freeing them first.
>
> Maybe the ext4 people could clarify what is happening here as I'm not
> familiar with the code.
>
Can you try this patch ?
commit 4cc505d4c16c86f8f590ce4b288c920572bf2be9
Author: Aneesh Kumar K.V <aneesh.kumar@...ux.vnet.ibm.com>
Date: Wed Jul 15 13:20:37 2009 +0530
ext4: Memory leak fix ext4_group_info allocation.
commit 5f21b0e642d7bf6fe4434c9ba12bc9cb96b17cf7 was done to
reallocate groupinfo struct during resize properly. That goal
was to allocate new groupinfo struct when we are adding new block
groups during resize. Calling ext4_mb_add_group_info in the
mballoc initialization code path resulted in we reallocating
the group info struct . Fix this by not separately allocating
group info in the mballoc init path and always depend on
ext4_mb_add_group_info to allocate group info struct.
The earlier code also had a bug that we allocated less number of
group info struct for the last meta group. But on resize we
expected that we had EXT4_DESC_PER_BLOCK group info struct for
each meta group.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@...ux.vnet.ibm.com>
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 519a0a6..96ed1d8 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2615,22 +2615,6 @@ static int ext4_mb_init_backend(struct super_block *sb)
goto err_freesgi;
}
EXT4_I(sbi->s_buddy_cache)->i_disksize = 0;
-
- metalen = sizeof(*meta_group_info) << EXT4_DESC_PER_BLOCK_BITS(sb);
- for (i = 0; i < num_meta_group_infos; i++) {
- if ((i + 1) == num_meta_group_infos)
- metalen = sizeof(*meta_group_info) *
- (ngroups -
- (i << EXT4_DESC_PER_BLOCK_BITS(sb)));
- meta_group_info = kmalloc(metalen, GFP_KERNEL);
- if (meta_group_info == NULL) {
- printk(KERN_ERR "EXT4-fs: can't allocate mem for a "
- "buddy group\n");
- goto err_freemeta;
- }
- sbi->s_group_info[i] = meta_group_info;
- }
-
for (i = 0; i < ngroups; i++) {
desc = ext4_get_group_desc(sb, i, NULL);
if (desc == NULL) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists