Re: [PATCH] x86, e820: panic on sanitizing invalid memory map
On 10/17/2014 09:41 PM, Martin Kelly wrote: > sanitize_e820_map returns two possible values: > -1: Returned when either the provided memory map has length 1 (ok) or > when the provided memory map is invalid (not ok). > 0: Returned when the memory map was correctly sanitized. > > In addition, most code ignores the returned value, and none actually > handles it (except possibly by panicking). > > This patch changes the behavior so that sanitize_e820_map is a void > function. When the provided memory map has length 1 or it is sanitized > (both ok cases), it returns nothing. If the provided memory map is > invalid, then it panics. > > Signed-off-by: Martin Kelly > --- > arch/x86/include/asm/e820.h | 2 +- > arch/x86/kernel/e820.c | 95 > ++--- > 2 files changed, 47 insertions(+), 50 deletions(-) > > diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h > index 779c2ef..739f8db 100644 > --- a/arch/x86/include/asm/e820.h > +++ b/arch/x86/include/asm/e820.h > @@ -18,7 +18,7 @@ extern int e820_any_mapped(u64 start, u64 end, unsigned > type); > extern int e820_all_mapped(u64 start, u64 end, unsigned type); > extern void e820_add_region(u64 start, u64 size, int type); > extern void e820_print_map(char *who); > -extern int > +extern void > sanitize_e820_map(struct e820entry *biosmap, int max_nr_map, u32 *pnr_map); > extern u64 e820_update_range(u64 start, u64 size, unsigned old_type, > unsigned new_type); > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c > index 49f8864..96ad559 100644 > --- a/arch/x86/kernel/e820.c > +++ b/arch/x86/kernel/e820.c > @@ -188,47 +188,48 @@ void __init e820_print_map(char *who) > * be updated on return, with the new number of valid entries > * (something no more than max_nr_map.) > * > - * The return value from sanitize_e820_map() is zero if it > - * successfully 'sanitized' the map entries passed in, and is -1 > - * if it did nothing, which can happen if either of (1) it was > - * only passed one map entry, or (2) any of the input map entries > - * were invalid (start + size < start, meaning that the size was > - * so big the described memory range wrapped around through zero.) > + * There are three possible actions that sanitize_e820_map() can take: > + * (1) If the map entry count is 1, do nothing and return. > + * (2) If any of the input map entries were invalid > + * (start + size < start), then the size was so big that the > described > + * memory range wrapped around through zero. In this case, panic. > + * (3) If the map entry count is greater than 1 and the map is valid, > + * sanitize the map and return. > * > - * Visually we're performing the following > - * (1,2,3,4 = memory types)... > + * Visually we're performing the following > + * (1,2,3,4 = memory types)... > * > - * Sample memory map (w/overlaps): > - * 22__ > - * __4_ > - * > - * _44_ > - * > - * 33__ > - * ___44___ > - * __3_ > - * __22 > - * ____ > - * _1__ > - * _11_ > - * _4__ > + * Sample memory map (w/overlaps): > + *22__ > + *__4_ > + * > + *_44_ > + * > + *33__ > + *___44___ > + *__3_ > + *__22 > + *____ > + *_1__ > + *_11_ > + *_4__ > * > - * Sanitized equivalent (no overlap): > - * 1___ > - * _44_ > - * ___1 > - * 22__ > - * __11 > - * _1__ > - * __3_ > - * ___44___ > - * _33_ > - * ___2 > - * 1___ > - * _4__ > - * ___2 > - * 33__ > - * __4_ > + * Sanitized equivalent (no overlap): > + *1___ > + *_44_ > + *___1 > + *22__ > + *__11 > + *_1__ > + *__3_ > + *___44___ > + *_33_ > + *___2 > + *1___ > + *_4__ > + *___2 > + *
[PATCH] x86, e820: panic on sanitizing invalid memory map
sanitize_e820_map returns two possible values:
-1: Returned when either the provided memory map has length 1 (ok) or
when the provided memory map is invalid (not ok).
0: Returned when the memory map was correctly sanitized.
In addition, most code ignores the returned value, and none actually
handles it (except possibly by panicking).
This patch changes the behavior so that sanitize_e820_map is a void
function. When the provided memory map has length 1 or it is sanitized
(both ok cases), it returns nothing. If the provided memory map is
invalid, then it panics.
Signed-off-by: Martin Kelly
---
arch/x86/include/asm/e820.h | 2 +-
arch/x86/kernel/e820.c | 95 ++---
2 files changed, 47 insertions(+), 50 deletions(-)
diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h
index 779c2ef..739f8db 100644
--- a/arch/x86/include/asm/e820.h
+++ b/arch/x86/include/asm/e820.h
@@ -18,7 +18,7 @@ extern int e820_any_mapped(u64 start, u64 end, unsigned type);
extern int e820_all_mapped(u64 start, u64 end, unsigned type);
extern void e820_add_region(u64 start, u64 size, int type);
extern void e820_print_map(char *who);
-extern int
+extern void
sanitize_e820_map(struct e820entry *biosmap, int max_nr_map, u32 *pnr_map);
extern u64 e820_update_range(u64 start, u64 size, unsigned old_type,
unsigned new_type);
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 49f8864..96ad559 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -188,47 +188,48 @@ void __init e820_print_map(char *who)
* be updated on return, with the new number of valid entries
* (something no more than max_nr_map.)
*
- * The return value from sanitize_e820_map() is zero if it
- * successfully 'sanitized' the map entries passed in, and is -1
- * if it did nothing, which can happen if either of (1) it was
- * only passed one map entry, or (2) any of the input map entries
- * were invalid (start + size < start, meaning that the size was
- * so big the described memory range wrapped around through zero.)
+ * There are three possible actions that sanitize_e820_map() can take:
+ * (1) If the map entry count is 1, do nothing and return.
+ * (2) If any of the input map entries were invalid
+ * (start + size < start), then the size was so big that the described
+ * memory range wrapped around through zero. In this case, panic.
+ * (3) If the map entry count is greater than 1 and the map is valid,
+ * sanitize the map and return.
*
- * Visually we're performing the following
- * (1,2,3,4 = memory types)...
+ * Visually we're performing the following
+ * (1,2,3,4 = memory types)...
*
- * Sample memory map (w/overlaps):
- *22__
- *__4_
- *
- *_44_
- *
- *33__
- *___44___
- *__3_
- *__22
- *____
- *_1__
- *_11_
- *_4__
+ * Sample memory map (w/overlaps):
+ *22__
+ *__4_
+ *
+ *_44_
+ *
+ *33__
+ *___44___
+ *__3_
+ *__22
+ *____
+ *_1__
+ *_11_
+ *_4__
*
- * Sanitized equivalent (no overlap):
- *1___
- *_44_
- *___1
- *22__
- *__11
- *_1__
- *__3_
- *___44___
- *_33_
- *___2
- *1___
- *_4__
- *___2
- *33__
- *__4_
+ * Sanitized equivalent (no overlap):
+ *1___
+ *_44_
+ *___1
+ *22__
+ *__11
+ *_1__
+ *__3_
+ *___44___
+ *_33_
+ *___2
+ *1___
+ *_4__
+ *___2
+ *33__
+ *__4_
*/
struct change_member {
struct e820entry *pbios; /* pointer to original bios entry */
@@ -252,7 +253,7 @@ static int __init cpcompare(const void *a, const void *
