Re: [PATCH 1/1] arm: mmu: catch NULL pointer dereferences

2013-09-04 Thread Sascha Hauer 
On Mon, Sep 02, 2013 at 12:24:06PM +0200, Jean-Christophe PLAGNIOL-VILLARD 
wrote:
 On 11:05 Mon 02 Sep , Sascha Hauer wrote:
  On Sat, Aug 31, 2013 at 05:54:22PM +0200, Jean-Christophe PLAGNIOL-VILLARD 
  wrote:
   For high vectors if memory start at 0x0. We have to
   live without being able to catch NULL pointer dereferences.
   
  Why not add the check to vectors_init() instead of passing this as
  argument?
  
  I must say I'm not entirely happy with this approach. Being able to
  catch NULL pointer derefs is really a good thing. When exactly does the
  faulting zero SDRAM page become a problem for you? obviously something
  like
  
  cp /somehing /dev/ram0
  
  does not work. Some care must probably be taken when setting up the
  kernel parameters. Are these the problems or is there something else?
 yes it's get issue with uImage that get data load there :(
 
 as I can not control the load addr from uImage :(
 
 and on ux5x0 and arm board I have ddr starting at 0x0

Maybe we should add some hook which dynamically enables/disables the
zero page. The bootm code could then disable it.

Anyway, for now we can also apply the current patch, but please move the
is_start_zero test to vectors_init().

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/1] arm: mmu: catch NULL pointer dereferences

2013-09-02 Thread Sascha Hauer 
On Sat, Aug 31, 2013 at 05:54:22PM +0200, Jean-Christophe PLAGNIOL-VILLARD 
wrote:
 For high vectors if memory start at 0x0. We have to
 live without being able to catch NULL pointer dereferences.
 
 Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
 ---
  arch/arm/cpu/mmu.c | 27 +--
  1 file changed, 21 insertions(+), 6 deletions(-)
 
 diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
 index e3ea3b6..2e69b16 100644
 --- a/arch/arm/cpu/mmu.c
 +++ b/arch/arm/cpu/mmu.c
 @@ -218,9 +218,9 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
  /*
   * Map vectors and zero page
   */
 -static void vectors_init(void)
 +static void vectors_init(bool zero_valid_addr)
  {
 - u32 *exc, *zero = NULL;
 + u32 *exc;
   void *vectors;
   u32 cr;
  
 @@ -235,10 +235,19 @@ static void vectors_init(void)
* page table for the high vectors and zero page
*/
   exc = arm_create_pte(0xfff0);
 - zero = arm_create_pte(0x0);
  
 - /* Set the zero page to faulting */
 - zero[0] = 0;
 + /*
 +  * Memory start at 0x0. We have to
 +  * live without being able to catch NULL pointer dereferences
 +  */
 + if (!zero_valid_addr) {
 + u32 *zero = NULL;
 +
 + zero = arm_create_pte(0x0);
 +
 + /* Set the zero page to faulting */
 + zero[0] = 0;
 + }
   } else {
   /*
* Otherwise map the vectors to the zero page. We have to
 @@ -267,6 +276,7 @@ static int mmu_init(void)
  {
   struct memory_bank *bank;
   int i;
 + bool is_start_zero = 0;
  
   arm_set_cache_functions();
  
 @@ -303,7 +313,12 @@ static int mmu_init(void)
   create_sections(0, 0, PAGE_SIZE, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
   PMD_TYPE_SECT);
  
 - vectors_init();
 + for_each_memory_bank(bank) {
 + if (bank-start == 0x0)
 + is_start_zero = 1;
 + }
 +
 + vectors_init(is_start_zero);

Why not add the check to vectors_init() instead of passing this as
argument?

I must say I'm not entirely happy with this approach. Being able to
catch NULL pointer derefs is really a good thing. When exactly does the
faulting zero SDRAM page become a problem for you? obviously something
like

cp /somehing /dev/ram0

does not work. Some care must probably be taken when setting up the
kernel parameters. Are these the problems or is there something else?

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/1] arm: mmu: catch NULL pointer dereferences

2013-09-02 Thread Jean-Christophe PLAGNIOL-VILLARD 
On 11:05 Mon 02 Sep , Sascha Hauer wrote:
 On Sat, Aug 31, 2013 at 05:54:22PM +0200, Jean-Christophe PLAGNIOL-VILLARD 
 wrote:
  For high vectors if memory start at 0x0. We have to
  live without being able to catch NULL pointer dereferences.
  
  Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
  ---
   arch/arm/cpu/mmu.c | 27 +--
   1 file changed, 21 insertions(+), 6 deletions(-)
  
  diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
  index e3ea3b6..2e69b16 100644
  --- a/arch/arm/cpu/mmu.c
  +++ b/arch/arm/cpu/mmu.c
  @@ -218,9 +218,9 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
   /*
* Map vectors and zero page
*/
  -static void vectors_init(void)
  +static void vectors_init(bool zero_valid_addr)
   {
  -   u32 *exc, *zero = NULL;
  +   u32 *exc;
  void *vectors;
  u32 cr;
   
  @@ -235,10 +235,19 @@ static void vectors_init(void)
   * page table for the high vectors and zero page
   */
  exc = arm_create_pte(0xfff0);
  -   zero = arm_create_pte(0x0);
   
  -   /* Set the zero page to faulting */
  -   zero[0] = 0;
  +   /*
  +* Memory start at 0x0. We have to
  +* live without being able to catch NULL pointer dereferences
  +*/
  +   if (!zero_valid_addr) {
  +   u32 *zero = NULL;
  +
  +   zero = arm_create_pte(0x0);
  +
  +   /* Set the zero page to faulting */
  +   zero[0] = 0;
  +   }
  } else {
  /*
   * Otherwise map the vectors to the zero page. We have to
  @@ -267,6 +276,7 @@ static int mmu_init(void)
   {
  struct memory_bank *bank;
  int i;
  +   bool is_start_zero = 0;
   
  arm_set_cache_functions();
   
  @@ -303,7 +313,12 @@ static int mmu_init(void)
  create_sections(0, 0, PAGE_SIZE, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
  PMD_TYPE_SECT);
   
  -   vectors_init();
  +   for_each_memory_bank(bank) {
  +   if (bank-start == 0x0)
  +   is_start_zero = 1;
  +   }
  +
  +   vectors_init(is_start_zero);
 
 Why not add the check to vectors_init() instead of passing this as
 argument?
 
 I must say I'm not entirely happy with this approach. Being able to
 catch NULL pointer derefs is really a good thing. When exactly does the
 faulting zero SDRAM page become a problem for you? obviously something
 like
 
 cp /somehing /dev/ram0
 
 does not work. Some care must probably be taken when setting up the
 kernel parameters. Are these the problems or is there something else?
yes it's get issue with uImage that get data load there :(

as I can not control the load addr from uImage :(

and on ux5x0 and arm board I have ddr starting at 0x0

Best Regards,
J.

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/1] arm: mmu: catch NULL pointer dereferences

2013-08-31 Thread Jean-Christophe PLAGNIOL-VILLARD 
For high vectors if memory start at 0x0. We have to
live without being able to catch NULL pointer dereferences.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
---
 arch/arm/cpu/mmu.c | 27 +--
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index e3ea3b6..2e69b16 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -218,9 +218,9 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
 /*
  * Map vectors and zero page
  */
-static void vectors_init(void)
+static void vectors_init(bool zero_valid_addr)
 {
-   u32 *exc, *zero = NULL;
+   u32 *exc;
void *vectors;
u32 cr;
 
@@ -235,10 +235,19 @@ static void vectors_init(void)
 * page table for the high vectors and zero page
 */
exc = arm_create_pte(0xfff0);
-   zero = arm_create_pte(0x0);
 
-   /* Set the zero page to faulting */
-   zero[0] = 0;
+   /*
+* Memory start at 0x0. We have to
+* live without being able to catch NULL pointer dereferences
+*/
+   if (!zero_valid_addr) {
+   u32 *zero = NULL;
+
+   zero = arm_create_pte(0x0);
+
+   /* Set the zero page to faulting */
+   zero[0] = 0;
+   }
} else {
/*
 * Otherwise map the vectors to the zero page. We have to
@@ -267,6 +276,7 @@ static int mmu_init(void)
 {
struct memory_bank *bank;
int i;
+   bool is_start_zero = 0;
 
arm_set_cache_functions();
 
@@ -303,7 +313,12 @@ static int mmu_init(void)
create_sections(0, 0, PAGE_SIZE, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
PMD_TYPE_SECT);
 
-   vectors_init();
+   for_each_memory_bank(bank) {
+   if (bank-start == 0x0)
+   is_start_zero = 1;
+   }
+
+   vectors_init(is_start_zero);
 
/*
 * First remap sdram cached using sections.
-- 
1.8.4.rc1


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox