Re: [PATCH] memblock: Fix big size with find_region()
From: Yinghai Lu
Date: Wed Oct 06 2010 - 17:57:33 EST
On 10/06/2010 02:06 PM, Andrew Morton wrote:
> On Tue, 28 Sep 2010 01:47:32 -0700
> Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
>
>>
>> When trying to find huge range for crashkernel, get
>>
>> [ 0.000000] ------------[ cut here ]------------
>> [ 0.000000] WARNING: at arch/x86/mm/memblock.c:248 memblock_x86_reserve_range+0x40/0x7a()
>> [ 0.000000] Hardware name: Sun Fire x4800
>> [ 0.000000] memblock_x86_reserve_range: wrong range [0xffffffff37000000, 0x137000000)
>> [ 0.000000] Modules linked in:
>> [ 0.000000] Pid: 0, comm: swapper Not tainted 2.6.36-rc5-tip-yh-01876-g1cac214-dirty #59
>> [ 0.000000] Call Trace:
>> [ 0.000000] [<ffffffff82816f7e>] ? memblock_x86_reserve_range+0x40/0x7a
>> [ 0.000000] [<ffffffff81078c2d>] warn_slowpath_common+0x85/0x9e
>> [ 0.000000] [<ffffffff81078d38>] warn_slowpath_fmt+0x6e/0x70
>> [ 0.000000] [<ffffffff8281e77c>] ? memblock_find_region+0x40/0x78
>> [ 0.000000] [<ffffffff8281eb1f>] ? memblock_find_base+0x9a/0xb9
>> [ 0.000000] [<ffffffff82816f7e>] memblock_x86_reserve_range+0x40/0x7a
>> [ 0.000000] [<ffffffff8280452c>] setup_arch+0x99d/0xb2a
>> [ 0.000000] [<ffffffff810a3e02>] ? trace_hardirqs_off+0xd/0xf
>> [ 0.000000] [<ffffffff81cec7d8>] ? _raw_spin_unlock_irqrestore+0x3d/0x4c
>> [ 0.000000] [<ffffffff827ffcec>] start_kernel+0xde/0x3f1
>> [ 0.000000] [<ffffffff827ff2d4>] x86_64_start_reservations+0xa0/0xa4
>> [ 0.000000] [<ffffffff827ff3de>] x86_64_start_kernel+0x106/0x10d
>> [ 0.000000] ---[ end trace a7919e7f17c0a725 ]---
>> [ 0.000000] Reserving 8192MB of memory at 17592186041200MB for crashkernel (System RAM: 526336MB)
>>
>> Because memblock_find_region() can not handle size > end, base will be set to huge num.
>>
>> Try to check size with end.
>>
>> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
>> ---
>> mm/memblock.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> Index: linux-2.6/mm/memblock.c
>> ===================================================================
>> --- linux-2.6.orig/mm/memblock.c
>> +++ linux-2.6/mm/memblock.c
>> @@ -105,13 +105,18 @@ static phys_addr_t __init memblock_find_
>> phys_addr_t base, res_base;
>> long j;
>>
>> + /* In case, huge size is requested */
>> + if (end < size)
>> + return MEMBLOCK_ERROR;
>> +
>> + base = memblock_align_down((end - size), align);
>
> This seems rather odd. If some caller is passing in size>end then that
> caller is buggy isn't it? A memory block which ends at 0x1000 and has
> a size of 0x2000 is nonsensical.
>
> So shouldn't we at leat emit a warning so tht the offending caller can
> be found and fixed?
hpa already put the patch in tip with new title.
| Commit-ID: f1af98c7629a1b76fd7336decbc776acdeed2120
| Gitweb: http://git.kernel.org/tip/f1af98c7629a1b76fd7336decbc776acdeed2120
| Committer: H. Peter Anvin <hpa@xxxxxxxxx>
| CommitDate: Tue, 5 Oct 2010 21:45:35 -0700
|
| memblock: Fix wraparound in find_region()
Please check following on top of that patch.
Yinghai
[PATCH] memblock: Add input size checking with memblock_find_region()
Make sure two callers have right inputs.
and add print warning to catch other offending callers.
Suggested-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
---
mm/memblock.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Index: linux-2.6/mm/memblock.c
===================================================================
--- linux-2.6.orig/mm/memblock.c
+++ linux-2.6/mm/memblock.c
@@ -106,7 +106,7 @@ static phys_addr_t __init memblock_find_
long j;
/* In case, huge size is requested */
- if (end < size)
+ if (WARN_ONCE(end < size, "memblock_find_region: wrong range [%#llx-%#llx] size %#llx", start, end, size))
return MEMBLOCK_ERROR;
base = memblock_align_down((end - size), align);
@@ -152,14 +152,14 @@ static phys_addr_t __init_memblock membl
phys_addr_t memblocksize = memblock.memory.regions[i].size;
phys_addr_t bottom, top, found;
- if (memblocksize < size)
- continue;
if ((memblockbase + memblocksize) <= start)
break;
bottom = max(memblockbase, start);
top = min(memblockbase + memblocksize, end);
if (bottom >= top)
continue;
+ if ((top - bottom) < size)
+ continue;
found = memblock_find_region(bottom, top, size, align);
if (found != MEMBLOCK_ERROR)
return found;
@@ -547,7 +547,7 @@ static phys_addr_t __init memblock_alloc
int this_nid;
this_end = memblock_nid_range(start, end, &this_nid);
- if (this_nid == nid) {
+ if (this_nid == nid && (this_end - start) >= size) {
phys_addr_t ret = memblock_find_region(start, this_end, size, align);
if (ret != MEMBLOCK_ERROR &&
memblock_add_region(&memblock.reserved, ret, size) >= 0)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/