Skip to contentSkip to conversation

arm64: ensure __dump_instr() checks addr_limit

PATCH New Mark Rutland 3 participants 3 messages Nov 2, 2017

Subsystems (2)
Stable branch arm64
lore
Share & tools
lore

Mark Rutland

author reviewer
#0 lore raw

PATCH 1/1 arm64: ensure __dump_instr() checks addr_limit

It's possible for a user to deliberately trigger __dump_instr with a chosen kernel address.

Let's avoid problems resulting from this by using get_user() rather than __get_user(), ensuring that we don't erroneously access kernel memory.

Where we use __dump_instr() on kernel text, we already switch to KERNEL_DS, so this shouldn't adversely affect those cases.

Signed-off-by Mark Rutland <[email protected]>
Fixes 60ffc30d5652810d ("arm64: Exception handling")
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc stable at vger.kernel.org
---
 arch/arm64/kernel/traps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/traps.c +1 -1
index 5ea4b85aee0e..8383af15a759 100644
@@ -118,7 +118,7 @@ static void __dump_instr(const char *lvl, struct pt_regs *regs)
 	for (i = -4; i < 1; i++) {
 		unsigned int val, bad;
 
-		bad = __get_user(val, &((u32 *)addr)[i]);
+		bad = get_user(val, &((u32 *)addr)[i]);
 
 		if (!bad)
 			p += sprintf(p, i == 0 ? "(%08x) " : "%08x ", val);
Signature · 1 line
2.11.0
#1 lore raw

PATCH 1/1 arm64: ensure __dump_instr() checks addr_limit

Quoted 9 lines · Mark Rutland

It's possible for a user to deliberately trigger __dump_instr with a chosen kernel address.

Let's avoid problems resulting from this by using get_user() rather than __get_user(), ensuring that we don't erroneously access kernel memory.

Where we use __dump_instr() on kernel text, we already switch to KERNEL_DS, so this shouldn't adversely affect those cases.

Quoted 8 lines
Signed-off-by Mark Rutland <[email protected]>
Fixes 60ffc30d5652810d ("arm64: Exception handling")
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc [email protected]
---
 arch/arm64/kernel/traps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by Will Deacon <[email protected]>

Catalin, can you take this as a fix please?

Will

Quoted 16 lines
arch/arm64/kernel/traps.c +1 -1
index 5ea4b85aee0e..8383af15a759 100644
@@ -118,7 +118,7 @@ static void __dump_instr(const char *lvl, struct pt_regs *regs)
 	for (i = -4; i < 1; i++) {
 		unsigned int val, bad;
 
-		bad = __get_user(val, &((u32 *)addr)[i]);
+		bad = get_user(val, &((u32 *)addr)[i]);
 
 		if (!bad)
 			p += sprintf(p, i == 0 ? "(%08x) " : "%08x ", val);
Signature · 1 line
2.11.0
maintainer Applied
#2 lore raw

PATCH 1/1 arm64: ensure __dump_instr() checks addr_limit

In reply to Mark Rutland
Quoted 6 lines · Mark Rutland

It's possible for a user to deliberately trigger __dump_instr with a chosen kernel address.

Let's avoid problems resulting from this by using get_user() rather than __get_user(), ensuring that we don't erroneously access kernel memory.

Quoted 8 lines

Where we use __dump_instr() on kernel text, we already switch to KERNEL_DS, so this shouldn't adversely affect those cases.

Signed-off-by Mark Rutland <[email protected]>
Fixes 60ffc30d5652810d ("arm64: Exception handling")
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc stable at vger.kernel.org

Queued for 4.14. Thanks.

Signature · 1 line
Catalin