Uninitialized nodemask in do_mbind()

Uninitialized nodemask in do_mbind()

[Alexander Potapenko]

Hi linux-mm maintainers,

the following program:

====================
#define _GNU_SOURCE
#include <endian.h>
#include <stdint.h>
#include <string.h>
#include <sys/syscall.h>
#include <unistd.h>

int main()
{
  syscall(__NR_mmap, 0x20000000, 0xa000, 3, 0x32, -1, 0);
  syscall(__NR_mlock, 0x20002000, 0x4000);
  syscall(__NR_munlock, 0x20003000, 0x3000);
  *(uint64_t*)0x20006000 = 0;
  syscall(__NR_mbind, 0x20002000, 0x4000, 1, 0x20006000, 0x3ff, 0);
  return 0;
}
====================

triggers use of uninitialized memory in __mpol_equal() here:

  case MPOL_PREFERRED:
    return a->v.preferred_node == b->v.preferred_node;
(https://elixir.bootlin.com/linux/latest/source/mm/mempolicy.c#L2108)

It is detectable with KMSAN (see the report below) or with the following patch:

===============================================
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index d879f1d8a44a..26afdc657f32 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -279,6 +279,7 @@ static struct mempolicy *mpol_new(unsigned short
mode, unsigned short flags,
        atomic_set(&policy->refcnt, 1);
        policy->mode = mode;
        policy->flags = flags;
+       policy->v.preferred_node = 0xfefa;

        return policy;
 }
@@ -2124,6 +2125,8 @@ bool __mpol_equal(struct mempolicy *a, struct
mempolicy *b)
        case MPOL_INTERLEAVE:
                return !!nodes_equal(a->v.nodes, b->v.nodes);
        case MPOL_PREFERRED:
+               BUG_ON(a->v.preferred_node = 0xfefa);
+               BUG_ON(b->v.preferred_node = 0xfefa);
                return a->v.preferred_node == b->v.preferred_node;
===============================================

It's sufficient to zero-initialize v.preferred_node in mpol_new(), but
looks like it's required to call mpol_set_nodemask() after mpol_new(),
which didn't happen on this path (namely we skipped the "if (flags &
(MPOL_MF_MOVE | MPOL_MF_MOVE_ALL))" condition in do_mbind()), so the
fix should be a bit more involved.

KMSAN report is as follows:

==================================================================
BUG: KMSAN: use of uninitialized memory in vma_merge+0x876/0x1fa0 mm/mmap.c:1134
CPU: 0 PID: 3510 Comm: syz-executor1 Not tainted 4.16.0-rc4+ #3858
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:17 [inline]
 dump_stack+0x185/0x1d0 lib/dump_stack.c:53
 kmsan_report+0x142/0x1f0 mm/kmsan/kmsan.c:1093
 __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:676
 vma_merge+0x876/0x1fa0 mm/mmap.c:1134
 mbind_range mm/mempolicy.c:731 [inline]
 do_mbind mm/mempolicy.c:1233 [inline]
 SYSC_mbind+0x1436/0x2200 mm/mempolicy.c:1357
 SyS_mbind+0x8a/0xb0 mm/mempolicy.c:1339
 do_syscall_64+0x2f1/0x450 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x3d/0xa2
RIP: 0033:0x449099
RSP: 002b:00007fd01b907c68 EFLAGS: 00000246 ORIG_RAX: 00000000000000ed
RAX: ffffffffffffffda RBX: 00007fd01b9086cc RCX: 0000000000449099
RDX: 0000000000000001 RSI: 0000000000004000 RDI: 0000000020002000
RBP: 000000000071bea0 R08: 00000000000003ff R09: 0000000000000000
R10: 0000000020006000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 0000000000005b20 R14: 00000000006ebbc0 R15: 00007fd01b908700
origin:
 kmsan_save_stack_with_flags mm/kmsan/kmsan.c:303 [inline]
 kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:213
 kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:339
 kmem_cache_alloc+0xa5b/0xc60 mm/slub.c:2756
 mpol_new+0x35f/0x550 mm/mempolicy.c:276
 do_mbind mm/mempolicy.c:1190 [inline]
 SYSC_mbind+0x6bc/0x2200 mm/mempolicy.c:1357
 SyS_mbind+0x8a/0xb0 mm/mempolicy.c:1339
 do_syscall_64+0x2f1/0x450 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x3d/0xa2
==================================================================

WBR,
Alexander


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

[PATCH] mm/mempolicy: Avoid use uninitialized preferred_node

[Yisheng Xie]

Alexander reported an use of uninitialized memory in __mpol_equal(),
which is caused by incorrect use of preferred_node.

When mempolicy in mode MPOL_PREFERRED with flags MPOL_F_LOCAL, it use
numa_node_id() instead of preferred_node, however, __mpol_equeue() use
preferred_node without check whether it is MPOL_F_LOCAL or not.

Reported-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 mm/mempolicy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index d879f1d..641545e 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2124,6 +2124,9 @@ bool __mpol_equal(struct mempolicy *a, struct mempolicy *b)
 	case MPOL_INTERLEAVE:
 		return !!nodes_equal(a->v.nodes, b->v.nodes);
 	case MPOL_PREFERRED:
+		/* a's flags is the same as b's */
+		if (a->flags & MPOL_F_LOCAL)
+			return true;
 		return a->v.preferred_node == b->v.preferred_node;
 	default:
 		BUG();
-- 
1.8.3.1

Re: Uninitialized nodemask in do_mbind()

[Yisheng Xie]

On 2018/3/9 0:29, Alexander Potapenko wrote:
> Hi linux-mm maintainers,
> 
> the following program:
> 
> ====================
> #define _GNU_SOURCE
> #include <endian.h>
> #include <stdint.h>
> #include <string.h>
> #include <sys/syscall.h>
> #include <unistd.h>
> 
> int main()
> {
>   syscall(__NR_mmap, 0x20000000, 0xa000, 3, 0x32, -1, 0);
>   syscall(__NR_mlock, 0x20002000, 0x4000);
>   syscall(__NR_munlock, 0x20003000, 0x3000);
>   *(uint64_t*)0x20006000 = 0;
>   syscall(__NR_mbind, 0x20002000, 0x4000, 1, 0x20006000, 0x3ff, 0);
>   return 0;
> }
> ====================
> 
> triggers use of uninitialized memory in __mpol_equal() here:
> 
>   case MPOL_PREFERRED:
>     return a->v.preferred_node == b->v.preferred_node;
> (https://elixir.bootlin.com/linux/latest/source/mm/mempolicy.c#L2108)
> 
> It is detectable with KMSAN (see the report below) or with the following patch:
> 
> ===============================================
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index d879f1d8a44a..26afdc657f32 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -279,6 +279,7 @@ static struct mempolicy *mpol_new(unsigned short
> mode, unsigned short flags,
>         atomic_set(&policy->refcnt, 1);
>         policy->mode = mode;
>         policy->flags = flags;
> +       policy->v.preferred_node = 0xfefa;
> 
>         return policy;
>  }
> @@ -2124,6 +2125,8 @@ bool __mpol_equal(struct mempolicy *a, struct
> mempolicy *b)
>         case MPOL_INTERLEAVE:
>                 return !!nodes_equal(a->v.nodes, b->v.nodes);
>         case MPOL_PREFERRED:
> +               BUG_ON(a->v.preferred_node = 0xfefa);
> +               BUG_ON(b->v.preferred_node = 0xfefa);
>                 return a->v.preferred_node == b->v.preferred_node;
hmm, the problem is here, when mempolicy->flags & MPOL_F_LOCAL, it should use
numa_node_id instead of preferred_node, so following maybe the right fix:

--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2124,6 +2124,9 @@ bool __mpol_equal(struct mempolicy *a, struct mempolicy *b)
        case MPOL_INTERLEAVE:
                return !!nodes_equal(a->v.nodes, b->v.nodes);
        case MPOL_PREFERRED:
+               /* a's flags is the same as b's */
+               if (a->flags & MPOL_F_LOCAL)
+                       return true;
                return a->v.preferred_node == b->v.preferred_node;
        default:
                BUG();

Thanks
Yisheng

> ===============================================
> 
> It's sufficient to zero-initialize v.preferred_node in mpol_new(), but
> looks like it's required to call mpol_set_nodemask() after mpol_new(),
> which didn't happen on this path (namely we skipped the "if (flags &
> (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL))" condition in do_mbind()), so the
> fix should be a bit more involved.
> 
> KMSAN report is as follows:
> 
> ==================================================================
> BUG: KMSAN: use of uninitialized memory in vma_merge+0x876/0x1fa0 mm/mmap.c:1134
> CPU: 0 PID: 3510 Comm: syz-executor1 Not tainted 4.16.0-rc4+ #3858
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> Call Trace:
>  __dump_stack lib/dump_stack.c:17 [inline]
>  dump_stack+0x185/0x1d0 lib/dump_stack.c:53
>  kmsan_report+0x142/0x1f0 mm/kmsan/kmsan.c:1093
>  __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:676
>  vma_merge+0x876/0x1fa0 mm/mmap.c:1134
>  mbind_range mm/mempolicy.c:731 [inline]
>  do_mbind mm/mempolicy.c:1233 [inline]
>  SYSC_mbind+0x1436/0x2200 mm/mempolicy.c:1357
>  SyS_mbind+0x8a/0xb0 mm/mempolicy.c:1339
>  do_syscall_64+0x2f1/0x450 arch/x86/entry/common.c:287
>  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> RIP: 0033:0x449099
> RSP: 002b:00007fd01b907c68 EFLAGS: 00000246 ORIG_RAX: 00000000000000ed
> RAX: ffffffffffffffda RBX: 00007fd01b9086cc RCX: 0000000000449099
> RDX: 0000000000000001 RSI: 0000000000004000 RDI: 0000000020002000
> RBP: 000000000071bea0 R08: 00000000000003ff R09: 0000000000000000
> R10: 0000000020006000 R11: 0000000000000246 R12: 00000000ffffffff
> R13: 0000000000005b20 R14: 00000000006ebbc0 R15: 00007fd01b908700
> origin:
>  kmsan_save_stack_with_flags mm/kmsan/kmsan.c:303 [inline]
>  kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:213
>  kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:339
>  kmem_cache_alloc+0xa5b/0xc60 mm/slub.c:2756
>  mpol_new+0x35f/0x550 mm/mempolicy.c:276
>  do_mbind mm/mempolicy.c:1190 [inline]
>  SYSC_mbind+0x6bc/0x2200 mm/mempolicy.c:1357
>  SyS_mbind+0x8a/0xb0 mm/mempolicy.c:1339
>  do_syscall_64+0x2f1/0x450 arch/x86/entry/common.c:287
>  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> ==================================================================
> 
> WBR,
> Alexander
> 
> 

Re: [PATCH] mm/mempolicy: Avoid use uninitialized preferred_node

[Alexander Potapenko]

On Fri, Mar 9, 2018 at 6:21 AM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> Alexander reported an use of uninitialized memory in __mpol_equal(),
> which is caused by incorrect use of preferred_node.
>
> When mempolicy in mode MPOL_PREFERRED with flags MPOL_F_LOCAL, it use
> numa_node_id() instead of preferred_node, however, __mpol_equeue() use
> preferred_node without check whether it is MPOL_F_LOCAL or not.
>
> Reported-by: Alexander Potapenko <glider@google.com>
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
Tested-by: Alexander Potapenko <glider@google.com>

I confirm that the patch fixes the problem. Thanks for the quick turnaround!
Any idea which commit had introduced the bug in the first place?
> ---
>  mm/mempolicy.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index d879f1d..641545e 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2124,6 +2124,9 @@ bool __mpol_equal(struct mempolicy *a, struct mempolicy *b)
>         case MPOL_INTERLEAVE:
>                 return !!nodes_equal(a->v.nodes, b->v.nodes);
>         case MPOL_PREFERRED:
> +               /* a's flags is the same as b's */
> +               if (a->flags & MPOL_F_LOCAL)
> +                       return true;
>                 return a->v.preferred_node == b->v.preferred_node;
>         default:
>                 BUG();
> --
> 1.8.3.1
>



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Re: [PATCH] mm/mempolicy: Avoid use uninitialized preferred_node

[Yisheng Xie]

Hi Alexander ,

On 2018/3/9 18:49, Alexander Potapenko wrote:
> On Fri, Mar 9, 2018 at 6:21 AM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
>> Alexander reported an use of uninitialized memory in __mpol_equal(),
>> which is caused by incorrect use of preferred_node.
>>
>> When mempolicy in mode MPOL_PREFERRED with flags MPOL_F_LOCAL, it use
>> numa_node_id() instead of preferred_node, however, __mpol_equeue() use
>> preferred_node without check whether it is MPOL_F_LOCAL or not.
>>
>> Reported-by: Alexander Potapenko <glider@google.com>
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> Tested-by: Alexander Potapenko <glider@google.com>

Thanks,
> 
> I confirm that the patch fixes the problem. Thanks for the quick turnaround!
> Any idea which commit had introduced the bug in the first place?

IIUC, It is introduce by:
Fixes: fc36b8d3d819 (mempolicy: use MPOL_F_LOCAL to Indicate Preferred Local Policy)

Thanks
Yisheng
>> ---
>>  mm/mempolicy.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>> index d879f1d..641545e 100644
>> --- a/mm/mempolicy.c
>> +++ b/mm/mempolicy.c
>> @@ -2124,6 +2124,9 @@ bool __mpol_equal(struct mempolicy *a, struct mempolicy *b)
>>         case MPOL_INTERLEAVE:
>>                 return !!nodes_equal(a->v.nodes, b->v.nodes);
>>         case MPOL_PREFERRED:
>> +               /* a's flags is the same as b's */
>> +               if (a->flags & MPOL_F_LOCAL)
>> +                       return true;
>>                 return a->v.preferred_node == b->v.preferred_node;
>>         default:
>>                 BUG();
>> --
>> 1.8.3.1
>>
> 
> 
>