2a38a002fbee06556489091c30b04746222167e4 is first bad commit
commit 2a38a002fbee06556489091c30b04746222167e4
Author: Xiaotian Feng <[email protected]>
Date: Wed Jul 22 17:03:57 2009 +0800
slub: sysfs_slab_remove should free kmem_cache when debug is enabled
kmem_cache_destroy use sysfs_slab_remove to release the kmem_cache,
but when CONFIG_SLUB_DEBUG is enabled, sysfs_slab_remove just release
related kobject, the whole kmem_cache is missed to release and cause
a memory leak.
Acked-by: Christoph Lameer <[email protected]>
Signed-off-by: Xiaotian Feng <[email protected]>
Signed-off-by: Pekka Enberg <[email protected]>
CONFIG_SLUB_DEBUG=y
CONFIG_SLUB=y
CONFIG_SLUB_DEBUG_ON=y
# CONFIG_SLUB_STATS is not set
I created a very simple kernel module which consisted only of:
static int __init kmem_cache_test_init_module(void)
{
struct kmem_cache *test_cachep;
test_cachep = kmem_cache_create("test_cachep", 32, 0, 0, NULL);
if (test_cachep)
kmem_cache_destroy(test_cachep);
return 0;
}
Before this patch it works just fine. After this patch I get a bug like
this:
[ 59.921431] kmem_cache_test_init_module:
[ 59.922415] =============================================================================
[ 59.922418] BUG kmalloc-8192: Object already free
[ 59.922419] -----------------------------------------------------------------------------
[ 59.922420]
[ 59.922453] INFO: Allocated in kmem_cache_create+0x70/0x320 age=1 cpu=3 pid=1781
[ 59.922458] INFO: Freed in kmem_cache_release+0x23/0x40 age=0 cpu=3 pid=1781
[ 59.922461] INFO: Slab 0xffffea0000373cc0 objects=3 used=1 fp=0xffff8800087fa048 flags=0x200000000040c3
[ 59.922463] INFO: Object 0xffff8800087fa048 @offset=8264 fp=0xffff8800087fc090
[ 59.922463]
[ 59.922465] Bytes b4 0xffff8800087fa038: 00 00 00 00 00 00 00 00 5a 5a 5a 5a 5a 5a 5a 5a ........ZZZZZZZZ
[ 59.922477] Object 0xffff8800087fa048: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
[ 59.922487] Object 0xffff8800087fa058: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
[snip]
[ 59.923261] Object 0xffff8800087fb028: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
[ 59.923261] Object 0xffff8800087fb038: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
[ 59.923261] Redzone 0xffff8800087fc048: bb bb bb bb bb bb bb bb »»»»»»»»
[ 59.923261] Padding 0xffff8800087fc088: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ
[ 59.923261] Pid: 1781, comm: insmod Not tainted 2.6.31-rc2 #33
[ 59.923261] Call Trace:
[ 59.923261] [<ffffffff81142e1b>] print_trailer+0xfb/0x160
[ 59.923261] [<ffffffff81142ec9>] object_err+0x49/0x70
[ 59.923261] [<ffffffff81146166>] __slab_free+0x266/0x3c0
[ 59.923261] [<ffffffff811463ac>] kfree+0xec/0x220
[ 59.923261] [<ffffffff81146c4e>] ? kmem_cache_destroy+0x20e/0x230
[ 59.923261] [<ffffffffa02d1000>] ? kmem_cache_test_init_module+0x0/0x67 [cache_test]
[ 59.923261] [<ffffffffa02d1000>] ? kmem_cache_test_init_module+0x0/0x67 [cache_test]
[ 59.923261] [<ffffffff81146c4e>] kmem_cache_destroy+0x20e/0x230
[ 59.923261] [<ffffffffa02d1000>] ? kmem_cache_test_init_module+0x0/0x67 [cache_test]
[ 59.923261] [<ffffffffa02d104f>] kmem_cache_test_init_module+0x4f/0x67 [cache_test]
[ 59.923261] [<ffffffff8100a07b>] do_one_initcall+0x4b/0x1a0
[ 59.923261] [<ffffffff810b5f08>] sys_init_module+0x108/0x260
[ 59.923261] [<ffffffff81014282>] system_call_fastpath+0x16/0x1b
[ 59.923261] FIX kmalloc-8192: Object at 0xffff8800087fa048 not freed
On Sun, 2009-09-13 at 14:33 -0400, Eric Paris wrote:
> 2a38a002fbee06556489091c30b04746222167e4 is first bad commit
> commit 2a38a002fbee06556489091c30b04746222167e4
> Author: Xiaotian Feng <[email protected]>
> Date: Wed Jul 22 17:03:57 2009 +0800
>
> slub: sysfs_slab_remove should free kmem_cache when debug is enabled
>
> kmem_cache_destroy use sysfs_slab_remove to release the kmem_cache,
> but when CONFIG_SLUB_DEBUG is enabled, sysfs_slab_remove just release
> related kobject, the whole kmem_cache is missed to release and cause
> a memory leak.
>
> Acked-by: Christoph Lameer <[email protected]>
> Signed-off-by: Xiaotian Feng <[email protected]>
> Signed-off-by: Pekka Enberg <[email protected]>
>
> CONFIG_SLUB_DEBUG=y
> CONFIG_SLUB=y
> CONFIG_SLUB_DEBUG_ON=y
> # CONFIG_SLUB_STATS is not set
I also had problems destroying a kmem_cache in a security_initcall()
function which had a different backtrace (it's what made me create the
module and bisect.) So be sure to let me know what you find so I can
be sure that we fix that place as well (I believe that was a kref
problem rather than a double free)
-Eric
On 09/14/2009 07:11 AM, Eric Paris wrote:
> On Sun, 2009-09-13 at 14:33 -0400, Eric Paris wrote:
>> 2a38a002fbee06556489091c30b04746222167e4 is first bad commit
>> commit 2a38a002fbee06556489091c30b04746222167e4
>> Author: Xiaotian Feng<[email protected]>
>> Date: Wed Jul 22 17:03:57 2009 +0800
>>
>> slub: sysfs_slab_remove should free kmem_cache when debug is enabled
>>
>> kmem_cache_destroy use sysfs_slab_remove to release the kmem_cache,
>> but when CONFIG_SLUB_DEBUG is enabled, sysfs_slab_remove just release
>> related kobject, the whole kmem_cache is missed to release and cause
>> a memory leak.
>>
>> Acked-by: Christoph Lameer<[email protected]>
>> Signed-off-by: Xiaotian Feng<[email protected]>
>> Signed-off-by: Pekka Enberg<[email protected]>
>>
>> CONFIG_SLUB_DEBUG=y
>> CONFIG_SLUB=y
>> CONFIG_SLUB_DEBUG_ON=y
>> # CONFIG_SLUB_STATS is not set
>
> I also had problems destroying a kmem_cache in a security_initcall()
> function which had a different backtrace (it's what made me create the
> module and bisect.) So be sure to let me know what you find so I can
> be sure that we fix that place as well (I believe that was a kref
> problem rather than a double free)
>
> -Eric
>
>
Could you please tell me the tree you're using? I'll debug on it first...
On Mon, 2009-09-14 at 09:52 +0800, Danny Feng wrote:
> On 09/14/2009 07:11 AM, Eric Paris wrote:
> > On Sun, 2009-09-13 at 14:33 -0400, Eric Paris wrote:
> >> 2a38a002fbee06556489091c30b04746222167e4 is first bad commit
> >> commit 2a38a002fbee06556489091c30b04746222167e4
> >> Author: Xiaotian Feng<[email protected]>
> >> Date: Wed Jul 22 17:03:57 2009 +0800
> >>
> >> slub: sysfs_slab_remove should free kmem_cache when debug is enabled
> >>
> >> kmem_cache_destroy use sysfs_slab_remove to release the kmem_cache,
> >> but when CONFIG_SLUB_DEBUG is enabled, sysfs_slab_remove just release
> >> related kobject, the whole kmem_cache is missed to release and cause
> >> a memory leak.
> >>
> >> Acked-by: Christoph Lameer<[email protected]>
> >> Signed-off-by: Xiaotian Feng<[email protected]>
> >> Signed-off-by: Pekka Enberg<[email protected]>
> >>
> >> CONFIG_SLUB_DEBUG=y
> >> CONFIG_SLUB=y
> >> CONFIG_SLUB_DEBUG_ON=y
> >> # CONFIG_SLUB_STATS is not set
> >
> > I also had problems destroying a kmem_cache in a security_initcall()
> > function which had a different backtrace (it's what made me create the
> > module and bisect.) So be sure to let me know what you find so I can
> > be sure that we fix that place as well (I believe that was a kref
> > problem rather than a double free)
> >
> > -Eric
> >
> >
> Could you please tell me the tree you're using? I'll debug on it first...
I was looking at the linux-next tree from Sept 11
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=summary
-Eric
On 09/14/2009 02:33 AM, Eric Paris wrote:
> 2a38a002fbee06556489091c30b04746222167e4 is first bad commit
> commit 2a38a002fbee06556489091c30b04746222167e4
> Author: Xiaotian Feng<[email protected]>
> Date: Wed Jul 22 17:03:57 2009 +0800
>
> slub: sysfs_slab_remove should free kmem_cache when debug is enabled
>
> kmem_cache_destroy use sysfs_slab_remove to release the kmem_cache,
> but when CONFIG_SLUB_DEBUG is enabled, sysfs_slab_remove just release
> related kobject, the whole kmem_cache is missed to release and cause
> a memory leak.
>
> Acked-by: Christoph Lameer<[email protected]>
> Signed-off-by: Xiaotian Feng<[email protected]>
> Signed-off-by: Pekka Enberg<[email protected]>
>
> CONFIG_SLUB_DEBUG=y
> CONFIG_SLUB=y
> CONFIG_SLUB_DEBUG_ON=y
> # CONFIG_SLUB_STATS is not set
>
> I created a very simple kernel module which consisted only of:
>
> static int __init kmem_cache_test_init_module(void)
> {
> struct kmem_cache *test_cachep;
>
> test_cachep = kmem_cache_create("test_cachep", 32, 0, 0, NULL);
> if (test_cachep)
> kmem_cache_destroy(test_cachep);
>
> return 0;
> }
>
> Before this patch it works just fine. After this patch I get a bug like
> this:
>
> [ 59.921431] kmem_cache_test_init_module:
> [ 59.922415] =============================================================================
> [ 59.922418] BUG kmalloc-8192: Object already free
> [ 59.922419] -----------------------------------------------------------------------------
> [ 59.922420]
> [ 59.922453] INFO: Allocated in kmem_cache_create+0x70/0x320 age=1 cpu=3 pid=1781
> [ 59.922458] INFO: Freed in kmem_cache_release+0x23/0x40 age=0 cpu=3 pid=1781
> [ 59.922461] INFO: Slab 0xffffea0000373cc0 objects=3 used=1 fp=0xffff8800087fa048 flags=0x200000000040c3
> [ 59.922463] INFO: Object 0xffff8800087fa048 @offset=8264 fp=0xffff8800087fc090
> [ 59.922463]
> [ 59.922465] Bytes b4 0xffff8800087fa038: 00 00 00 00 00 00 00 00 5a 5a 5a 5a 5a 5a 5a 5a ........ZZZZZZZZ
> [ 59.922477] Object 0xffff8800087fa048: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 59.922487] Object 0xffff8800087fa058: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [snip]
> [ 59.923261] Object 0xffff8800087fb028: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 59.923261] Object 0xffff8800087fb038: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 59.923261] Redzone 0xffff8800087fc048: bb bb bb bb bb bb bb bb »»»»»»»»
> [ 59.923261] Padding 0xffff8800087fc088: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ
> [ 59.923261] Pid: 1781, comm: insmod Not tainted 2.6.31-rc2 #33
> [ 59.923261] Call Trace:
> [ 59.923261] [<ffffffff81142e1b>] print_trailer+0xfb/0x160
> [ 59.923261] [<ffffffff81142ec9>] object_err+0x49/0x70
> [ 59.923261] [<ffffffff81146166>] __slab_free+0x266/0x3c0
> [ 59.923261] [<ffffffff811463ac>] kfree+0xec/0x220
> [ 59.923261] [<ffffffff81146c4e>] ? kmem_cache_destroy+0x20e/0x230
> [ 59.923261] [<ffffffffa02d1000>] ? kmem_cache_test_init_module+0x0/0x67 [cache_test]
> [ 59.923261] [<ffffffffa02d1000>] ? kmem_cache_test_init_module+0x0/0x67 [cache_test]
> [ 59.923261] [<ffffffff81146c4e>] kmem_cache_destroy+0x20e/0x230
> [ 59.923261] [<ffffffffa02d1000>] ? kmem_cache_test_init_module+0x0/0x67 [cache_test]
> [ 59.923261] [<ffffffffa02d104f>] kmem_cache_test_init_module+0x4f/0x67 [cache_test]
> [ 59.923261] [<ffffffff8100a07b>] do_one_initcall+0x4b/0x1a0
> [ 59.923261] [<ffffffff810b5f08>] sys_init_module+0x108/0x260
> [ 59.923261] [<ffffffff81014282>] system_call_fastpath+0x16/0x1b
> [ 59.923261] FIX kmalloc-8192: Object at 0xffff8800087fa048 not freed
>
>
I think I got the real problem, that's introduced from former commit
a0e1d1be204612ee83b3afe8aa24c5d27e63d464, this results kmem_cache
always be freed at kmem_cache_create....
Can following patch fix this issue?
On Mon, 2009-09-14 at 11:18 +0800, Danny Feng wrote:
> diff --git a/mm/slub.c b/mm/slub.c
> index b627675..40e12d5 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3337,8 +3337,8 @@ struct kmem_cache *kmem_cache_create(const char
> *name, size_t size,
> goto err;
> }
> return s;
> - }
> - kfree(s);
> + } else
> + kfree(s);
> }
> up_write(&slub_lock);
>
Doesn't the return inside the conditional take care of this? I'll give
it a try in the morning, but I don't see how this can solve the
problem....
-Eric
On 09/14/2009 11:25 AM, Eric Paris wrote:
> On Mon, 2009-09-14 at 11:18 +0800, Danny Feng wrote:
>> diff --git a/mm/slub.c b/mm/slub.c
>> index b627675..40e12d5 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -3337,8 +3337,8 @@ struct kmem_cache *kmem_cache_create(const char
>> *name, size_t size,
>> goto err;
>> }
>> return s;
>> - }
>> - kfree(s);
>> + } else
>> + kfree(s);
>> }
>> up_write(&slub_lock);
>>
>
> Doesn't the return inside the conditional take care of this? I'll give
> it a try in the morning, but I don't see how this can solve the
> problem....
>
> -Eric
>
>
err, you're right... let me try to find why. It's strange, if SLUB_DEBUG
is not set, sysfs_slab_remove is just to free kmem_cache s. So I think
if SLUB_DEBUG is not set, we also have the same issue....
On 09/14/2009 07:11 AM, Eric Paris wrote:
> On Sun, 2009-09-13 at 14:33 -0400, Eric Paris wrote:
>
>> 2a38a002fbee06556489091c30b04746222167e4 is first bad commit
>> commit 2a38a002fbee06556489091c30b04746222167e4
>> Author: Xiaotian Feng<[email protected]>
>> Date: Wed Jul 22 17:03:57 2009 +0800
>>
>> slub: sysfs_slab_remove should free kmem_cache when debug is enabled
>>
>> kmem_cache_destroy use sysfs_slab_remove to release the kmem_cache,
>> but when CONFIG_SLUB_DEBUG is enabled, sysfs_slab_remove just release
>> related kobject, the whole kmem_cache is missed to release and cause
>> a memory leak.
>>
>> Acked-by: Christoph Lameer<[email protected]>
>> Signed-off-by: Xiaotian Feng<[email protected]>
>> Signed-off-by: Pekka Enberg<[email protected]>
>>
>> CONFIG_SLUB_DEBUG=y
>> CONFIG_SLUB=y
>> CONFIG_SLUB_DEBUG_ON=y
>> # CONFIG_SLUB_STATS is not set
>>
> I also had problems destroying a kmem_cache in a security_initcall()
> function which had a different backtrace (it's what made me create the
> module and bisect.) So be sure to let me know what you find so I can
> be sure that we fix that place as well (I believe that was a kref
> problem rather than a double free)
>
> -Eric
>
>
>
That's my fault... Please drop this patch, I didn't notice the free
action in kobject release phase.. Thanks for point it out.
On Mon, Sep 14, 2009 at 6:41 AM, Danny Feng <[email protected]> wrote:
> That's my fault... Please drop this patch, I didn't notice the free action
> in kobject release phase.. Thanks for point it out.
Dropped, thanks Eric!