Skip to content

Commit 295cf15

Browse files
rmurphy-armwilldeacon
authored andcommitted
arm64: Avoid premature usercopy failure
Al reminds us that the usercopy API must only return complete failure if absolutely nothing could be copied. Currently, if userspace does something silly like giving us an unaligned pointer to Device memory, or a size which overruns MTE tag bounds, we may fail to honour that requirement when faulting on a multi-byte access even though a smaller access could have succeeded. Add a mitigation to the fixup routines to fall back to a single-byte copy if we faulted on a larger access before anything has been written to the destination, to guarantee making *some* forward progress. We needn't be too concerned about the overall performance since this should only occur when callers are doing something a bit dodgy in the first place. Particularly broken userspace might still be able to trick generic_perform_write() into an infinite loop by targeting write() at an mmap() of some read-only device register where the fault-in load succeeds but any store synchronously aborts such that copy_to_user() is genuinely unable to make progress, but, well, don't do that... CC: [email protected] Reported-by: Chen Huang <[email protected]> Suggested-by: Al Viro <[email protected]> Reviewed-by: Catalin Marinas <[email protected]> Signed-off-by: Robin Murphy <[email protected]> Link: https://lore.kernel.org/r/dc03d5c675731a1f24a62417dba5429ad744234e.1626098433.git.robin.murphy@arm.com Signed-off-by: Will Deacon <[email protected]>
1 parent 8cdd23c commit 295cf15

File tree

3 files changed

+35
-13
lines changed

3 files changed

+35
-13
lines changed

arch/arm64/lib/copy_from_user.S

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,32 +29,34 @@
2929
.endm
3030

3131
.macro ldrh1 reg, ptr, val
32-
user_ldst 9998f, ldtrh, \reg, \ptr, \val
32+
user_ldst 9997f, ldtrh, \reg, \ptr, \val
3333
.endm
3434

3535
.macro strh1 reg, ptr, val
3636
strh \reg, [\ptr], \val
3737
.endm
3838

3939
.macro ldr1 reg, ptr, val
40-
user_ldst 9998f, ldtr, \reg, \ptr, \val
40+
user_ldst 9997f, ldtr, \reg, \ptr, \val
4141
.endm
4242

4343
.macro str1 reg, ptr, val
4444
str \reg, [\ptr], \val
4545
.endm
4646

4747
.macro ldp1 reg1, reg2, ptr, val
48-
user_ldp 9998f, \reg1, \reg2, \ptr, \val
48+
user_ldp 9997f, \reg1, \reg2, \ptr, \val
4949
.endm
5050

5151
.macro stp1 reg1, reg2, ptr, val
5252
stp \reg1, \reg2, [\ptr], \val
5353
.endm
5454

5555
end .req x5
56+
srcin .req x15
5657
SYM_FUNC_START(__arch_copy_from_user)
5758
add end, x0, x2
59+
mov srcin, x1
5860
#include "copy_template.S"
5961
mov x0, #0 // Nothing to copy
6062
ret
@@ -63,6 +65,11 @@ EXPORT_SYMBOL(__arch_copy_from_user)
6365

6466
.section .fixup,"ax"
6567
.align 2
68+
9997: cmp dst, dstin
69+
b.ne 9998f
70+
// Before being absolutely sure we couldn't copy anything, try harder
71+
USER(9998f, ldtrb tmp1w, [srcin])
72+
strb tmp1w, [dst], #1
6673
9998: sub x0, end, dst // bytes not copied
6774
ret
6875
.previous

arch/arm64/lib/copy_in_user.S

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,33 +30,34 @@
3030
.endm
3131

3232
.macro ldrh1 reg, ptr, val
33-
user_ldst 9998f, ldtrh, \reg, \ptr, \val
33+
user_ldst 9997f, ldtrh, \reg, \ptr, \val
3434
.endm
3535

3636
.macro strh1 reg, ptr, val
37-
user_ldst 9998f, sttrh, \reg, \ptr, \val
37+
user_ldst 9997f, sttrh, \reg, \ptr, \val
3838
.endm
3939

4040
.macro ldr1 reg, ptr, val
41-
user_ldst 9998f, ldtr, \reg, \ptr, \val
41+
user_ldst 9997f, ldtr, \reg, \ptr, \val
4242
.endm
4343

4444
.macro str1 reg, ptr, val
45-
user_ldst 9998f, sttr, \reg, \ptr, \val
45+
user_ldst 9997f, sttr, \reg, \ptr, \val
4646
.endm
4747

4848
.macro ldp1 reg1, reg2, ptr, val
49-
user_ldp 9998f, \reg1, \reg2, \ptr, \val
49+
user_ldp 9997f, \reg1, \reg2, \ptr, \val
5050
.endm
5151

5252
.macro stp1 reg1, reg2, ptr, val
53-
user_stp 9998f, \reg1, \reg2, \ptr, \val
53+
user_stp 9997f, \reg1, \reg2, \ptr, \val
5454
.endm
5555

5656
end .req x5
57-
57+
srcin .req x15
5858
SYM_FUNC_START(__arch_copy_in_user)
5959
add end, x0, x2
60+
mov srcin, x1
6061
#include "copy_template.S"
6162
mov x0, #0
6263
ret
@@ -65,6 +66,12 @@ EXPORT_SYMBOL(__arch_copy_in_user)
6566

6667
.section .fixup,"ax"
6768
.align 2
69+
9997: cmp dst, dstin
70+
b.ne 9998f
71+
// Before being absolutely sure we couldn't copy anything, try harder
72+
USER(9998f, ldtrb tmp1w, [srcin])
73+
USER(9998f, sttrb tmp1w, [dst])
74+
add dst, dst, #1
6875
9998: sub x0, end, dst // bytes not copied
6976
ret
7077
.previous

arch/arm64/lib/copy_to_user.S

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,28 +32,30 @@
3232
.endm
3333

3434
.macro strh1 reg, ptr, val
35-
user_ldst 9998f, sttrh, \reg, \ptr, \val
35+
user_ldst 9997f, sttrh, \reg, \ptr, \val
3636
.endm
3737

3838
.macro ldr1 reg, ptr, val
3939
ldr \reg, [\ptr], \val
4040
.endm
4141

4242
.macro str1 reg, ptr, val
43-
user_ldst 9998f, sttr, \reg, \ptr, \val
43+
user_ldst 9997f, sttr, \reg, \ptr, \val
4444
.endm
4545

4646
.macro ldp1 reg1, reg2, ptr, val
4747
ldp \reg1, \reg2, [\ptr], \val
4848
.endm
4949

5050
.macro stp1 reg1, reg2, ptr, val
51-
user_stp 9998f, \reg1, \reg2, \ptr, \val
51+
user_stp 9997f, \reg1, \reg2, \ptr, \val
5252
.endm
5353

5454
end .req x5
55+
srcin .req x15
5556
SYM_FUNC_START(__arch_copy_to_user)
5657
add end, x0, x2
58+
mov srcin, x1
5759
#include "copy_template.S"
5860
mov x0, #0
5961
ret
@@ -62,6 +64,12 @@ EXPORT_SYMBOL(__arch_copy_to_user)
6264

6365
.section .fixup,"ax"
6466
.align 2
67+
9997: cmp dst, dstin
68+
b.ne 9998f
69+
// Before being absolutely sure we couldn't copy anything, try harder
70+
ldrb tmp1w, [srcin]
71+
USER(9998f, sttrb tmp1w, [dst])
72+
add dst, dst, #1
6573
9998: sub x0, end, dst // bytes not copied
6674
ret
6775
.previous

0 commit comments

Comments
 (0)