[PATCH] [ARM] sharpsl_param: fix invalid memory access in sharpsl_save_param()

Subsystems: arm port, the rest

STALE4984d

3 messages, 2 authors, 2012-12-13 · open the first message on its own page

[PATCH] [ARM] sharpsl_param: fix invalid memory access in sharpsl_save_param()

From: dromede at gmail.com <hidden>
Date: 2012-12-13 14:44:53

From: Marko Katic <dromede.gmail.com>

Devices that call sharpsl_save_param() will hang on boot due to
a memcpy call that uses physical addresses that are no longer
valid. This is caused by the following commit:

commit 72662e01088394577be4a3f14da94cf87bea2591
Author: Will Deacon [off-list ref]
Date:   Wed Nov 23 12:03:27 2011 +0000

    ARM: head.S: only include __turn_mmu_on in the initial identity mapping

Tested  on Zaurus corgi/poodle/spitz devices.

Signed-off-by: Marko Katic <redacted>
---
 arch/arm/common/sharpsl_param.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm/common/sharpsl_param.c b/arch/arm/common/sharpsl_param.c
index d56c932..04555dc 100644
--- a/arch/arm/common/sharpsl_param.c
+++ b/arch/arm/common/sharpsl_param.c
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/string.h>
+#include <asm/memory.h>
 #include <asm/mach/sharpsl_param.h>
 
 /*
@@ -41,7 +42,8 @@ EXPORT_SYMBOL(sharpsl_param);
 
 void sharpsl_save_param(void)
 {
-	memcpy(&sharpsl_param, (void *)PARAM_BASE, sizeof(struct sharpsl_param_info));
+	void *param_start = phys_to_virt(PARAM_BASE);
+	memcpy(&sharpsl_param, param_start, sizeof(struct sharpsl_param_info));
 
 	if (sharpsl_param.comadj_keyword != COMADJ_MAGIC)
 		sharpsl_param.comadj=-1;
-- 
1.7.10.4

[PATCH] [ARM] sharpsl_param: fix invalid memory access in sharpsl_save_param()

From: Will Deacon <hidden>
Date: 2012-12-13 14:50:12

Hi Marko,

On Thu, Dec 13, 2012 at 02:44:53PM +0000, dromede at gmail.com wrote:
From: Marko Katic <dromede.gmail.com>

Devices that call sharpsl_save_param() will hang on boot due to
a memcpy call that uses physical addresses that are no longer
valid. This is caused by the following commit:

commit 72662e01088394577be4a3f14da94cf87bea2591
Author: Will Deacon [off-list ref]
Date:   Wed Nov 23 12:03:27 2011 +0000

    ARM: head.S: only include __turn_mmu_on in the initial identity mapping

Tested  on Zaurus corgi/poodle/spitz devices.

Signed-off-by: Marko Katic <redacted>
Interesting find! Now, I wonder, if you revert Dave's patch conditionalising
the hyp mode entry (1ecec696c8bb "ARM: 7599/1: head: Remove boot-time HYP
mode check for v5 and below") and apply your patch below, does your i2c
still work?

Cheers,

Will
quoted hunk
---
 arch/arm/common/sharpsl_param.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm/common/sharpsl_param.c b/arch/arm/common/sharpsl_param.c
index d56c932..04555dc 100644
--- a/arch/arm/common/sharpsl_param.c
+++ b/arch/arm/common/sharpsl_param.c
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/string.h>
+#include <asm/memory.h>
 #include <asm/mach/sharpsl_param.h>
 
 /*
@@ -41,7 +42,8 @@ EXPORT_SYMBOL(sharpsl_param);
 
 void sharpsl_save_param(void)
 {
-	memcpy(&sharpsl_param, (void *)PARAM_BASE, sizeof(struct sharpsl_param_info));
+	void *param_start = phys_to_virt(PARAM_BASE);
+	memcpy(&sharpsl_param, param_start, sizeof(struct sharpsl_param_info));
 
 	if (sharpsl_param.comadj_keyword != COMADJ_MAGIC)
 		sharpsl_param.comadj=-1;
-- 
1.7.10.4

[PATCH] [ARM] sharpsl_param: fix invalid memory access in sharpsl_save_param()

From: Marko Katić <hidden>
Date: 2012-12-13 15:54:05

On Thu, Dec 13, 2012 at 3:50 PM, Will Deacon [off-list ref] wrote:
Hi Marko,

On Thu, Dec 13, 2012 at 02:44:53PM +0000, dromede at gmail.com wrote:
quoted
From: Marko Katic <dromede.gmail.com>

Devices that call sharpsl_save_param() will hang on boot due to
a memcpy call that uses physical addresses that are no longer
valid. This is caused by the following commit:

commit 72662e01088394577be4a3f14da94cf87bea2591
Author: Will Deacon [off-list ref]
Date:   Wed Nov 23 12:03:27 2011 +0000

    ARM: head.S: only include __turn_mmu_on in the initial identity mapping

Tested  on Zaurus corgi/poodle/spitz devices.

Signed-off-by: Marko Katic <redacted>
Interesting find! Now, I wonder, if you revert Dave's patch conditionalising
the hyp mode entry (1ecec696c8bb "ARM: 7599/1: head: Remove boot-time HYP
mode check for v5 and below") and apply your patch below, does your i2c
still work?
If i revert 1ecec696c8bb my machine simply would not boot. This was
nicely explained by Russell in this post:

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/136218.html

In a nutshell, there never was a problem with i2c devices, the problem
was the kernel
booted with the wrong machine ID  and thus machine_is_xxx() checks
would fail and break
i2c init code in mach-pxa/spitz.c.

Sharpsl_param.c patch is a separate issue and is not related to issues
mentioned above.
This is also and older issue, dating back to 3.3-rc1 when 72662e0108
was commited. I posted
this patch earlier but either nobody noticed or my commit message was
bad. Probably the latter.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help