From: Zhang Yanfei <zhangyanfei.yes@gmail.com>
To: Ghennadi Procopciuc <unix140@gmail.com>
Cc: akpm@linux-foundation.org, js1304@gmail.com, rientjes@google.com,
minchan@kernel.org, kosaki.motohiro@jp.fujitsu.com,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Daniel Baluta <dbaluta@ixiacom.com>
Subject: Re: [PATCH] mm: fix overflow in alloc_vmap_area
Date: Fri, 21 Jun 2013 00:59:38 +0800 [thread overview]
Message-ID: <51C334FA.6080604@gmail.com> (raw)
In-Reply-To: <1371741150-10861-1-git-send-email-unix140@gmail.com>
On 06/20/2013 11:12 PM, Ghennadi Procopciuc wrote:
> Inserting the following kernel module:
>
> <snip>
>
> static int simple_test_init(void)
> {
> size_t i, j;
> void *address;
>
> for (i = 0 * MB; i< 60 * MB; i += 1 * MB) {
> for (j = i; j< i + 1 * MB; j += KB) {
> address = vmalloc(j);
> vfree(address);
> }
> }
>
> return 0;
> }
>
> </snip>
>
> triggers BUG at mm/vmalloc.c:310 on a x86 machine:
>
> [ 95.218283] Kernel BUG at c1126cdb [verbose debug info unavailable]
> [ 95.218306] invalid opcode: 0000 [#1] SMP
> [ 95.218324] Modules linked in: lkma_test(OF+)<snip lots of not tainted modules>
> [ 95.218559] Pid: 2419, comm: insmod Tainted: GF O 3.9.0+ #57 Hewlett-Packard HP Compaq 8200 Elite CMT PC/1494
> [ 95.218597] EIP: 0060:[<c1126cdb>] EFLAGS: 00010207 CPU: 3
> [ 95.218617] EIP is at __insert_vmap_area+0xfb/0x100
> [ 95.218635] EAX: f85dc000 EBX: ef05cac0 ECX: f7be08c4 EDX: 00000007
> [ 95.218657] ESI: f2ed044c EDI: c1a220b8 EBP: f027bd34 ESP: f027bd14
> [ 95.218680] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> [ 95.218699] CR0: 80050033 CR2: b5995118 CR3: 30364000 CR4: 000407f0
> [ 95.218721] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> [ 95.218743] DR6: ffff0ff0 DR7: 00000400
> [ 95.218758] Process insmod (pid: 2419, ti=f027a000 task=efe64010 task.ti=f027a000)
> [ 95.218784] Stack:
> [ 95.218792] c177b964 fecfb000 f85e2000 00000000 f85dc000 ffbfe000 f02da1c0 0007f000
> [ 95.218829] f027bd7c c112802d c177b9b0 fecfb000 01305000 ffbfe000 00000001 ef05cac0
> [ 95.218866] 00000000 00000000 f83fe000 01304fff f83fe000 ffffffff 01305000 ef05c500
> [ 95.218903] Call Trace:
> [ 95.218915] [<f85e2000>] ? 0xf85e1fff
> [ 95.218930] [<f85dc000>] ? 0xf85dbfff
> [ 95.218945] [<c112802d>] alloc_vmap_area.isra.16+0x1bd/0x2f0
> [ 95.218967] [<c112867f>] __get_vm_area_node.isra.17+0x8f/0x130
> [ 95.218988] [<c1128d77>] __vmalloc_node_range+0x57/0x200
> [ 95.219009] [<f85f3075>] ? lkma_test_init+0x45/0x70 [lkma_test]
> [ 95.219031] [<c1128f82>] __vmalloc_node+0x62/0x70
> [ 95.219049] [<f85f3075>] ? lkma_test_init+0x45/0x70 [lkma_test]
> [ 95.219071] [<c1129058>] vmalloc+0x38/0x40
> [ 95.219087] [<f85f3075>] ? lkma_test_init+0x45/0x70 [lkma_test]
> [ 95.219109] [<f85f3075>] lkma_test_init+0x45/0x70 [lkma_test]
> [ 95.219131] [<f85f3030>] ? kzalloc+0x10/0x10 [lkma_test]
> [ 95.219151] [<c1001222>] do_one_initcall+0x112/0x160
> [ 95.219171] [<c15ca3cf>] ? set_section_ro_nx+0x54/0x59
> [ 95.219190] [<c1099b69>] load_module+0x1d79/0x2660
> [ 95.219209] [<c114721d>] ? create_object+0x19d/0x280
> [ 95.219230] [<c109a4c8>] sys_init_module+0x78/0xb0
> [ 95.219250] [<c15d9801>] sysenter_do_call+0x12/0x22
> [ 95.219268] Code: 39 03 73 0c 8b 3f 89 f0 83 c7 08 e9 3d ff ff ff 8b 46 f4 39 43 04 76 13 8b 3f 89 f0 83 c7 04 e9 29 ff ff ff e8 fb 1b 4a 00 eb ab<0f> 0b 8d 76 00 55 89 e5 56 53 66 66 66 66 90 83 60 0c df 89 c6
> [ 95.219415] EIP: [<c1126cdb>] __insert_vmap_area+0xfb/0x100 SS:ESP 0068:f027bd14
> [ 95.228313] ---[ end trace e0a1efb2acb97c98 ]---
>
> A printk placed in __insert_vmap_area will show:
>
> [ 95.218256] va->va_start=0xfecfb000 tmp_va->va_end=0xf85e2000 va->va_end=0 tmp_va->va_start=0xf85dc000
>
> and another one, before sum operation in alloc_vmap_area:
>
> [ 95.218204] addr = 0xfecfb000 size = 19943424 vend = 0xffbfe000
>
> If after addition the result is smaller than one of the arguments,
> then an overflow occurred. In our case there is an obvious overflow.
>
> Signed-off-by: Ghennadi Procopciuc <unix140@gmail.com>
> Cc: Daniel Baluta<dbaluta@ixiacom.com>
>
> ---
> Don't know if this is the right solution, but the bug happens for me in
> 3.10-rc6 and 3.9.
Hello Ghennadi,
Could you please try the below patch to see if it is ok? The patch is based
on today's linus' tree.
------------------------------------------------------
WARNING: multiple messages have this Message-ID (diff)
From: Zhang Yanfei <zhangyanfei.yes@gmail.com>
To: Ghennadi Procopciuc <unix140@gmail.com>
Cc: akpm@linux-foundation.org, js1304@gmail.com, rientjes@google.com,
minchan@kernel.org, kosaki.motohiro@jp.fujitsu.com,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Daniel Baluta <dbaluta@ixiacom.com>
Subject: Re: [PATCH] mm: fix overflow in alloc_vmap_area
Date: Fri, 21 Jun 2013 00:59:38 +0800 [thread overview]
Message-ID: <51C334FA.6080604@gmail.com> (raw)
In-Reply-To: <1371741150-10861-1-git-send-email-unix140@gmail.com>
On 06/20/2013 11:12 PM, Ghennadi Procopciuc wrote:
> Inserting the following kernel module:
>
> <snip>
>
> static int simple_test_init(void)
> {
> size_t i, j;
> void *address;
>
> for (i = 0 * MB; i< 60 * MB; i += 1 * MB) {
> for (j = i; j< i + 1 * MB; j += KB) {
> address = vmalloc(j);
> vfree(address);
> }
> }
>
> return 0;
> }
>
> </snip>
>
> triggers BUG at mm/vmalloc.c:310 on a x86 machine:
>
> [ 95.218283] Kernel BUG at c1126cdb [verbose debug info unavailable]
> [ 95.218306] invalid opcode: 0000 [#1] SMP
> [ 95.218324] Modules linked in: lkma_test(OF+)<snip lots of not tainted modules>
> [ 95.218559] Pid: 2419, comm: insmod Tainted: GF O 3.9.0+ #57 Hewlett-Packard HP Compaq 8200 Elite CMT PC/1494
> [ 95.218597] EIP: 0060:[<c1126cdb>] EFLAGS: 00010207 CPU: 3
> [ 95.218617] EIP is at __insert_vmap_area+0xfb/0x100
> [ 95.218635] EAX: f85dc000 EBX: ef05cac0 ECX: f7be08c4 EDX: 00000007
> [ 95.218657] ESI: f2ed044c EDI: c1a220b8 EBP: f027bd34 ESP: f027bd14
> [ 95.218680] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> [ 95.218699] CR0: 80050033 CR2: b5995118 CR3: 30364000 CR4: 000407f0
> [ 95.218721] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> [ 95.218743] DR6: ffff0ff0 DR7: 00000400
> [ 95.218758] Process insmod (pid: 2419, ti=f027a000 task=efe64010 task.ti=f027a000)
> [ 95.218784] Stack:
> [ 95.218792] c177b964 fecfb000 f85e2000 00000000 f85dc000 ffbfe000 f02da1c0 0007f000
> [ 95.218829] f027bd7c c112802d c177b9b0 fecfb000 01305000 ffbfe000 00000001 ef05cac0
> [ 95.218866] 00000000 00000000 f83fe000 01304fff f83fe000 ffffffff 01305000 ef05c500
> [ 95.218903] Call Trace:
> [ 95.218915] [<f85e2000>] ? 0xf85e1fff
> [ 95.218930] [<f85dc000>] ? 0xf85dbfff
> [ 95.218945] [<c112802d>] alloc_vmap_area.isra.16+0x1bd/0x2f0
> [ 95.218967] [<c112867f>] __get_vm_area_node.isra.17+0x8f/0x130
> [ 95.218988] [<c1128d77>] __vmalloc_node_range+0x57/0x200
> [ 95.219009] [<f85f3075>] ? lkma_test_init+0x45/0x70 [lkma_test]
> [ 95.219031] [<c1128f82>] __vmalloc_node+0x62/0x70
> [ 95.219049] [<f85f3075>] ? lkma_test_init+0x45/0x70 [lkma_test]
> [ 95.219071] [<c1129058>] vmalloc+0x38/0x40
> [ 95.219087] [<f85f3075>] ? lkma_test_init+0x45/0x70 [lkma_test]
> [ 95.219109] [<f85f3075>] lkma_test_init+0x45/0x70 [lkma_test]
> [ 95.219131] [<f85f3030>] ? kzalloc+0x10/0x10 [lkma_test]
> [ 95.219151] [<c1001222>] do_one_initcall+0x112/0x160
> [ 95.219171] [<c15ca3cf>] ? set_section_ro_nx+0x54/0x59
> [ 95.219190] [<c1099b69>] load_module+0x1d79/0x2660
> [ 95.219209] [<c114721d>] ? create_object+0x19d/0x280
> [ 95.219230] [<c109a4c8>] sys_init_module+0x78/0xb0
> [ 95.219250] [<c15d9801>] sysenter_do_call+0x12/0x22
> [ 95.219268] Code: 39 03 73 0c 8b 3f 89 f0 83 c7 08 e9 3d ff ff ff 8b 46 f4 39 43 04 76 13 8b 3f 89 f0 83 c7 04 e9 29 ff ff ff e8 fb 1b 4a 00 eb ab<0f> 0b 8d 76 00 55 89 e5 56 53 66 66 66 66 90 83 60 0c df 89 c6
> [ 95.219415] EIP: [<c1126cdb>] __insert_vmap_area+0xfb/0x100 SS:ESP 0068:f027bd14
> [ 95.228313] ---[ end trace e0a1efb2acb97c98 ]---
>
> A printk placed in __insert_vmap_area will show:
>
> [ 95.218256] va->va_start=0xfecfb000 tmp_va->va_end=0xf85e2000 va->va_end=0 tmp_va->va_start=0xf85dc000
>
> and another one, before sum operation in alloc_vmap_area:
>
> [ 95.218204] addr = 0xfecfb000 size = 19943424 vend = 0xffbfe000
>
> If after addition the result is smaller than one of the arguments,
> then an overflow occurred. In our case there is an obvious overflow.
>
> Signed-off-by: Ghennadi Procopciuc <unix140@gmail.com>
> Cc: Daniel Baluta<dbaluta@ixiacom.com>
>
> ---
> Don't know if this is the right solution, but the bug happens for me in
> 3.10-rc6 and 3.9.
Hello Ghennadi,
Could you please try the below patch to see if it is ok? The patch is based
on today's linus' tree.
------------------------------------------------------
>From d513596c298cb90b6d4defa7a6e839ca2f9467c8 Mon Sep 17 00:00:00 2001
From: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
Date: Fri, 21 Jun 2013 00:37:56 +0800
Subject: [PATCH] mm, vmalloc: Fix a potential overflow bug in alloc_vmap_area
When searching a vmap area in the vmalloc space, we use
(addr + size - 1) to check if the value is less than addr, which
is an overflow. But we assign (addr + size) to vmap_area->va_end.
So if we come across the below case:
(addr + size - 1) : not overflow
(addr + size) : overflow
we will assign an overflow value (e.g 0) to vmap_area->va_end,
And this will trigger BUG in __insert_vmap_area, causing system
panic.
So using (addr + size) to check the overflow should be the correct
behaviour, not (addr + size - 1).
Reported-by: Ghennadi Procopciuc <unix140@gmail.com>
Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
mm/vmalloc.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index d365724..d456560 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -388,12 +388,12 @@ nocache:
addr = ALIGN(first->va_end, align);
if (addr < vstart)
goto nocache;
- if (addr + size - 1 < addr)
+ if (addr + size < addr)
goto overflow;
} else {
addr = ALIGN(vstart, align);
- if (addr + size - 1 < addr)
+ if (addr + size < addr)
goto overflow;
n = vmap_area_root.rb_node;
@@ -420,7 +420,7 @@ nocache:
if (addr + cached_hole_size < first->va_start)
cached_hole_size = first->va_start - addr;
addr = ALIGN(first->va_end, align);
- if (addr + size - 1 < addr)
+ if (addr + size < addr)
goto overflow;
if (list_is_last(&first->list, &vmap_area_list))
--
1.7.1
next prev parent reply other threads:[~2013-06-20 16:59 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-20 15:12 [PATCH] mm: fix overflow in alloc_vmap_area Ghennadi Procopciuc
2013-06-20 15:12 ` Ghennadi Procopciuc
2013-06-20 16:59 ` Zhang Yanfei [this message]
2013-06-20 16:59 ` Zhang Yanfei
2013-06-20 22:51 ` Daniel Baluta
2013-06-20 22:51 ` Daniel Baluta
2013-06-21 8:52 ` Zhang Yanfei
2013-06-21 8:52 ` Zhang Yanfei
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51C334FA.6080604@gmail.com \
--to=zhangyanfei.yes@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=dbaluta@ixiacom.com \
--cc=js1304@gmail.com \
--cc=kosaki.motohiro@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=minchan@kernel.org \
--cc=rientjes@google.com \
--cc=unix140@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.