* [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