2003-02-28 14:54:32

by Petr Vandrovec

[permalink] [raw]
Subject: Memory modified after freeing in 2.5.63?

Hi,
for some time I'm using patch attached at the end of this email,
which modifies check_poison function to not only verify that
last byte is POISON_END, but also that all preceeding bytes are
either POISON_BEFORE or POISON_AFTER bytes.

And now when I returned from my month vacation and upgraded
from 2.5.52 to 2.5.63, when dselect/apt updates dozens of packages,
I'm getting memory corruption reports as shown below - 22nd byte
in vm_area_struct - which looks like that VM_ACCOUNT in vm_flags
is set after vma is freeed... Any clue? Setting VM_ACCOUNT
in mremap.c:move_vma after calling do_unmap() looks suspicious
to me, but as I know almost nothing about MM...

Kernel is SMP, 2.5.63, with all debugging except stack frame
enabled, running on UP P4.
Thanks,
Petr Vandrovec
[email protected]

(* = POISON_AFTER, . = POISON_BEFORE, number = other value)

Corruption! start=d134abb4, expend=d134abf3, problemat=d134abca
Data: **********************7B ****************************************A5
Next: 71 F0 2C .A5 C2 0F 17 20 E2 32 DA 00 40 01 40 00 50 01 40 A4 9F 9A D9 25 00 00 00 73 00 10 00
slab error in check_poison_obj(): cache `vm_area_struct': object was modified after freeing
Call Trace:
[<c014165c>] check_poison_obj+0xf7/0x130
[<c0143ba9>] cache_alloc_debugcheck_after+0xbd/0xc5
[<c0142ac7>] kmem_cache_alloc+0x6c/0xaa
[<c014cb50>] split_vma+0x56/0x123
[<c014ccdd>] do_munmap+0xc0/0x1cd
[<c014b832>] sys_brk+0x127/0x12b
[<c01096cf>] syscall_call+0x7/0xb

Corruption! start=d134a734, expend=d134a773, problemat=d134a74a
Data: **********************7B ****************************************A5
Next: 71 F0 2C .71 F0 2C .************************
slab error in check_poison_obj(): cache `vm_area_struct': object was modified after freeing
Call Trace:
[<c014165c>] check_poison_obj+0xf7/0x130
[<c0143ba9>] cache_alloc_debugcheck_after+0xbd/0xc5
[<c0142212>] kmem_flagcheck+0x1f/0x2b
[<c0142ac7>] kmem_cache_alloc+0x6c/0xaa
[<c014e27f>] move_vma+0x269/0x598
[<c012b0be>] update_wall_time+0xd/0x36
[<c014d3f8>] arch_get_unmapped_area+0x72/0xb7
[<c014e83b>] do_mremap+0x28d/0x3eb
[<c014ea0a>] sys_mremap+0x71/0x9a
[<c01096cf>] syscall_call+0x7/0xb

Corruption! start=d13b76a4, expend=d13b76e3, problemat=d13b76ba
Data: **********************7B ****************************************A5
Next: 71 F0 2C .A5 C2 0F 17 90 A8 F3 D9 00 20 20 40 00 20 21 40 94 01 B6 D9 25 00 00 00 75 00 00 00
slab error in check_poison_obj(): cache `vm_area_struct': object was modified after freeing
Call Trace:
[<c014165c>] check_poison_obj+0xf7/0x130
[<c0143ba9>] cache_alloc_debugcheck_after+0xbd/0xc5
[<c0142ac7>] kmem_cache_alloc+0x6c/0xaa
[<c014cb50>] split_vma+0x56/0x123
[<c014ccdd>] do_munmap+0xc0/0x1cd
[<c014b832>] sys_brk+0x127/0x12b
[<c01096cf>] syscall_call+0x7/0xb

Corruption! start=d134aecc, expend=d134af0b, problemat=d134aee2
Data: **********************7B ****************************************A5
Next: 71 F0 2C .71 F0 2C .************************
slab error in check_poison_obj(): cache `vm_area_struct': object was modified after freeing
Call Trace:
[<c014165c>] check_poison_obj+0xf7/0x130
[<c0143ba9>] cache_alloc_debugcheck_after+0xbd/0xc5
[<c0142ac7>] kmem_cache_alloc+0x6c/0xaa
[<c014e27f>] move_vma+0x269/0x598
[<c012b0be>] update_wall_time+0xd/0x36
[<c014d3f8>] arch_get_unmapped_area+0x72/0xb7
[<c014e83b>] do_mremap+0x28d/0x3eb
[<c014ea0a>] sys_mremap+0x71/0x9a
[<c01096cf>] syscall_call+0x7/0xb

diff -urdN linux/mm/slab.c linux/mm/slab.c
--- linux/mm/slab.c 2003-02-24 20:20:02.000000000 +0000
+++ linux/mm/slab.c 2003-02-24 20:54:51.000000000 +0000
@@ -768,6 +768,20 @@
*(unsigned char *)(addr+size-1) = POISON_END;
}

+static void* fprob(unsigned char* addr, unsigned int size) {
+ unsigned char* end;
+
+ end = addr + size - 1;
+
+ for (; addr < end; addr++) {
+ if (*addr != POISON_BEFORE && *addr != POISON_AFTER)
+ return addr;
+ }
+ if (*addr != POISON_END)
+ return addr;
+ return NULL;
+}
+
static void check_poison_obj(kmem_cache_t *cachep, void *addr)
{
int size = cachep->objsize;
@@ -776,9 +790,32 @@
addr += BYTES_PER_WORD;
size -= 2*BYTES_PER_WORD;
}
- end = memchr(addr, POISON_END, size);
- if (end != (addr+size-1))
+ end = fprob(addr, size);
+ if (end) {
+ int s;
+ printk(KERN_ERR "Corruption! start=%p, expend=%p, problemat=%p\n", addr, addr+size-1, end);
+ printk(KERN_ERR "Data: ");
+ for (s = 0; s < size; s++) {
+ if (((char*)addr)[s] == POISON_BEFORE)
+ printk(".");
+ else if (((char*)addr)[s] == POISON_AFTER)
+ printk("*");
+ else
+ printk("%02X ", ((unsigned char*)addr)[s]);
+ }
+ printk("\n");
+ printk(KERN_ERR "Next: ");
+ for (; s < size + 32; s++) {
+ if (((char*)addr)[s] == POISON_BEFORE)
+ printk(".");
+ else if (((char*)addr)[s] == POISON_AFTER)
+ printk("*");
+ else
+ printk("%02X ", ((unsigned char*)addr)[s]);
+ }
+ printk("\n");
slab_error(cachep, "object was modified after freeing");
+ }
}
#endif


2003-02-28 21:16:37

by Andrew Morton

[permalink] [raw]
Subject: Re: Memory modified after freeing in 2.5.63?

Petr Vandrovec <[email protected]> wrote:
>
> Hi,
> for some time I'm using patch attached at the end of this email,
> which modifies check_poison function to not only verify that
> last byte is POISON_END, but also that all preceeding bytes are
> either POISON_BEFORE or POISON_AFTER bytes.

Nice patch.

> And now when I returned from my month vacation and upgraded
> from 2.5.52 to 2.5.63, when dselect/apt updates dozens of packages,
> I'm getting memory corruption reports as shown below - 22nd byte
> in vm_area_struct - which looks like that VM_ACCOUNT in vm_flags
> is set after vma is freeed... Any clue? Setting VM_ACCOUNT
> in mremap.c:move_vma after calling do_unmap() looks suspicious
> to me, but as I know almost nothing about MM...

Ha! I've been harrassing Hugh over this ;)

Thanks.


2003-02-28 21:38:57

by Hugh Dickins

[permalink] [raw]
Subject: Re: Memory modified after freeing in 2.5.63?

On Fri, 28 Feb 2003, Petr Vandrovec wrote:
> for some time I'm using patch attached at the end of this email,
> which modifies check_poison function to not only verify that
> last byte is POISON_END, but also that all preceeding bytes are
> either POISON_BEFORE or POISON_AFTER bytes.
>
> And now when I returned from my month vacation and upgraded
> from 2.5.52 to 2.5.63, when dselect/apt updates dozens of packages,
> I'm getting memory corruption reports as shown below - 22nd byte
> in vm_area_struct - which looks like that VM_ACCOUNT in vm_flags
> is set after vma is freeed... Any clue? Setting VM_ACCOUNT
> in mremap.c:move_vma after calling do_unmap() looks suspicious
> to me, but as I know almost nothing about MM...

Petr, do you have a corner I can hide in? Andrew will whip me.
He nags me from time to time about that code, he's sceptical
whereas I believed it fragile but okay for now. Looks like
you've caught me out, perhaps I should just take your POISON.

Hmm, it's really not what I want to be thinking about right now:
the real fix is to split_vma explicitly before the do_unmap, but
hard to get into that without opening up other cans of worms
(what to do if split_vma fails for lack of memory). Many thanks
for the report, I won't be able to fix it in the next three days,
but will get back to you when I have a patch to try.

Hugh