From: <zijun_hu@htc.com>
To: tj@kernel.org
Cc: akpm@linux-foundation.org, kuleshovmail@gmail.com,
ard.biesheuvel@linaro.org, weiyang@linux.vnet.ibm.com,
dev@g0hl1n.net, david@gibson.dropbear.id.au, linux-mm@kvack.org
Subject: [PATCH] mm/memblock.c: fix index adjustment error in __next_mem_range_rev()
Date: Wed, 27 Jul 2016 07:12:48 +0000 [thread overview]
Message-ID: <42A378E55677204FAE257FE7EED241CB7E8EF15F@CN-MBX01.HTC.COM.TW> (raw)
[-- Attachment #1.1: Type: text/plain, Size: 3999 bytes --]
Hi Tejun,
I find that there is a bug in __next_mem_range_rev() defined in mm/memblock.c
patch 0001 can fix the issue and pass test successfully, please help to review
and phase-in it
patch 0002 is used to verify the solution only and is provided for explaining
test method, please don't apply it
for __next_mem_range_rev(), it not only triggers null deref issue but also
doesn't iterate through memory regions contained in type_a in reversed
order rightly if its parameter type_b == NULL,moreover, it will cause mass
error loops if macro for_each_mem_range_rev is called with parameter
type_b == NULL
the patch 0001 corrects region index idx_a adjustment and initialize idx_b
to 0 to promise getting the last reversed region correctly if parameter
type_b == NULL as showed below
my test method is simple, namely, dump all types of regions with right kernel
interface and fixed __next_mem_range separately ,then check whether
fixed__next_mem_range achieves desired purpose, see test patch
segments below or entire patch 0002 for more info
thanks for Tejun's guidance and helping
fix patch 0001 is showed as follows
From da2f3cafab9632d59261cf0801f62e909d0bfde1 Mon Sep 17 00:00:00 2001
From: zijun_hu <zijun_hu@htc.com>
Date: Mon, 25 Jul 2016 15:06:57 +0800
Subject: [PATCH 1/2] mm/memblock.c: fix index adjustment error in
__next_mem_range_rev()
fix region index adjustment error when parameter type_b of
__next_mem_range_rev() == NULL
Signed-off-by: zijun_hu <zijun_hu@htc.com>
---
mm/memblock.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/mm/memblock.c b/mm/memblock.c
index ac12489..e95f95f 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -991,7 +991,10 @@ void __init_memblock __next_mem_range_rev(u64 *idx, int nid, ulong flags,
if (*idx == (u64)ULLONG_MAX) {
idx_a = type_a->cnt - 1;
- idx_b = type_b->cnt;
+ if (type_b != NULL)
+ idx_b = type_b->cnt;
+ else
+ idx_b = 0;
}
for (; idx_a >= 0; idx_a--) {
@@ -1024,7 +1027,7 @@ void __init_memblock __next_mem_range_rev(u64 *idx, int nid, ulong flags,
*out_end = m_end;
if (out_nid)
*out_nid = m_nid;
- idx_a++;
+ idx_a--;
*idx = (u32)idx_a | (u64)idx_b << 32;
return;
}
--
1.9.1
Test patch 002 code segments is showed as follows
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index d45f862..0db80bb 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -326,6 +326,13 @@ void __init bootmem_init(void)
high_memory = __va((max << PAGE_SHIFT) - 1) + 1;
memblock_dump_all();
+
+ if (!memblock_debug)
+ __memblock_dump_all();
+ /*
+ * extern void memblock_patch_verify(void);
+ */
+ memblock_patch_verify();
}
diff --git a/mm/memblock.c b/mm/memblock.c
index e95f95f..5c179ae 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1652,6 +1652,31 @@ void __init_memblock __memblock_dump_all(void)
memblock_dump(&memblock.reserved, "reserved");
}
+void __init_memblock memblock_patch_verify(void)
+{
+ u64 i;
+ phys_addr_t this_start, this_end;
+
+ pr_info("in %s: memory\n", __func__);
+ for_each_mem_range_rev(i, &memblock.memory, NULL, NUMA_NO_NODE,
+ MEMBLOCK_NONE, &this_start, &this_end, NULL)
+ pr_info("[%#016llx]\t[%#016llx-%#016llx]\n",
+ i, this_start, this_end);
+
+ pr_info("in %s: reserved\n", __func__);
+ for_each_mem_range_rev(i, &memblock.reserved, NULL, NUMA_NO_NODE,
+ MEMBLOCK_NONE, &this_start, &this_end, NULL)
+ pr_info("[%#016llx]\t[%#016llx-%#016llx]\n",
+ i, this_start, this_end);
+
+ pr_info("in %s: memory X reserved\n", __func__);
+ for_each_mem_range_rev(i, &memblock.memory, &memblock.reserved,
+ NUMA_NO_NODE, MEMBLOCK_NONE,
+ &this_start, &this_end, NULL)
+ pr_info("[%#016llx]\t[%#016llx-%#016llx]\n",
+ i, this_start, this_end);
+}
Zijun Hu
F1 Building, 299 Kang Wei Road, Pudong New Area,
Shanghai 201315, China
htc.com
[-- Attachment #1.2: Type: text/html, Size: 6231 bytes --]
[-- Attachment #2: 0001-mm-memblock.c-fix-index-adjustment-error-in.patch --]
[-- Type: application/octet-stream, Size: 1108 bytes --]
From da2f3cafab9632d59261cf0801f62e909d0bfde1 Mon Sep 17 00:00:00 2001
From: zijun_hu <zijun_hu@htc.com>
Date: Mon, 25 Jul 2016 15:06:57 +0800
Subject: [PATCH 1/2] mm/memblock.c: fix index adjustment error in
__next_mem_range_rev()
fix region index adjustment error when parameter type_b of
__next_mem_range_rev() == NULL
Signed-off-by: zijun_hu <zijun_hu@htc.com>
---
mm/memblock.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/mm/memblock.c b/mm/memblock.c
index ac12489..e95f95f 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -991,7 +991,10 @@ void __init_memblock __next_mem_range_rev(u64 *idx, int nid, ulong flags,
if (*idx == (u64)ULLONG_MAX) {
idx_a = type_a->cnt - 1;
- idx_b = type_b->cnt;
+ if (type_b != NULL)
+ idx_b = type_b->cnt;
+ else
+ idx_b = 0;
}
for (; idx_a >= 0; idx_a--) {
@@ -1024,7 +1027,7 @@ void __init_memblock __next_mem_range_rev(u64 *idx, int nid, ulong flags,
*out_end = m_end;
if (out_nid)
*out_nid = m_nid;
- idx_a++;
+ idx_a--;
*idx = (u32)idx_a | (u64)idx_b << 32;
return;
}
--
1.9.1
[-- Attachment #3: 0002-mm-temporary-patch-for-fix-memblock-issue-test.patch --]
[-- Type: application/octet-stream, Size: 2502 bytes --]
From df753d7d9426b4d2a5518958d281be2985ccd40d Mon Sep 17 00:00:00 2001
From: zijun_hu <zijun_hu@htc.com>
Date: Wed, 27 Jul 2016 12:13:37 +0800
Subject: [PATCH 2/2] mm: temporary patch for fix memblock issue test
temporary patch for fix memblock issue test
Signed-off-by: zijun_hu <zijun_hu@htc.com>
---
arch/arm64/mm/init.c | 7 +++++++
include/linux/memblock.h | 1 +
mm/memblock.c | 25 +++++++++++++++++++++++++
3 files changed, 33 insertions(+)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index d45f862..0db80bb 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -326,6 +326,13 @@ void __init bootmem_init(void)
high_memory = __va((max << PAGE_SHIFT) - 1) + 1;
memblock_dump_all();
+
+ if (!memblock_debug)
+ __memblock_dump_all();
+ /*
+ * extern void memblock_patch_verify(void);
+ */
+ memblock_patch_verify();
}
#ifndef CONFIG_SPARSEMEM_VMEMMAP
diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 3106ac1..c62df1e 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -340,6 +340,7 @@ bool memblock_is_reserved(phys_addr_t addr);
bool memblock_is_region_reserved(phys_addr_t base, phys_addr_t size);
extern void __memblock_dump_all(void);
+extern void memblock_patch_verify(void);
static inline void memblock_dump_all(void)
{
diff --git a/mm/memblock.c b/mm/memblock.c
index e95f95f..5c179ae 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1652,6 +1652,31 @@ void __init_memblock __memblock_dump_all(void)
memblock_dump(&memblock.reserved, "reserved");
}
+void __init_memblock memblock_patch_verify(void)
+{
+ u64 i;
+ phys_addr_t this_start, this_end;
+
+ pr_info("in %s: memory\n", __func__);
+ for_each_mem_range_rev(i, &memblock.memory, NULL, NUMA_NO_NODE,
+ MEMBLOCK_NONE, &this_start, &this_end, NULL)
+ pr_info("[%#016llx]\t[%#016llx-%#016llx]\n",
+ i, this_start, this_end);
+
+ pr_info("in %s: reserved\n", __func__);
+ for_each_mem_range_rev(i, &memblock.reserved, NULL, NUMA_NO_NODE,
+ MEMBLOCK_NONE, &this_start, &this_end, NULL)
+ pr_info("[%#016llx]\t[%#016llx-%#016llx]\n",
+ i, this_start, this_end);
+
+ pr_info("in %s: memory X reserved\n", __func__);
+ for_each_mem_range_rev(i, &memblock.memory, &memblock.reserved,
+ NUMA_NO_NODE, MEMBLOCK_NONE,
+ &this_start, &this_end, NULL)
+ pr_info("[%#016llx]\t[%#016llx-%#016llx]\n",
+ i, this_start, this_end);
+}
+
void __init memblock_allow_resize(void)
{
memblock_can_resize = 1;
--
1.9.1
next reply other threads:[~2016-07-27 7:12 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-27 7:12 zijun_hu [this message]
-- strict thread matches above, loose matches on Subject: below --
2016-07-27 6:49 zijun_hu
2016-07-27 17:36 ` Tejun Heo
2016-07-25 7:34 zijun_hu
2016-07-25 18:52 ` Tejun Heo
2016-07-26 15:03 ` zijun_hu
2016-07-26 16:50 ` Tejun Heo
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=42A378E55677204FAE257FE7EED241CB7E8EF15F@CN-MBX01.HTC.COM.TW \
--to=zijun_hu@htc.com \
--cc=akpm@linux-foundation.org \
--cc=ard.biesheuvel@linaro.org \
--cc=david@gibson.dropbear.id.au \
--cc=dev@g0hl1n.net \
--cc=kuleshovmail@gmail.com \
--cc=linux-mm@kvack.org \
--cc=tj@kernel.org \
--cc=weiyang@linux.vnet.ibm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox