* [PATCH] kexec-tools, ppc64: Fix segfault parsing DR memory property
@ 2010-05-11 8:07 Matt Evans
2010-05-12 2:50 ` Michael Neuling
0 siblings, 1 reply; 2+ messages in thread
From: Matt Evans @ 2010-05-11 8:07 UTC (permalink / raw)
To: kexec; +Cc: linuxppc-dev, horms
add_dyn_reconf_usable_mem_property() iterates over memory spans
in /ibm,dynamic-reconfiguration-memory/ibm,dynamic-memory and intersects
these with usablemem_rgns ranges. Not only did it seem to write
null properties for every range that didn't match, but it used an unchecked
fixed-size array which will overrun on machines with lots of LMBs.
This patch stops add_dyn_reconf_usable_mem_property() from adding null ranges
to the linux,drconf-usable-memory property and removes its fixed-size array,
as well as the array in add_usable_mem_property, in lieu of malloc/realloc/free.
Signed-off-by: Matt Evans <matt@ozlabs.org>
---
kexec/arch/ppc64/fs2dt.c | 66 +++++++++++++++++++++++++++++++++++++--------
1 files changed, 54 insertions(+), 12 deletions(-)
diff --git a/kexec/arch/ppc64/fs2dt.c b/kexec/arch/ppc64/fs2dt.c
index 762bf04..7470132 100644
--- a/kexec/arch/ppc64/fs2dt.c
+++ b/kexec/arch/ppc64/fs2dt.c
@@ -37,7 +37,7 @@
#define NAMESPACE 16384 /* max bytes for property names */
#define INIT_TREE_WORDS 65536 /* Initial num words for prop values */
#define MEMRESERVE 256 /* max number of reserved memory blocks */
-#define MAX_MEMORY_RANGES 1024
+#define MEM_RANGE_CHUNK_SZ 2048 /* Initial num dwords for mem ranges */
static char pathname[MAXPATH], *pathstart;
static char propnames[NAMESPACE] = { 0 };
@@ -148,7 +148,8 @@ static void add_dyn_reconf_usable_mem_property(int fd)
{
char fname[MAXPATH], *bname;
uint64_t buf[32];
- uint64_t ranges[2*MAX_MEMORY_RANGES];
+ uint64_t *ranges;
+ int ranges_size = MEM_RANGE_CHUNK_SZ;
uint64_t base, end, loc_base, loc_end;
size_t i, rngs_cnt, range;
int rlen = 0;
@@ -165,6 +166,11 @@ static void add_dyn_reconf_usable_mem_property(int fd)
die("unrecoverable error: error seeking in \"%s\": %s\n",
pathname, strerror(errno));
+ ranges = malloc(ranges_size*8);
+ if (!ranges)
+ die("unrecoverable error: can't alloc %d bytes for ranges.\n",
+ ranges_size*8);
+
rlen = 0;
for (i = 0; i < num_of_lmbs; i++) {
if (read(fd, buf, 24) < 0)
@@ -180,24 +186,41 @@ static void add_dyn_reconf_usable_mem_property(int fd)
rngs_cnt = 0;
for (range = 0; range < usablemem_rgns.size; range++) {
+ int add = 0;
loc_base = usablemem_rgns.ranges[range].start;
loc_end = usablemem_rgns.ranges[range].end;
if (loc_base >= base && loc_end <= end) {
- ranges[rlen++] = loc_base;
- ranges[rlen++] = loc_end - loc_base;
- rngs_cnt++;
+ add = 1;
} else if (base < loc_end && end > loc_base) {
if (loc_base < base)
loc_base = base;
if (loc_end > end)
loc_end = end;
+ add = 1;
+ }
+
+ if (add) {
+ if (rlen >= (ranges_size-2)) {
+ ranges_size += MEM_RANGE_CHUNK_SZ;
+ ranges = realloc(ranges, ranges_size*8);
+ if (!ranges)
+ die("unrecoverable error: can't"
+ " realloc %d bytes for"
+ " ranges.\n",
+ ranges_size*8);
+ }
ranges[rlen++] = loc_base;
ranges[rlen++] = loc_end - loc_base;
rngs_cnt++;
}
}
- /* Store the count of (base, size) duple */
- ranges[tmp_indx] = rngs_cnt;
+ if (rngs_cnt == 0) {
+ /* Don't store anything for unwritten iterations! */
+ rlen = tmp_indx;
+ } else {
+ /* Store the count of (base, size) duple */
+ ranges[tmp_indx] = rngs_cnt;
+ }
}
rlen = rlen * sizeof(uint64_t);
@@ -210,7 +233,8 @@ static void add_dyn_reconf_usable_mem_property(int fd)
*dt++ = propnum("linux,drconf-usable-memory");
if ((rlen >= 8) && ((unsigned long)dt & 0x4))
dt++;
- memcpy(dt, &ranges, rlen);
+ memcpy(dt, ranges, rlen);
+ free(ranges);
dt += (rlen + 3)/4;
}
@@ -218,7 +242,8 @@ static void add_usable_mem_property(int fd, size_t len)
{
char fname[MAXPATH], *bname;
uint64_t buf[2];
- uint64_t ranges[2*MAX_MEMORY_RANGES];
+ uint64_t *ranges;
+ int ranges_size = MEM_RANGE_CHUNK_SZ;
uint64_t base, end, loc_base, loc_end;
size_t range;
int rlen = 0;
@@ -247,17 +272,33 @@ static void add_usable_mem_property(int fd, size_t len)
base = buf[0];
end = base + buf[1];
+ ranges = malloc(ranges_size*8);
+ if (!ranges)
+ die("unrecoverable error: can't alloc %d bytes for ranges.\n",
+ ranges_size*8);
+
for (range = 0; range < usablemem_rgns.size; range++) {
+ int add = 0;
loc_base = usablemem_rgns.ranges[range].start;
loc_end = usablemem_rgns.ranges[range].end;
if (loc_base >= base && loc_end <= end) {
- ranges[rlen++] = loc_base;
- ranges[rlen++] = loc_end - loc_base;
+ add = 1;
} else if (base < loc_end && end > loc_base) {
if (loc_base < base)
loc_base = base;
if (loc_end > end)
loc_end = end;
+ add = 1;
+ }
+ if (add) {
+ if (rlen >= (ranges_size-2)) {
+ ranges_size += MEM_RANGE_CHUNK_SZ;
+ ranges = realloc(ranges, ranges_size*8);
+ if (!ranges)
+ die("unrecoverable error: can't realloc"
+ "%d bytes for ranges.\n",
+ ranges_size*8);
+ }
ranges[rlen++] = loc_base;
ranges[rlen++] = loc_end - loc_base;
}
@@ -283,7 +324,8 @@ static void add_usable_mem_property(int fd, size_t len)
*dt++ = propnum("linux,usable-memory");
if ((rlen >= 8) && ((unsigned long)dt & 0x4))
dt++;
- memcpy(dt,&ranges,rlen);
+ memcpy(dt, ranges, rlen);
+ free(ranges);
dt += (rlen + 3)/4;
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] kexec-tools, ppc64: Fix segfault parsing DR memory property
2010-05-11 8:07 [PATCH] kexec-tools, ppc64: Fix segfault parsing DR memory property Matt Evans
@ 2010-05-12 2:50 ` Michael Neuling
0 siblings, 0 replies; 2+ messages in thread
From: Michael Neuling @ 2010-05-12 2:50 UTC (permalink / raw)
To: Matt Evans; +Cc: linuxppc-dev, horms, kexec
In message <4BE9102C.6040404@axio.ms> you wrote:
>
> add_dyn_reconf_usable_mem_property() iterates over memory spans
> in /ibm,dynamic-reconfiguration-memory/ibm,dynamic-memory and intersects
> these with usablemem_rgns ranges. Not only did it seem to write
> null properties for every range that didn't match, but it used an unchecked
> fixed-size array which will overrun on machines with lots of LMBs.
>
> This patch stops add_dyn_reconf_usable_mem_property() from adding null ranges
> to the linux,drconf-usable-memory property and removes its fixed-size array,
> as well as the array in add_usable_mem_property, in lieu of malloc/realloc/fr
ee.
>
Thanks matt!
Acked-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Matt Evans <matt@ozlabs.org>
> ---
> kexec/arch/ppc64/fs2dt.c | 66 +++++++++++++++++++++++++++++++++++++-------
-
> 1 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/kexec/arch/ppc64/fs2dt.c b/kexec/arch/ppc64/fs2dt.c
> index 762bf04..7470132 100644
> --- a/kexec/arch/ppc64/fs2dt.c
> +++ b/kexec/arch/ppc64/fs2dt.c
> @@ -37,7 +37,7 @@
> #define NAMESPACE 16384 /* max bytes for property names */
> #define INIT_TREE_WORDS 65536 /* Initial num words for prop values */
> #define MEMRESERVE 256 /* max number of reserved memory blocks
*/
> -#define MAX_MEMORY_RANGES 1024
> +#define MEM_RANGE_CHUNK_SZ 2048 /* Initial num dwords for mem ranges */
>
> static char pathname[MAXPATH], *pathstart;
> static char propnames[NAMESPACE] = { 0 };
> @@ -148,7 +148,8 @@ static void add_dyn_reconf_usable_mem_property(int fd)
> {
> char fname[MAXPATH], *bname;
> uint64_t buf[32];
> - uint64_t ranges[2*MAX_MEMORY_RANGES];
> + uint64_t *ranges;
> + int ranges_size = MEM_RANGE_CHUNK_SZ;
> uint64_t base, end, loc_base, loc_end;
> size_t i, rngs_cnt, range;
> int rlen = 0;
> @@ -165,6 +166,11 @@ static void add_dyn_reconf_usable_mem_property(int fd)
> die("unrecoverable error: error seeking in \"%s\": %s\n",
> pathname, strerror(errno));
>
> + ranges = malloc(ranges_size*8);
> + if (!ranges)
> + die("unrecoverable error: can't alloc %d bytes for ranges.\n",
> + ranges_size*8);
> +
> rlen = 0;
> for (i = 0; i < num_of_lmbs; i++) {
> if (read(fd, buf, 24) < 0)
> @@ -180,24 +186,41 @@ static void add_dyn_reconf_usable_mem_property(int fd)
>
> rngs_cnt = 0;
> for (range = 0; range < usablemem_rgns.size; range++) {
> + int add = 0;
> loc_base = usablemem_rgns.ranges[range].start;
> loc_end = usablemem_rgns.ranges[range].end;
> if (loc_base >= base && loc_end <= end) {
> - ranges[rlen++] = loc_base;
> - ranges[rlen++] = loc_end - loc_base;
> - rngs_cnt++;
> + add = 1;
> } else if (base < loc_end && end > loc_base) {
> if (loc_base < base)
> loc_base = base;
> if (loc_end > end)
> loc_end = end;
> + add = 1;
> + }
> +
> + if (add) {
> + if (rlen >= (ranges_size-2)) {
> + ranges_size += MEM_RANGE_CHUNK_SZ;
> + ranges = realloc(ranges, ranges_size*8)
;
> + if (!ranges)
> + die("unrecoverable error: can't
"
> + " realloc %d bytes for"
> + " ranges.\n",
> + ranges_size*8);
> + }
> ranges[rlen++] = loc_base;
> ranges[rlen++] = loc_end - loc_base;
> rngs_cnt++;
> }
> }
> - /* Store the count of (base, size) duple */
> - ranges[tmp_indx] = rngs_cnt;
> + if (rngs_cnt == 0) {
> + /* Don't store anything for unwritten iterations! */
> + rlen = tmp_indx;
> + } else {
> + /* Store the count of (base, size) duple */
> + ranges[tmp_indx] = rngs_cnt;
> + }
> }
>
> rlen = rlen * sizeof(uint64_t);
> @@ -210,7 +233,8 @@ static void add_dyn_reconf_usable_mem_property(int fd)
> *dt++ = propnum("linux,drconf-usable-memory");
> if ((rlen >= 8) && ((unsigned long)dt & 0x4))
> dt++;
> - memcpy(dt, &ranges, rlen);
> + memcpy(dt, ranges, rlen);
> + free(ranges);
> dt += (rlen + 3)/4;
> }
>
> @@ -218,7 +242,8 @@ static void add_usable_mem_property(int fd, size_t len)
> {
> char fname[MAXPATH], *bname;
> uint64_t buf[2];
> - uint64_t ranges[2*MAX_MEMORY_RANGES];
> + uint64_t *ranges;
> + int ranges_size = MEM_RANGE_CHUNK_SZ;
> uint64_t base, end, loc_base, loc_end;
> size_t range;
> int rlen = 0;
> @@ -247,17 +272,33 @@ static void add_usable_mem_property(int fd, size_t len)
> base = buf[0];
> end = base + buf[1];
>
> + ranges = malloc(ranges_size*8);
> + if (!ranges)
> + die("unrecoverable error: can't alloc %d bytes for ranges.\n",
> + ranges_size*8);
> +
> for (range = 0; range < usablemem_rgns.size; range++) {
> + int add = 0;
> loc_base = usablemem_rgns.ranges[range].start;
> loc_end = usablemem_rgns.ranges[range].end;
> if (loc_base >= base && loc_end <= end) {
> - ranges[rlen++] = loc_base;
> - ranges[rlen++] = loc_end - loc_base;
> + add = 1;
> } else if (base < loc_end && end > loc_base) {
> if (loc_base < base)
> loc_base = base;
> if (loc_end > end)
> loc_end = end;
> + add = 1;
> + }
> + if (add) {
> + if (rlen >= (ranges_size-2)) {
> + ranges_size += MEM_RANGE_CHUNK_SZ;
> + ranges = realloc(ranges, ranges_size*8);
> + if (!ranges)
> + die("unrecoverable error: can't realloc
"
> + "%d bytes for ranges.\n",
> + ranges_size*8);
> + }
> ranges[rlen++] = loc_base;
> ranges[rlen++] = loc_end - loc_base;
> }
> @@ -283,7 +324,8 @@ static void add_usable_mem_property(int fd, size_t len)
> *dt++ = propnum("linux,usable-memory");
> if ((rlen >= 8) && ((unsigned long)dt & 0x4))
> dt++;
> - memcpy(dt,&ranges,rlen);
> + memcpy(dt, ranges, rlen);
> + free(ranges);
> dt += (rlen + 3)/4;
> }
>
> --
> 1.6.3.3
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2010-05-12 2:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-11 8:07 [PATCH] kexec-tools, ppc64: Fix segfault parsing DR memory property Matt Evans
2010-05-12 2:50 ` Michael Neuling
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).