[BUG] Invalid return address of mmap() followed by mbind() in multithreaded context

[BUG] Invalid return address of mmap() followed by mbind() in multithreaded context

[Vasileios Karakasis]

[-- Attachment #1.1: Type: text/plain, Size: 1747 bytes --]

Hi,

I am implementing a multithreaded numa aware code where each thread
mmap()'s an anonymous private region and then mbind()'s it to its local
node. The threads are performing a series of such mmap() + mbind()
operations. My program crashed with SIGSEGV and I noticed that mmap()
returned an invalid address.

I am sending you a simple program that reproduces the error. The program
creates two threads and each thread starts allocating pages and then
binds them to the local node 0. After a number of iterations the program
crashes as it tries to dereference the address returned by mmap(). The
bug doesn't come up when using a single thread, neither when using only
mmap().

I am running a 2.6.39.1 kernel on a 64-bit dual-core machine, but I
tracked this bug back down to the 2.6.34.9 version.

This bug also affects libnuma.

Regards,
-- 
V.K.


#include <assert.h>
#include <sys/mman.h>
#include <pthread.h>
#include <numaif.h>

#define NR_ITER 10240
#define PAGE_SIZE 4096

void *thread_func(void *args)
{
    unsigned char *addr;
    int err, i;
    unsigned long node = 0x1;

    for (i = 0; i < NR_ITER; i++) {
        addr = mmap(0, PAGE_SIZE, PROT_READ | PROT_WRITE,
                    MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
        if (addr == (void *) -1) {
            assert(0 && "mmap failed");
        }
        *addr = 0;

        err = mbind(addr, PAGE_SIZE, MPOL_BIND, &node, sizeof(node), 0);
        if (err < 0) {
            assert(0 && "mbind failed");
        }
    }
    return (void *) 0;
}

int main(void)
{
    pthread_t thread;
    pthread_create(&thread, NULL, thread_func, NULL);
    thread_func(NULL);
    pthread_join(thread, NULL);
    return 0;
}

[-- Attachment #1.2: 0x17A67A9C.asc --]
[-- Type: application/pgp-keys, Size: 2859 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [BUG] Invalid return address of mmap() followed by mbind() in multithreaded context

[Andi Kleen]

>     for (i = 0; i < NR_ITER; i++) {
>         addr = mmap(0, PAGE_SIZE, PROT_READ | PROT_WRITE,
>                     MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
>         if (addr == (void *) -1) {
>             assert(0 && "mmap failed");
>         }
>         *addr = 0;
> 
>         err = mbind(addr, PAGE_SIZE, MPOL_BIND, &node, sizeof(node), 0);

mbind() can be only done before the first touch. you're not actually testing 
numa policy.

-andi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [BUG] Invalid return address of mmap() followed by mbind() in multithreaded context

[Vasileios Karakasis]

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

That's right, but what I want to demonstrate is that the address
returned by mmap() is invalid and the dereference crashes the program,
while it shouldn't. I could equally omit this statement, in which case
mbind() would fail with EFAULT.

On 06/18/2011 09:12 PM, Andi Kleen wrote:
>>     for (i = 0; i < NR_ITER; i++) {
>>         addr = mmap(0, PAGE_SIZE, PROT_READ | PROT_WRITE,
>>                     MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
>>         if (addr == (void *) -1) {
>>             assert(0 && "mmap failed");
>>         }
>>         *addr = 0;
>>
>>         err = mbind(addr, PAGE_SIZE, MPOL_BIND, &node, sizeof(node), 0);
> 
> mbind() can be only done before the first touch. you're not actually testing 
> numa policy.
> 
> -andi

-- 
V.K.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [BUG] Invalid return address of mmap() followed by mbind() in multithreaded context

[Vasileios Karakasis]

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

I'm sending you a slightly modified version that actually makes clear
how libnuma is affected. If you compile with -DUSE_LIBNUMA, you will get
an EFAULT from mbind() and then crash.

This is the gdb output where the address passed to mbind() is invalid.

(gdb) r
Starting program: a.out
[Thread debugging using libthread_db enabled]
[New Thread 0x7ffff7633700 (LWP 17977)]
a.out: mmap-bug.c:29: thread_func: Assertion `0 && "mbind() failed"' failed.

Program received signal SIGABRT, Aborted.
0x00007ffff7667a75 in *__GI_raise (sig=<value optimized out>)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64	../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
	in ../nptl/sysdeps/unix/sysv/linux/raise.c
(gdb) f 3
#3  0x00000000004007b8 in thread_func (args=0x0) at mmap-bug.c:29
29	            assert(0 && "mbind() failed");
(gdb) p addr
$1 = (unsigned char *) 0x7ffff5c27000 <Address 0x7ffff5c27000 out of bounds>


#include <assert.h>
#include <sys/mman.h>
#include <pthread.h>
#include <numa.h>
#include <numaif.h>

#define NR_ITER 10240
#define PAGE_SIZE 4096

void *thread_func(void *args)
{
    unsigned char *addr;
    int err, i;
    unsigned long node = 0x1;

    for (i = 0; i < NR_ITER; i++) {
#ifdef USE_LIBNUMA
        addr = numa_alloc_onnode(PAGE_SIZE, 0);
#else
        addr = mmap(0, PAGE_SIZE, PROT_READ | PROT_WRITE,
                    MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
        if (addr == (void *) -1)
            assert(0 && "mmap() failed");

        err = mbind(addr, PAGE_SIZE, MPOL_BIND, &node, sizeof(node), 0);
        if (err < 0)
            assert(0 && "mbind() failed");
#endif
        *addr = 0;
    }

    return (void *) 0;
}

int main(void)
{
    pthread_t thread;
    pthread_create(&thread, NULL, thread_func, NULL);
    thread_func(NULL);
    pthread_join(thread, NULL);
    return 0;
}



On 06/18/2011 09:41 PM, Vasileios Karakasis wrote:
> That's right, but what I want to demonstrate is that the address
> returned by mmap() is invalid and the dereference crashes the program,
> while it shouldn't. I could equally omit this statement, in which case
> mbind() would fail with EFAULT.
> 
> On 06/18/2011 09:12 PM, Andi Kleen wrote:
>>
>> mbind() can be only done before the first touch. you're not actually testing 
>> numa policy.
>>
>> -andi
> 

-- 
V.K.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [BUG] Invalid return address of mmap() followed by mbind() in multithreaded context

[Kornilios Kourtis]

Hi,

On Fri, Jun 17, 2011 at 06:21:49PM +0300, Vasileios Karakasis wrote:
> Hi,
> 
> I am implementing a multithreaded numa aware code where each thread
> mmap()'s an anonymous private region and then mbind()'s it to its local
> node. The threads are performing a series of such mmap() + mbind()
> operations. My program crashed with SIGSEGV and I noticed that mmap()
> returned an invalid address.

I've taken a closer look at this issue.

As Vasileios said, it can be reproduced by having two threads doing the
following loop:
| for {
| 	addr = mmap(4096, MAP_ANONUMOUS)
| 	if (addr == (void *)-1)
| 		continue
| 	mbind(addr, 4096, 0x1) // do mbind on first NUMA node
| }
After a couple of iterations, mbind() will return EFAULT, although the addr is
valid.

Doing a bisect, pins it down to the following commit (Author added to To:):
	9d8cebd4bcd7c3878462fdfda34bbcdeb4df7ef4
	mm: fix mbind vma merge problem
Which adds merging of vmas in the mbind() path.
Reverting this commit, seems to fix the issue.

I 've added some printks to track down the issue, and EFAULT is returned on:
mm/mempolicy.c: mbind_range()
|   vma = find_vma_prev(mm. start, &prev);
|   if (!vma |vma->vm_start > start)
|       return EFAULT;
Where: vma->start > start

I am not sure what exactly happens, but concurrent merges and splits
of (already mapped) VMAs do not seem to work well together.

cheers,
-Kornilios

-- 
Kornilios Kourtis

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [BUG] Invalid return address of mmap() followed by mbind() in multithreaded context

[KOSAKI Motohiro]

(2011/06/28 2:18), Kornilios Kourtis wrote:
> 
> Hi,
> 
> On Fri, Jun 17, 2011 at 06:21:49PM +0300, Vasileios Karakasis wrote:
>> Hi,
>>
>> I am implementing a multithreaded numa aware code where each thread
>> mmap()'s an anonymous private region and then mbind()'s it to its local
>> node. The threads are performing a series of such mmap() + mbind()
>> operations. My program crashed with SIGSEGV and I noticed that mmap()
>> returned an invalid address.
> 
> I've taken a closer look at this issue.
> 
> As Vasileios said, it can be reproduced by having two threads doing the
> following loop:
> | for {
> | 	addr = mmap(4096, MAP_ANONUMOUS)
> | 	if (addr == (void *)-1)
> | 		continue
> | 	mbind(addr, 4096, 0x1) // do mbind on first NUMA node
> | }
> After a couple of iterations, mbind() will return EFAULT, although the addr is
> valid.
> 
> Doing a bisect, pins it down to the following commit (Author added to To:):
> 	9d8cebd4bcd7c3878462fdfda34bbcdeb4df7ef4
> 	mm: fix mbind vma merge problem
> Which adds merging of vmas in the mbind() path.
> Reverting this commit, seems to fix the issue.
> 
> I 've added some printks to track down the issue, and EFAULT is returned on:
> mm/mempolicy.c: mbind_range()
> |   vma = find_vma_prev(mm. start, &prev);
> |   if (!vma |vma->vm_start > start)
> |       return EFAULT;
> Where: vma->start > start
> 
> I am not sure what exactly happens, but concurrent merges and splits
> of (already mapped) VMAs do not seem to work well together.

Hi

Thank you for digging this! I look it at soon as far as possible.

 - kosaki

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>