[PATCH 04/11] Define the virtual space of KASan's shadow region

[Abbott Liu]

Define KASAN_SHADOW_OFFSET,KASAN_SHADOW_START and KASAN_SHADOW_END for arm
kernel address sanitizer.

+----+ 0xffffffff
| |
| |
| |
+----+ CONFIG_PAGE_OFFSET
| |\
| | |-> module virtual address space area.
| |/
+----+ MODULE_VADDR = KASAN_SHADOW_END
| |\
| | |-> the shadow area of kernel virtual address.
| |/
+----+ TASK_SIZE(start of kernel space) = KASAN_SHADOW_START the shadow address of MODULE_VADDR
| |\
| | ---------------------+
| | |
+ + KASAN_SHADOW_OFFSET |-> the user space area. Kernel address sanitizer do not use this space.
| | |
| | ---------------------+
| |/
------ 0

1)KASAN_SHADOW_OFFSET:
This value is used to map an address to the corresponding shadow address by the
following formula:
shadow_addr = (address >> 3) + KASAN_SHADOW_OFFSET;

2)KASAN_SHADOW_START
This value is the MODULE_VADDR's shadow address. It is the start of kernel virtual
space.

3) KASAN_SHADOW_END
This value is the 0x100000000's shadow address. It is the end of kernel address
sanitizer's shadow area. It is also the start of the module area.

Cc: Andrey Ryabinin <a.rya...@samsung.com>
---
arch/arm/include/asm/kasan_def.h | 51 ++++++++++++++++++++++++++++++++++++++++
arch/arm/include/asm/memory.h | 5 ++++
arch/arm/kernel/entry-armv.S | 7 +++++-
3 files changed, 62 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/include/asm/kasan_def.h

diff --git a/arch/arm/include/asm/kasan_def.h b/arch/arm/include/asm/kasan_def.h
new file mode 100644
index 0000000..7746908
--- /dev/null
+++ b/arch/arm/include/asm/kasan_def.h
@@ -0,0 +1,51 @@
+#ifndef __ASM_KASAN_DEF_H
+#define __ASM_KASAN_DEF_H
+
+#ifdef CONFIG_KASAN
+
+/*
+ * +----+ 0xffffffff
+ * | |
+ * | |
+ * | |
+ * +----+ CONFIG_PAGE_OFFSET
+ * | |\
+ * | | |-> module virtual address space area.
+ * | |/
+ * +----+ MODULE_VADDR = KASAN_SHADOW_END
+ * | |\
+ * | | |-> the shadow area of kernel virtual address.
+ * | |/
+ * +----+ TASK_SIZE(start of kernel space) = KASAN_SHADOW_START the shadow address of MODULE_VADDR
+ * | |\
+ * | | ---------------------+
+ * | | |
+ * + + KASAN_SHADOW_OFFSET |-> the user space area. Kernel address sanitizer do not use this space.
+ * | | |
+ * | | ---------------------+
+ * | |/
+ * ------ 0
+ *
+ *1)KASAN_SHADOW_OFFSET:
+ * This value is used to map an address to the corresponding shadow address by the
+ * following formula:
+ * shadow_addr = (address >> 3) + KASAN_SHADOW_OFFSET;
+ *
+ * 2)KASAN_SHADOW_START
+ * This value is the MODULE_VADDR's shadow address. It is the start of kernel virtual
+ * space.
+ *
+ * 3) KASAN_SHADOW_END
+ * This value is the 0x100000000's shadow address. It is the end of kernel address
+ * sanitizer's shadow area. It is also the start of the module area.
+ *
+ */
+
+#define KASAN_SHADOW_OFFSET (KASAN_SHADOW_END - (1<<29))
+
+#define KASAN_SHADOW_START ((KASAN_SHADOW_END >> 3) + KASAN_SHADOW_OFFSET)
+
+#define KASAN_SHADOW_END (UL(CONFIG_PAGE_OFFSET) - UL(SZ_16M))
+
+#endif
+#endif
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 1f54e4e..069710d 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -21,6 +21,7 @@
#ifdef CONFIG_NEED_MACH_MEMORY_H
#include <mach/memory.h>
#endif
+#include <asm/kasan_def.h>

/*
* Allow for constants defined here to be used from assembly code
@@ -37,7 +38,11 @@
* TASK_SIZE - the maximum size of a user space task.
* TASK_UNMAPPED_BASE - the lower boundary of the mmap VM area
*/
+#ifndef CONFIG_KASAN
#define TASK_SIZE (UL(CONFIG_PAGE_OFFSET) - UL(SZ_16M))
+#else
+#define TASK_SIZE (KASAN_SHADOW_START)
+#endif
#define TASK_UNMAPPED_BASE ALIGN(TASK_SIZE / 3, SZ_16M)

/*
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index fbc7076..f9efea3 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -187,7 +187,12 @@ ENDPROC(__und_invalid)

get_thread_info tsk
ldr r0, [tsk, #TI_ADDR_LIMIT]
- mov r1, #TASK_SIZE
+#ifdef CONFIG_KASAN
+ movw r1, #:lower16:TASK_SIZE
+ movt r1, #:upper16:TASK_SIZE
+#else
+ mov r1, #TASK_SIZE
+#endif
str r1, [tsk, #TI_ADDR_LIMIT]
str r0, [sp, #SVC_ADDR_LIMIT]

--
2.9.0

[kbuild test robot]

Hi Abbott,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.14-rc4]
[cannot apply to next-20171013]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Abbott-Liu/KASan-for-arm/20171014-104108
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm

All errors (new ones prefixed by >>):

arch/arm/kernel/entry-common.S: Assembler messages:
>> arch/arm/kernel/entry-common.S:83: Error: invalid constant (ffffffffb6e00000) after fixup
arch/arm/kernel/entry-common.S:118: Error: invalid constant (ffffffffb6e00000) after fixup
--
arch/arm/kernel/entry-armv.S: Assembler messages:
>> arch/arm/kernel/entry-armv.S:213: Error: selected processor does not support `movw r1,#:lower16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
>> arch/arm/kernel/entry-armv.S:213: Error: selected processor does not support `movt r1,#:upper16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
arch/arm/kernel/entry-armv.S:223: Error: selected processor does not support `movw r1,#:lower16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
arch/arm/kernel/entry-armv.S:223: Error: selected processor does not support `movt r1,#:upper16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
arch/arm/kernel/entry-armv.S:270: Error: selected processor does not support `movw r1,#:lower16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
arch/arm/kernel/entry-armv.S:270: Error: selected processor does not support `movt r1,#:upper16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
arch/arm/kernel/entry-armv.S:311: Error: selected processor does not support `movw r1,#:lower16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
arch/arm/kernel/entry-armv.S:311: Error: selected processor does not support `movt r1,#:upper16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
arch/arm/kernel/entry-armv.S:320: Error: selected processor does not support `movw r1,#:lower16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
arch/arm/kernel/entry-armv.S:320: Error: selected processor does not support `movt r1,#:upper16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
arch/arm/kernel/entry-armv.S:348: Error: selected processor does not support `movw r1,#:lower16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
arch/arm/kernel/entry-armv.S:348: Error: selected processor does not support `movt r1,#:upper16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode

vim +213 arch/arm/kernel/entry-armv.S

2dede2d8e Nicolas Pitre 2006-01-14 151
2190fed67 Russell King 2015-08-20 152 .macro svc_entry, stack_hole=0, trace=1, uaccess=1
c4c5716e1 Catalin Marinas 2009-02-16 153 UNWIND(.fnstart )
c4c5716e1 Catalin Marinas 2009-02-16 154 UNWIND(.save {r0 - pc} )
e6a9dc612 Russell King 2016-05-13 155 sub sp, sp, #(SVC_REGS_SIZE + \stack_hole - 4)
b86040a59 Catalin Marinas 2009-07-24 156 #ifdef CONFIG_THUMB2_KERNEL
b86040a59 Catalin Marinas 2009-07-24 157 SPFIX( str r0, [sp] ) @ temporarily saved
b86040a59 Catalin Marinas 2009-07-24 158 SPFIX( mov r0, sp )
b86040a59 Catalin Marinas 2009-07-24 159 SPFIX( tst r0, #4 ) @ test original stack alignment
b86040a59 Catalin Marinas 2009-07-24 160 SPFIX( ldr r0, [sp] ) @ restored
b86040a59 Catalin Marinas 2009-07-24 161 #else
2dede2d8e Nicolas Pitre 2006-01-14 162 SPFIX( tst sp, #4 )
b86040a59 Catalin Marinas 2009-07-24 163 #endif
b86040a59 Catalin Marinas 2009-07-24 164 SPFIX( subeq sp, sp, #4 )
b86040a59 Catalin Marinas 2009-07-24 165 stmia sp, {r1 - r12}
ccea7a19e Russell King 2005-05-31 166
b059bdc39 Russell King 2011-06-25 167 ldmia r0, {r3 - r5}
b059bdc39 Russell King 2011-06-25 168 add r7, sp, #S_SP - 4 @ here for interlock avoidance
b059bdc39 Russell King 2011-06-25 169 mov r6, #-1 @ "" "" "" ""
e6a9dc612 Russell King 2016-05-13 170 add r2, sp, #(SVC_REGS_SIZE + \stack_hole - 4)
b059bdc39 Russell King 2011-06-25 171 SPFIX( addeq r2, r2, #4 )
b059bdc39 Russell King 2011-06-25 172 str r3, [sp, #-4]! @ save the "real" r0 copied
ccea7a19e Russell King 2005-05-31 173 @ from the exception stack
ccea7a19e Russell King 2005-05-31 174
b059bdc39 Russell King 2011-06-25 175 mov r3, lr
^1da177e4 Linus Torvalds 2005-04-16 176
^1da177e4 Linus Torvalds 2005-04-16 177 @
^1da177e4 Linus Torvalds 2005-04-16 178 @ We are now ready to fill in the remaining blanks on the stack:
^1da177e4 Linus Torvalds 2005-04-16 179 @
b059bdc39 Russell King 2011-06-25 180 @ r2 - sp_svc
b059bdc39 Russell King 2011-06-25 181 @ r3 - lr_svc
b059bdc39 Russell King 2011-06-25 182 @ r4 - lr_<exception>, already fixed up for correct return/restart
b059bdc39 Russell King 2011-06-25 183 @ r5 - spsr_<exception>
b059bdc39 Russell King 2011-06-25 184 @ r6 - orig_r0 (see pt_regs definition in ptrace.h)
^1da177e4 Linus Torvalds 2005-04-16 185 @
b059bdc39 Russell King 2011-06-25 186 stmia r7, {r2 - r6}
^1da177e4 Linus Torvalds 2005-04-16 187
e6978e4bf Russell King 2016-05-13 188 get_thread_info tsk
e6978e4bf Russell King 2016-05-13 189 ldr r0, [tsk, #TI_ADDR_LIMIT]
74e552f98 Abbott Liu 2017-10-11 190 #ifdef CONFIG_KASAN
74e552f98 Abbott Liu 2017-10-11 191 movw r1, #:lower16:TASK_SIZE
74e552f98 Abbott Liu 2017-10-11 192 movt r1, #:upper16:TASK_SIZE
74e552f98 Abbott Liu 2017-10-11 193 #else
e6978e4bf Russell King 2016-05-13 194 mov r1, #TASK_SIZE
74e552f98 Abbott Liu 2017-10-11 195 #endif
e6978e4bf Russell King 2016-05-13 196 str r1, [tsk, #TI_ADDR_LIMIT]
e6978e4bf Russell King 2016-05-13 197 str r0, [sp, #SVC_ADDR_LIMIT]
e6978e4bf Russell King 2016-05-13 198
2190fed67 Russell King 2015-08-20 199 uaccess_save r0
2190fed67 Russell King 2015-08-20 200 .if \uaccess
2190fed67 Russell King 2015-08-20 201 uaccess_disable r0
2190fed67 Russell King 2015-08-20 202 .endif
2190fed67 Russell King 2015-08-20 203
c0e7f7ee7 Daniel Thompson 2014-09-17 204 .if \trace
02fe2845d Russell King 2011-06-25 205 #ifdef CONFIG_TRACE_IRQFLAGS
02fe2845d Russell King 2011-06-25 206 bl trace_hardirqs_off
02fe2845d Russell King 2011-06-25 207 #endif
c0e7f7ee7 Daniel Thompson 2014-09-17 208 .endif
f2741b78b Russell King 2011-06-25 209 .endm
^1da177e4 Linus Torvalds 2005-04-16 210
f2741b78b Russell King 2011-06-25 211 .align 5
f2741b78b Russell King 2011-06-25 212 __dabt_svc:
2190fed67 Russell King 2015-08-20 @213 svc_entry uaccess=0
^1da177e4 Linus Torvalds 2005-04-16 214 mov r2, sp
da7404725 Russell King 2011-06-26 215 dabt_helper
e16b31bf4 Marc Zyngier 2013-11-04 216 THUMB( ldr r5, [sp, #S_PSR] ) @ potentially updated CPSR
b059bdc39 Russell King 2011-06-25 217 svc_exit r5 @ return from exception
c4c5716e1 Catalin Marinas 2009-02-16 218 UNWIND(.fnend )
93ed39701 Catalin Marinas 2008-08-28 219 ENDPROC(__dabt_svc)
^1da177e4 Linus Torvalds 2005-04-16 220
^1da177e4 Linus Torvalds 2005-04-16 221 .align 5
^1da177e4 Linus Torvalds 2005-04-16 222 __irq_svc:
ccea7a19e Russell King 2005-05-31 223 svc_entry
187a51ad1 Russell King 2005-05-21 224 irq_handler
1613cc111 Russell King 2011-06-25 225

:::::: The code at line 213 was first introduced by commit
:::::: 2190fed67ba6f3e8129513929f2395843645e928 ARM: entry: provide uaccess assembly macro hooks

:::::: TO: Russell King <rmk+k...@arm.linux.org.uk>
:::::: CC: Russell King <rmk+k...@arm.linux.org.uk>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

[Liuwenliang (Lamb)]

On 10/16/2017 07:03 PM, Abbott Liu wrote:
>arch/arm/kernel/entry-armv.S:348: Error: selected processor does not support `movw r1,
#:lower16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
>arch/arm/kernel/entry-armv.S:348: Error: selected processor does not support `movt r1,

#:upper16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode

Thanks for building test. This error can be solved by following code:
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -188,8 +188,7 @@ ENDPROC(__und_invalid)

get_thread_info tsk
ldr r0, [tsk, #TI_ADDR_LIMIT]

#ifdef CONFIG_KASAN
- movw r1, #:lower16:TASK_SIZE
- movt r1, #:upper16:TASK_SIZE
+ ldr r1, =TASK_SIZE
#else
mov r1, #TASK_SIZE
#endif
@@ -446,7 +445,12 @@ ENDPROC(__fiq_abt)
@ if it was interrupted in a critical region. Here we
@ perform a quick test inline since it should be false
@ 99.9999% of the time. The rest is done out of line.
+#if CONFIG_KASAN
+ ldr r0, =TASK_SIZE
+ cmp r4, r0
+#else
cmp r4, #TASK_SIZE
+#endif
blhs kuser_cmpxchg64_fixup
#endif
#endif

movt,movw can only be used in ARMv6*, ARMv7 instruction set. But ldr can be used in ARMv4*, ARMv5T*, ARMv6*, ARMv7.
Maybe the performance is going to fall down by using ldr, but I think the influence of performance is very limited.

[Ard Biesheuvel]

On 16 October 2017 at 12:42, Liuwenliang (Lamb) <liuwe...@huawei.com> wrote:
> On 10/16/2017 07:03 PM, Abbott Liu wrote:
>>arch/arm/kernel/entry-armv.S:348: Error: selected processor does not support `movw r1,
> #:lower16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
>>arch/arm/kernel/entry-armv.S:348: Error: selected processor does not support `movt r1,
> #:upper16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
>
> Thanks for building test. This error can be solved by following code:
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -188,8 +188,7 @@ ENDPROC(__und_invalid)
> get_thread_info tsk
> ldr r0, [tsk, #TI_ADDR_LIMIT]
> #ifdef CONFIG_KASAN
> - movw r1, #:lower16:TASK_SIZE
> - movt r1, #:upper16:TASK_SIZE
> + ldr r1, =TASK_SIZE
> #else
> mov r1, #TASK_SIZE
> #endif

This is unnecessary:

ldr r1, =TASK_SIZE

will be converted to a mov instruction by the assembler if the value
of TASK_SIZE fits its 12-bit immediate field.

So please remove the whole #ifdef, and just use ldr r1, =xxx

[Liuwenliang (Lamb)]

On 10/17/2017 12:40 AM, Abbott Liu wrote:
> Ard Biesheuvel [ard.bie...@linaro.org] wrote

>This is unnecessary:
>
>ldr r1, =TASK_SIZE
>
>will be converted to a mov instruction by the assembler if the value of TASK_SIZE fits its 12-bit immediate field.
>
>So please remove the whole #ifdef, and just use ldr r1, =xxx

Thanks for your review.

The assembler on my computer don't convert ldr r1,=xxx into mov instruction. Here is the objdump for vmlinux:

c0a3b100 <__irq_svc>:
c0a3b100: e24dd04c sub sp, sp, #76 ; 0x4c
c0a3b104: e31d0004 tst sp, #4
c0a3b108: 024dd004 subeq sp, sp, #4
c0a3b10c: e88d1ffe stm sp, {r1, r2, r3, r4, r5, r6, r7, r8, r9, sl, fp, ip}
c0a3b110: e8900038 ldm r0, {r3, r4, r5}
c0a3b114: e28d7030 add r7, sp, #48 ; 0x30
c0a3b118: e3e06000 mvn r6, #0
c0a3b11c: e28d204c add r2, sp, #76 ; 0x4c
c0a3b120: 02822004 addeq r2, r2, #4
c0a3b124: e52d3004 push {r3} ; (str r3, [sp, #-4]!)
c0a3b128: e1a0300e mov r3, lr
c0a3b12c: e887007c stm r7, {r2, r3, r4, r5, r6}
c0a3b130: e1a0972d lsr r9, sp, #14
c0a3b134: e1a09709 lsl r9, r9, #14
c0a3b138: e5990008 ldr r0, [r9, #8]
---c0a3b13c: e59f1054 ldr r1, [pc, #84] ; c0a3b198 <__irq_svc+0x98> //ldr r1, =TASK_SIZE
c0a3b140: e5891008 str r1, [r9, #8]
c0a3b144: e58d004c str r0, [sp, #76] ; 0x4c
c0a3b148: ee130f10 mrc 15, 0, r0, cr3, cr0, {0}
c0a3b14c: e58d0048 str r0, [sp, #72] ; 0x48
c0a3b150: e3a00051 mov r0, #81 ; 0x51
c0a3b154: ee030f10 mcr 15, 0, r0, cr3, cr0, {0}
---c0a3b158: e59f103c ldr r1, [pc, #60] ; c0a3b19c <__irq_svc+0x9c> //orginal irq_svc also used same instruction
c0a3b15c: e1a0000d mov r0, sp
c0a3b160: e28fe000 add lr, pc, #0
c0a3b164: e591f000 ldr pc, [r1]
c0a3b168: e5998004 ldr r8, [r9, #4]
c0a3b16c: e5990000 ldr r0, [r9]
c0a3b170: e3380000 teq r8, #0
c0a3b174: 13a00000 movne r0, #0
c0a3b178: e3100002 tst r0, #2
c0a3b17c: 1b000007 blne c0a3b1a0 <svc_preempt>
c0a3b180: e59d104c ldr r1, [sp, #76] ; 0x4c
c0a3b184: e59d0048 ldr r0, [sp, #72] ; 0x48
c0a3b188: ee030f10 mcr 15, 0, r0, cr3, cr0, {0}
c0a3b18c: e5891008 str r1, [r9, #8]
c0a3b190: e16ff005 msr SPSR_fsxc, r5
c0a3b194: e8ddffff ldm sp, {r0, r1, r2, r3, r4, r5, r6, r7, r8, r9, sl, fp, ip, sp, lr, pc}^
---c0a3b198: b6e00000 .word 0xb6e00000 //TASK_SIZE:0xb6e00000
c0a3b19c: c0ccccf0 .word 0xc0ccccf0



Even "ldr r1, =TASK_SIZE" won't be converted to a mov instruction by some assembler, I also think it is better
to remove the whole #ifdef because the influence of performance by ldr is very limited.

[Ard Biesheuvel]

On 17 October 2017 at 12:27, Liuwenliang (Lamb) <liuwe...@huawei.com> wrote:
> On 10/17/2017 12:40 AM, Abbott Liu wrote:
>> Ard Biesheuvel [ard.bie...@linaro.org] wrote
>>This is unnecessary:
>>
>>ldr r1, =TASK_SIZE
>>
>>will be converted to a mov instruction by the assembler if the value of TASK_SIZE fits its 12-bit immediate field.
>>
>>So please remove the whole #ifdef, and just use ldr r1, =xxx
>
> Thanks for your review.
>
> The assembler on my computer don't convert ldr r1,=xxx into mov instruction.

What I said was

'if the value of TASK_SIZE fits its 12-bit immediate field'

and your value of TASK_SIZE is 0xb6e00000, which cannot be decomposed
in the right way.

If you build with KASAN disabled, it will generate a mov instruction instead.

[Liuwenliang (Lamb)]

On 10/17/2017 8:45 PM, Abbott Liu wrote:
>What I said was
>
>'if the value of TASK_SIZE fits its 12-bit immediate field'
>
>and your value of TASK_SIZE is 0xb6e00000, which cannot be decomposed in the right way.
>
>If you build with KASAN disabled, it will generate a mov instruction instead.

Thanks for your explain. I understand now. I has tested and the testing result proves that what
you said is right.

Here is test log:
c010e9e0 <__irq_svc>:
c010e9e0: e24dd04c sub sp, sp, #76 ; 0x4c
c010e9e4: e31d0004 tst sp, #4
c010e9e8: 024dd004 subeq sp, sp, #4
c010e9ec: e88d1ffe stm sp, {r1, r2, r3, r4, r5, r6, r7, r8, r9, sl, fp, ip}
c010e9f0: e8900038 ldm r0, {r3, r4, r5}
c010e9f4: e28d7030 add r7, sp, #48 ; 0x30
c010e9f8: e3e06000 mvn r6, #0
c010e9fc: e28d204c add r2, sp, #76 ; 0x4c
c010ea00: 02822004 addeq r2, r2, #4
c010ea04: e52d3004 push {r3} ; (str r3, [sp, #-4]!)
c010ea08: e1a0300e mov r3, lr
c010ea0c: e887007c stm r7, {r2, r3, r4, r5, r6}
c010ea10: e1a0972d lsr r9, sp, #14
c010ea14: e1a09709 lsl r9, r9, #14
c010ea18: e5990008 ldr r0, [r9, #8]
c010ea1c: e3a014bf mov r1, #-1090519040 ; 0xbf000000 // ldr r1,=0xbf000000

[Russell King - ARM Linux]

On Mon, Oct 16, 2017 at 11:42:05AM +0000, Liuwenliang (Lamb) wrote:
> On 10/16/2017 07:03 PM, Abbott Liu wrote:
> >arch/arm/kernel/entry-armv.S:348: Error: selected processor does not support `movw r1,
> #:lower16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
> >arch/arm/kernel/entry-armv.S:348: Error: selected processor does not support `movt r1,
> #:upper16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
>
> Thanks for building test. This error can be solved by following code:
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -188,8 +188,7 @@ ENDPROC(__und_invalid)
> get_thread_info tsk
> ldr r0, [tsk, #TI_ADDR_LIMIT]
> #ifdef CONFIG_KASAN
> - movw r1, #:lower16:TASK_SIZE
> - movt r1, #:upper16:TASK_SIZE
> + ldr r1, =TASK_SIZE
> #else
> mov r1, #TASK_SIZE
> #endif

We can surely do better than this with macros and condition support -
we can build-time test in the assembler whether TASK_SIZE can fit in a
normal "mov", whether we can use the movw/movt instructions, or fall
back to ldr if necessary. I'd rather we avoided "ldr" here where
possible.

> @@ -446,7 +445,12 @@ ENDPROC(__fiq_abt)
> @ if it was interrupted in a critical region. Here we
> @ perform a quick test inline since it should be false
> @ 99.9999% of the time. The rest is done out of line.
> +#if CONFIG_KASAN
> + ldr r0, =TASK_SIZE
> + cmp r4, r0
> +#else
> cmp r4, #TASK_SIZE

Same sort of thing goes for here - we can select the instruction at
runtime using the assembler's macros and condition support.

We know that TASK_SIZE is going to be one of a limited set of values.

--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[Russell King - ARM Linux]

On Mon, Oct 16, 2017 at 01:14:54PM +0100, Ard Biesheuvel wrote:
> On 16 October 2017 at 12:42, Liuwenliang (Lamb) <liuwe...@huawei.com> wrote:
> > On 10/16/2017 07:03 PM, Abbott Liu wrote:
> >>arch/arm/kernel/entry-armv.S:348: Error: selected processor does not support `movw r1,
> > #:lower16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
> >>arch/arm/kernel/entry-armv.S:348: Error: selected processor does not support `movt r1,
> > #:upper16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
> >
> > Thanks for building test. This error can be solved by following code:
> > --- a/arch/arm/kernel/entry-armv.S
> > +++ b/arch/arm/kernel/entry-armv.S
> > @@ -188,8 +188,7 @@ ENDPROC(__und_invalid)
> > get_thread_info tsk
> > ldr r0, [tsk, #TI_ADDR_LIMIT]
> > #ifdef CONFIG_KASAN
> > - movw r1, #:lower16:TASK_SIZE
> > - movt r1, #:upper16:TASK_SIZE
> > + ldr r1, =TASK_SIZE
> > #else
> > mov r1, #TASK_SIZE
> > #endif
>
> This is unnecessary:
>
> ldr r1, =TASK_SIZE
>
> will be converted to a mov instruction by the assembler if the value
> of TASK_SIZE fits its 12-bit immediate field.

It's an 8-bit immediate field for ARM.

What it won't do is expand it to a pair of movw/movt instructions if it
doesn't fit.

[Russell King - ARM Linux]

On Tue, Oct 17, 2017 at 11:27:19AM +0000, Liuwenliang (Lamb) wrote:
> ---c0a3b198: b6e00000 .word 0xb6e00000 //TASK_SIZE:0xb6e00000

It's probably going to be better all round to round TASK_SIZE down
to something that fits in an 8-bit rotated constant anyway (like
we already guarantee) which would mean this patch is not necessary.

[Liuwenliang (Lamb)]

On Tue, Oct 19, 2017 at 20:41 17PM +0000, Russell King - ARM Linux:

>On Mon, Oct 16, 2017 at 11:42:05AM +0000, Liuwenliang (Lamb) wrote:
>> On 10/16/2017 07:03 PM, Abbott Liu wrote:
> >arch/arm/kernel/entry-armv.S:348: Error: selected processor does not support `movw r1,
>> #:lower16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
>> >arch/arm/kernel/entry-armv.S:348: Error: selected processor does not support `movt r1,
>> #:upper16:((((0xC0000000-0x01000000)>>3)+((0xC0000000-0x01000000)-(1<<29))))' in ARM mode
>>
>> Thanks for building test. This error can be solved by following code:
>> --- a/arch/arm/kernel/entry-armv.S
>> +++ b/arch/arm/kernel/entry-armv.S
>> @@ -188,8 +188,7 @@ ENDPROC(__und_invalid)
>> get_thread_info tsk
>> ldr r0, [tsk, #TI_ADDR_LIMIT]
>> #ifdef CONFIG_KASAN
>> - movw r1, #:lower16:TASK_SIZE
>> - movt r1, #:upper16:TASK_SIZE
>> + ldr r1, =TASK_SIZE
>> #else
>> mov r1, #TASK_SIZE
>> #endif
>

>We can surely do better than this with macros and condition support -
>we can build-time test in the assembler whether TASK_SIZE can fit in a
>normal "mov", whether we can use the movw/movt instructions, or fall
>back to ldr if necessary. I'd rather we avoided "ldr" here where
>possible.

Thanks for your review.
I don't know why we need to avoided "ldr". The "ldr" maybe cause the
performance fall down, but it will be very limited, and as we know the
performance of kasan version is lower than the normal version. And usually
we don't use kasan version in our product, we only use kasan version when
we want to debug some memory corruption problem in laboratory(not not in
commercial product) because the performance of kasan version is lower than
normal version.

So I think we can accept the influence of the performance by using "ldr" here.




On Tue, Oct 19, 2017 at 20:44 17PM +0000, Russell King - ARM Linux:

>On Tue, Oct 17, 2017 at 11:27:19AM +0000, Liuwenliang (Lamb) wrote:
>> ---c0a3b198: b6e00000 .word 0xb6e00000 //TASK_SIZE:0xb6e00000
>
>It's probably going to be better all round to round TASK_SIZE down
>to something that fits in an 8-bit rotated constant anyway (like
>we already guarantee) which would mean this patch is not necessary.

Thanks for you review.
If we enable CONFIG_KASAN, we need steal 130MByte(0xb6e00000 ~ 0xbf000000) from user space.
If we change to steal 130MByte(0xb6000000 ~ 0xbe200000) , the 14MB of user space is going to be
wasted. I think it is better to to use "ldr" whose influence to the system are very limited than to waste
14MB user space by chaned TASK_SIZE from 0xb6e00000 from 0xb6000000.


If TASK_SIZE is an 8-bit rotated constant, the compiler can convert "ldr rx,=TASK_SIZE" into mov rx, #TASK_SIZE
If TASK_SIZE is not an 8-bit rotated constant, the compiler can convert "ldr rx,=TASK_SIZE" into ldr rx, [pc,xxx],
So we can use ldr to replace mov. Here is the code which is tested by me:

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index f9efea3..00a1833 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -187,12 +187,7 @@ ENDPROC(__und_invalid)

get_thread_info tsk
ldr r0, [tsk, #TI_ADDR_LIMIT]

-#ifdef CONFIG_KASAN

- movw r1, #:lower16:TASK_SIZE
- movt r1, #:upper16:TASK_SIZE

-#else
- mov r1, #TASK_SIZE
-#endif
+ ldr r1, =TASK_SIZE

str r1, [tsk, #TI_ADDR_LIMIT]
str r0, [sp, #SVC_ADDR_LIMIT]

@@ -446,7 +441,8 @@ ENDPROC(__fiq_abt)

@ if it was interrupted in a critical region. Here we
@ perform a quick test inline since it should be false
@ 99.9999% of the time. The rest is done out of line.

- cmp r4, #TASK_SIZE

+ ldr r0, =TASK_SIZE
+ cmp r4, r0

blhs kuser_cmpxchg64_fixup
#endif
#endif