* [PATCH] memblock: Fix big size with find_region()
@ 2010-09-28 8:47 Yinghai Lu
2010-10-06 21:06 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Yinghai Lu @ 2010-09-28 8:47 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Ingo Molnar, H. Peter Anvin,
Thomas Gleixner
Cc: linux-kernel@vger.kernel.org
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@kernel.org>
---
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);
+
/* Prevent allocations returning 0 as it's also used to
* indicate an allocation failure
*/
if (start == 0)
start = PAGE_SIZE;
- base = memblock_align_down((end - size), align);
while (start <= base) {
j = memblock_overlaps_region(&memblock.reserved, base, size);
if (j < 0)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memblock: Fix big size with find_region()
2010-09-28 8:47 [PATCH] memblock: Fix big size with find_region() Yinghai Lu
@ 2010-10-06 21:06 ` Andrew Morton
2010-10-06 21:55 ` Yinghai Lu
2010-10-06 22:12 ` H. Peter Anvin
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2010-10-06 21:06 UTC (permalink / raw)
To: Yinghai Lu
Cc: Benjamin Herrenschmidt, Ingo Molnar, H. Peter Anvin,
Thomas Gleixner, linux-kernel@vger.kernel.org
On Tue, 28 Sep 2010 01:47:32 -0700
Yinghai Lu <yinghai@kernel.org> 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@kernel.org>
> ---
> 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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memblock: Fix big size with find_region()
2010-10-06 21:06 ` Andrew Morton
@ 2010-10-06 21:55 ` Yinghai Lu
2010-10-06 22:12 ` H. Peter Anvin
1 sibling, 0 replies; 6+ messages in thread
From: Yinghai Lu @ 2010-10-06 21:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Benjamin Herrenschmidt, Ingo Molnar, H. Peter Anvin,
Thomas Gleixner, linux-kernel@vger.kernel.org
On 10/06/2010 02:06 PM, Andrew Morton wrote:
> On Tue, 28 Sep 2010 01:47:32 -0700
> Yinghai Lu <yinghai@kernel.org> 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@kernel.org>
>> ---
>> 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@zytor.com>
| 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@linux-foundation.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memblock: Fix big size with find_region()
2010-10-06 21:06 ` Andrew Morton
2010-10-06 21:55 ` Yinghai Lu
@ 2010-10-06 22:12 ` H. Peter Anvin
2010-10-06 22:45 ` Andrew Morton
1 sibling, 1 reply; 6+ messages in thread
From: H. Peter Anvin @ 2010-10-06 22:12 UTC (permalink / raw)
To: Andrew Morton
Cc: Yinghai Lu, Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner,
linux-kernel@vger.kernel.org
On 10/06/2010 02:06 PM, Andrew Morton wrote:
>
> 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?
>
I don't think this is necessarily a bug in the caller -- it just
indicates that it has a request that is impossible to fulfill -- but
that can happen for a lot of other reasons.
Keep in mind that the range and the size will typically come from
different origins. As such, it's not clear to me that this is something
that requires printing a specific error message for any more than any
other allocation failure, but perhaps you disagree?
-hpa
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memblock: Fix big size with find_region()
2010-10-06 22:12 ` H. Peter Anvin
@ 2010-10-06 22:45 ` Andrew Morton
2010-10-06 22:47 ` H. Peter Anvin
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2010-10-06 22:45 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Yinghai Lu, Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner,
linux-kernel@vger.kernel.org
On Wed, 06 Oct 2010 15:12:35 -0700
"H. Peter Anvin" <hpa@zytor.com> wrote:
> On 10/06/2010 02:06 PM, Andrew Morton wrote:
> >
> > 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?
> >
>
> I don't think this is necessarily a bug in the caller -- it just
> indicates that it has a request that is impossible to fulfill -- but
> that can happen for a lot of other reasons.
>
> Keep in mind that the range and the size will typically come from
> different origins. As such, it's not clear to me that this is something
> that requires printing a specific error message for any more than any
> other allocation failure, but perhaps you disagree?
>
whomeidunno. The question is "is this something we want to know
about". If it's a BIOS/acpi/whatever error then I'd think "yes",
because that would then lead to useful kernel workarounds or BIOS
updates?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memblock: Fix big size with find_region()
2010-10-06 22:45 ` Andrew Morton
@ 2010-10-06 22:47 ` H. Peter Anvin
0 siblings, 0 replies; 6+ messages in thread
From: H. Peter Anvin @ 2010-10-06 22:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Yinghai Lu, Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner,
linux-kernel@vger.kernel.org
On 10/06/2010 03:45 PM, Andrew Morton wrote:
>
> whomeidunno. The question is "is this something we want to know
> about". If it's a BIOS/acpi/whatever error then I'd think "yes",
> because that would then lead to useful kernel workarounds or BIOS
> updates?
I don't think it's inherently that... it could be in some cases, but not
in others. In either case, an allocation failure should be handled in
the calling layer... or we have a much more serious problem.
However, I suspect the most common case where we'll hit this is when
something wants to fit inside a chunk where it simply doesn't fit ...
and it'll either move on to the next plausible chunk or fail at that point.
-hpa
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-06 22:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-28 8:47 [PATCH] memblock: Fix big size with find_region() Yinghai Lu
2010-10-06 21:06 ` Andrew Morton
2010-10-06 21:55 ` Yinghai Lu
2010-10-06 22:12 ` H. Peter Anvin
2010-10-06 22:45 ` Andrew Morton
2010-10-06 22:47 ` H. Peter Anvin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox