* crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-03-19 11:54 ` Geert Uytterhoeven
0 siblings, 0 replies; 55+ messages in thread
From: Geert Uytterhoeven @ 2019-03-19 11:54 UTC (permalink / raw)
To: Kees Cook, Herbert Xu
Cc: linux-security-module, Linux ARM, Linux Crypto Mailing List,
Linux Kernel Mailing List
When running the sha1-asm crypto selftest on arm with
CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
usercopy: Kernel memory overwrite attempt detected to spans
multiple pages (offset 0, size 42)!
------------[ cut here ]------------
kernel BUG at mm/usercopy.c:102!
Internal error: Oops - BUG: 0 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
PC is at usercopy_abort+0x68/0x90
LR is at usercopy_abort+0x68/0x90
pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
sp : ea54bc60 ip : 00000010 fp : cccccccd
r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 30c5387d Table: 40003000 DAC: fffffffd
Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
Stack: (0xea54bc60 to 0xea54c000)
bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
00000000 c0310060
bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
ea54cfe0 ea538c00
bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
0000003a c0427a30
bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
0000002a c081cf80
bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
c0e0a408 eb074240
bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
ebef7480 eb074200
bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
c084061c c0428c38
bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
00000002 eabe4e80
bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
00000006 c09d544c
bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
eb074200 00000000
bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
00000000 c081cf70
bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
c0e4ee80 443e9884
bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
fefefefe fefefefe
be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
ea4f7a00 c03062bc
be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
00000002 eabe4e40
be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
eb074200 ea538c00
be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
c04dace8 00000006
be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
ea4f71a8 c0429420
bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
00000400 ffffffff
bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
c0e5a2a0 c0e5a2a0
bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
c0e5a2a0 c0269470
bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
ea537280 0000000e
bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
c09c9ed0 ea54bf5c
bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
c09c9ed0 ea537200
bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
ea4f71a8 c0426524
bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
00000000 00000000
bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
00000000 00000000
[<c030fd60>] (usercopy_abort) from [<c0310060>]
(__check_object_size+0x2d8/0x448)
[<c0310060>] (__check_object_size) from [<c0427a30>]
(build_test_sglist+0x268/0x2d8)
[<c0427a30>] (build_test_sglist) from [<c0428c38>]
(test_hash_vec_cfg+0x110/0x694)
[<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
(__alg_test_hash+0x158/0x1b8)
[<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
[<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
[<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
[<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
[<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
Exception stack(0xea54bfb0 to 0xea54bff8)
bfa0: 00000000 00000000
00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
---[ end trace 190b3cf48e720f78 ]---
BUG: sleeping function called from invalid context at
include/linux/percpu-rwsem.h:34
in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
[<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
[<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
[<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
[<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
[<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
[<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
[<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
[<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
Exception stack(0xea54bc10 to 0xea54bc58)
bc00: 0000005f 2abb4000
dd6cd422 dd6cd422
bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
00000000 cccccccd
bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
[<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
[<c030fd60>] (usercopy_abort) from [<c0310060>]
(__check_object_size+0x2d8/0x448)
[<c0310060>] (__check_object_size) from [<c0427a30>]
(build_test_sglist+0x268/0x2d8)
[<c0427a30>] (build_test_sglist) from [<c0428c38>]
(test_hash_vec_cfg+0x110/0x694)
[<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
(__alg_test_hash+0x158/0x1b8)
[<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
[<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
[<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
[<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
[<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
Exception stack(0xea54bfb0 to 0xea54bff8)
bfa0: 00000000 00000000
00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
A similar trace is seen with sha1-ce on arm64:
usercopy: Kernel memory overwrite attempt detected to spans
multiple pages (offset 0, size 42)!
------------[ cut here ]------------
kernel BUG at mm/usercopy.c:102!
Internal error: Oops - BUG: 0 [#1] SMP
Modules linked in:
CPU: 1 PID: 33 Comm: cryptomgr_test Not tainted
5.1.0-rc1-salvator-x-01109-gbeb7d6376ecfbf07-dirty #352
Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
pstate: 60400005 (nZCv daif +PAN -UAO)
pc : usercopy_abort+0x64/0x90
lr : usercopy_abort+0x64/0x90
sp : ffffff8011eb38d0
x29: ffffff8011eb38e0 x28: 6db6db6db6db6db7
x27: ffffffbf00000000 x26: 0000000000000038
x25: ffffffc0778fd009 x24: 0000000000000000
x23: ffffffc0778fd00a x22: ffffff8010d51000
x21: 0000000000000000 x20: 000000000000002a
x19: ffffffc0778fcfe0 x18: 000000000000000a
x17: 00000000526a1be5 x16: 0000000000000014
x15: 000000000009f6c2 x14: 0720072007200720
x13: 0720072007200720 x12: 0720072007200720
x11: 0720072007200720 x10: 0720072007200720
x9 : ffffff80110126c8 x8 : 0000000000000000
x7 : ffffff801015700c x6 : 0000000000000000
x5 : 0000000000000000 x4 : ffffff8011eb4000
x3 : 0000000000000080 x2 : a045404094166600
x1 : 0000000000000000 x0 : 000000000000005f
Process cryptomgr_test (pid: 33, stack limit = 0x(____ptrval____))
Call trace:
usercopy_abort+0x64/0x90
__check_object_size+0x64/0x464
build_test_sglist+0x238/0x2c8
test_hash_vec_cfg+0x130/0x660
__alg_test_hash+0x1b4/0x1f4
alg_test_hash+0x88/0x104
alg_test.part.6+0x2a8/0x330
alg_test+0x98/0xa0
cryptomgr_test+0x24/0x4c
kthread+0x120/0x130
ret_from_fork+0x10/0x18
Code: aa0003e3 b00053e0 91148000 97fc3bf2 (d4210000)
---[ end trace d9f3261d50a7f84f ]---
BUG: sleeping function called from invalid context at
include/linux/percpu-rwsem.h:34
in_atomic(): 0, irqs_disabled(): 128, pid: 33, name: cryptomgr_test
INFO: lockdep is turned off.
irq event stamp: 262
hardirqs last enabled at (261): [<ffffff8010157050>]
console_unlock+0x554/0x560
hardirqs last disabled at (262): [<ffffff8010081a28>]
do_debug_exception+0x48/0x13c
softirqs last enabled at (258): [<ffffff8010081ee4>]
__do_softirq+0x18c/0x4a0
softirqs last disabled at (245): [<ffffff80100f3e10>] irq_exit+0xa4/0x100
CPU: 1 PID: 33 Comm: cryptomgr_test Tainted: G D
5.1.0-rc1-salvator-x-01109-gbeb7d6376ecfbf07-dirty #352
Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
Call trace:
dump_backtrace+0x0/0x118
show_stack+0x14/0x1c
dump_stack+0xc8/0x118
___might_sleep+0x24c/0x25c
__might_sleep+0x70/0x80
exit_signals+0x48/0x278
do_exit+0x10c/0xa30
die+0x1f4/0x208
bug_handler+0x4c/0x78
brk_handler+0x15c/0x188
do_debug_exception+0xd4/0x13c
el1_dbg+0x18/0xbc
usercopy_abort+0x64/0x90
__check_object_size+0x64/0x464
build_test_sglist+0x238/0x2c8
test_hash_vec_cfg+0x130/0x660
__alg_test_hash+0x1b4/0x1f4
alg_test_hash+0x88/0x104
alg_test.part.6+0x2a8/0x330
alg_test+0x98/0xa0
cryptomgr_test+0x24/0x4c
kthread+0x120/0x130
ret_from_fork+0x10/0x18
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 55+ messages in thread
* crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-03-19 11:54 ` Geert Uytterhoeven
0 siblings, 0 replies; 55+ messages in thread
From: Geert Uytterhoeven @ 2019-03-19 11:54 UTC (permalink / raw)
To: Kees Cook, Herbert Xu
Cc: linux-security-module, Linux Crypto Mailing List, Linux ARM,
Linux Kernel Mailing List
When running the sha1-asm crypto selftest on arm with
CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
usercopy: Kernel memory overwrite attempt detected to spans
multiple pages (offset 0, size 42)!
------------[ cut here ]------------
kernel BUG at mm/usercopy.c:102!
Internal error: Oops - BUG: 0 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
PC is at usercopy_abort+0x68/0x90
LR is at usercopy_abort+0x68/0x90
pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
sp : ea54bc60 ip : 00000010 fp : cccccccd
r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 30c5387d Table: 40003000 DAC: fffffffd
Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
Stack: (0xea54bc60 to 0xea54c000)
bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
00000000 c0310060
bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
ea54cfe0 ea538c00
bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
0000003a c0427a30
bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
0000002a c081cf80
bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
c0e0a408 eb074240
bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
ebef7480 eb074200
bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
c084061c c0428c38
bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
00000002 eabe4e80
bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
00000006 c09d544c
bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
eb074200 00000000
bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
00000000 c081cf70
bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
c0e4ee80 443e9884
bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
fefefefe fefefefe
be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
ea4f7a00 c03062bc
be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
00000002 eabe4e40
be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
eb074200 ea538c00
be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
c04dace8 00000006
be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
ea4f71a8 c0429420
bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
00000400 ffffffff
bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
c0e5a2a0 c0e5a2a0
bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
c0e5a2a0 c0269470
bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
ea537280 0000000e
bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
c09c9ed0 ea54bf5c
bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
c09c9ed0 ea537200
bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
ea4f71a8 c0426524
bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
00000000 00000000
bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
00000000 00000000
[<c030fd60>] (usercopy_abort) from [<c0310060>]
(__check_object_size+0x2d8/0x448)
[<c0310060>] (__check_object_size) from [<c0427a30>]
(build_test_sglist+0x268/0x2d8)
[<c0427a30>] (build_test_sglist) from [<c0428c38>]
(test_hash_vec_cfg+0x110/0x694)
[<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
(__alg_test_hash+0x158/0x1b8)
[<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
[<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
[<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
[<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
[<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
Exception stack(0xea54bfb0 to 0xea54bff8)
bfa0: 00000000 00000000
00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
---[ end trace 190b3cf48e720f78 ]---
BUG: sleeping function called from invalid context at
include/linux/percpu-rwsem.h:34
in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
[<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
[<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
[<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
[<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
[<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
[<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
[<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
[<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
Exception stack(0xea54bc10 to 0xea54bc58)
bc00: 0000005f 2abb4000
dd6cd422 dd6cd422
bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
00000000 cccccccd
bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
[<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
[<c030fd60>] (usercopy_abort) from [<c0310060>]
(__check_object_size+0x2d8/0x448)
[<c0310060>] (__check_object_size) from [<c0427a30>]
(build_test_sglist+0x268/0x2d8)
[<c0427a30>] (build_test_sglist) from [<c0428c38>]
(test_hash_vec_cfg+0x110/0x694)
[<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
(__alg_test_hash+0x158/0x1b8)
[<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
[<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
[<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
[<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
[<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
Exception stack(0xea54bfb0 to 0xea54bff8)
bfa0: 00000000 00000000
00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
A similar trace is seen with sha1-ce on arm64:
usercopy: Kernel memory overwrite attempt detected to spans
multiple pages (offset 0, size 42)!
------------[ cut here ]------------
kernel BUG at mm/usercopy.c:102!
Internal error: Oops - BUG: 0 [#1] SMP
Modules linked in:
CPU: 1 PID: 33 Comm: cryptomgr_test Not tainted
5.1.0-rc1-salvator-x-01109-gbeb7d6376ecfbf07-dirty #352
Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
pstate: 60400005 (nZCv daif +PAN -UAO)
pc : usercopy_abort+0x64/0x90
lr : usercopy_abort+0x64/0x90
sp : ffffff8011eb38d0
x29: ffffff8011eb38e0 x28: 6db6db6db6db6db7
x27: ffffffbf00000000 x26: 0000000000000038
x25: ffffffc0778fd009 x24: 0000000000000000
x23: ffffffc0778fd00a x22: ffffff8010d51000
x21: 0000000000000000 x20: 000000000000002a
x19: ffffffc0778fcfe0 x18: 000000000000000a
x17: 00000000526a1be5 x16: 0000000000000014
x15: 000000000009f6c2 x14: 0720072007200720
x13: 0720072007200720 x12: 0720072007200720
x11: 0720072007200720 x10: 0720072007200720
x9 : ffffff80110126c8 x8 : 0000000000000000
x7 : ffffff801015700c x6 : 0000000000000000
x5 : 0000000000000000 x4 : ffffff8011eb4000
x3 : 0000000000000080 x2 : a045404094166600
x1 : 0000000000000000 x0 : 000000000000005f
Process cryptomgr_test (pid: 33, stack limit = 0x(____ptrval____))
Call trace:
usercopy_abort+0x64/0x90
__check_object_size+0x64/0x464
build_test_sglist+0x238/0x2c8
test_hash_vec_cfg+0x130/0x660
__alg_test_hash+0x1b4/0x1f4
alg_test_hash+0x88/0x104
alg_test.part.6+0x2a8/0x330
alg_test+0x98/0xa0
cryptomgr_test+0x24/0x4c
kthread+0x120/0x130
ret_from_fork+0x10/0x18
Code: aa0003e3 b00053e0 91148000 97fc3bf2 (d4210000)
---[ end trace d9f3261d50a7f84f ]---
BUG: sleeping function called from invalid context at
include/linux/percpu-rwsem.h:34
in_atomic(): 0, irqs_disabled(): 128, pid: 33, name: cryptomgr_test
INFO: lockdep is turned off.
irq event stamp: 262
hardirqs last enabled at (261): [<ffffff8010157050>]
console_unlock+0x554/0x560
hardirqs last disabled at (262): [<ffffff8010081a28>]
do_debug_exception+0x48/0x13c
softirqs last enabled at (258): [<ffffff8010081ee4>]
__do_softirq+0x18c/0x4a0
softirqs last disabled at (245): [<ffffff80100f3e10>] irq_exit+0xa4/0x100
CPU: 1 PID: 33 Comm: cryptomgr_test Tainted: G D
5.1.0-rc1-salvator-x-01109-gbeb7d6376ecfbf07-dirty #352
Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
Call trace:
dump_backtrace+0x0/0x118
show_stack+0x14/0x1c
dump_stack+0xc8/0x118
___might_sleep+0x24c/0x25c
__might_sleep+0x70/0x80
exit_signals+0x48/0x278
do_exit+0x10c/0xa30
die+0x1f4/0x208
bug_handler+0x4c/0x78
brk_handler+0x15c/0x188
do_debug_exception+0xd4/0x13c
el1_dbg+0x18/0xbc
usercopy_abort+0x64/0x90
__check_object_size+0x64/0x464
build_test_sglist+0x238/0x2c8
test_hash_vec_cfg+0x130/0x660
__alg_test_hash+0x1b4/0x1f4
alg_test_hash+0x88/0x104
alg_test.part.6+0x2a8/0x330
alg_test+0x98/0xa0
cryptomgr_test+0x24/0x4c
kthread+0x120/0x130
ret_from_fork+0x10/0x18
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-03-19 11:54 ` Geert Uytterhoeven
@ 2019-03-19 17:09 ` Eric Biggers
-1 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-03-19 17:09 UTC (permalink / raw)
To: Geert Uytterhoeven, Kees Cook
Cc: Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List
On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> When running the sha1-asm crypto selftest on arm with
> CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
>
> usercopy: Kernel memory overwrite attempt detected to spans
> multiple pages (offset 0, size 42)!
> ------------[ cut here ]------------
> kernel BUG at mm/usercopy.c:102!
> Internal error: Oops - BUG: 0 [#1] SMP ARM
> Modules linked in:
> CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
> 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> PC is at usercopy_abort+0x68/0x90
> LR is at usercopy_abort+0x68/0x90
> pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
> sp : ea54bc60 ip : 00000010 fp : cccccccd
> r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
> r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
> r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
> Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> Control: 30c5387d Table: 40003000 DAC: fffffffd
> Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
> Stack: (0xea54bc60 to 0xea54c000)
> bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
> 00000000 c0310060
> bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
> ea54cfe0 ea538c00
> bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
> 0000003a c0427a30
> bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
> 0000002a c081cf80
> bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
> c0e0a408 eb074240
> bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
> ebef7480 eb074200
> bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
> c084061c c0428c38
> bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
> 00000002 eabe4e80
> bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
> 00000006 c09d544c
> bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
> eb074200 00000000
> bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
> 00000000 c081cf70
> bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
> c0e4ee80 443e9884
> bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
> fefefefe fefefefe
> be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
> ea4f7a00 c03062bc
> be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
> 00000002 eabe4e40
> be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
> eb074200 ea538c00
> be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
> c04dace8 00000006
> be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
> ea4f71a8 c0429420
> bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
> 00000400 ffffffff
> bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
> c0e5a2a0 c0e5a2a0
> bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
> c0e5a2a0 c0269470
> bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
> ea537280 0000000e
> bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
> c09c9ed0 ea54bf5c
> bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
> c09c9ed0 ea537200
> bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
> ea4f71a8 c0426524
> bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
> 00000000 00000000
> bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
> 00000000 00000000
> bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
> bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> 00000000 00000000
> [<c030fd60>] (usercopy_abort) from [<c0310060>]
> (__check_object_size+0x2d8/0x448)
> [<c0310060>] (__check_object_size) from [<c0427a30>]
> (build_test_sglist+0x268/0x2d8)
> [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> (test_hash_vec_cfg+0x110/0x694)
> [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> (__alg_test_hash+0x158/0x1b8)
> [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> Exception stack(0xea54bfb0 to 0xea54bff8)
> bfa0: 00000000 00000000
> 00000000 00000000
> bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
> bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
> ---[ end trace 190b3cf48e720f78 ]---
> BUG: sleeping function called from invalid context at
> include/linux/percpu-rwsem.h:34
> in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
> CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
> 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
> [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
> [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
> [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
> [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
> [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
> [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
> [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
> Exception stack(0xea54bc10 to 0xea54bc58)
> bc00: 0000005f 2abb4000
> dd6cd422 dd6cd422
> bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
> 00000000 cccccccd
> bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
> [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
> [<c030fd60>] (usercopy_abort) from [<c0310060>]
> (__check_object_size+0x2d8/0x448)
> [<c0310060>] (__check_object_size) from [<c0427a30>]
> (build_test_sglist+0x268/0x2d8)
> [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> (test_hash_vec_cfg+0x110/0x694)
> [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> (__alg_test_hash+0x158/0x1b8)
> [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> Exception stack(0xea54bfb0 to 0xea54bff8)
> bfa0: 00000000 00000000
> 00000000 00000000
> bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
> bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
>
> A similar trace is seen with sha1-ce on arm64:
>
> usercopy: Kernel memory overwrite attempt detected to spans
> multiple pages (offset 0, size 42)!
> ------------[ cut here ]------------
> kernel BUG at mm/usercopy.c:102!
> Internal error: Oops - BUG: 0 [#1] SMP
> Modules linked in:
> CPU: 1 PID: 33 Comm: cryptomgr_test Not tainted
> 5.1.0-rc1-salvator-x-01109-gbeb7d6376ecfbf07-dirty #352
> Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
> pstate: 60400005 (nZCv daif +PAN -UAO)
> pc : usercopy_abort+0x64/0x90
> lr : usercopy_abort+0x64/0x90
> sp : ffffff8011eb38d0
> x29: ffffff8011eb38e0 x28: 6db6db6db6db6db7
> x27: ffffffbf00000000 x26: 0000000000000038
> x25: ffffffc0778fd009 x24: 0000000000000000
> x23: ffffffc0778fd00a x22: ffffff8010d51000
> x21: 0000000000000000 x20: 000000000000002a
> x19: ffffffc0778fcfe0 x18: 000000000000000a
> x17: 00000000526a1be5 x16: 0000000000000014
> x15: 000000000009f6c2 x14: 0720072007200720
> x13: 0720072007200720 x12: 0720072007200720
> x11: 0720072007200720 x10: 0720072007200720
> x9 : ffffff80110126c8 x8 : 0000000000000000
> x7 : ffffff801015700c x6 : 0000000000000000
> x5 : 0000000000000000 x4 : ffffff8011eb4000
> x3 : 0000000000000080 x2 : a045404094166600
> x1 : 0000000000000000 x0 : 000000000000005f
> Process cryptomgr_test (pid: 33, stack limit = 0x(____ptrval____))
> Call trace:
> usercopy_abort+0x64/0x90
> __check_object_size+0x64/0x464
> build_test_sglist+0x238/0x2c8
> test_hash_vec_cfg+0x130/0x660
> __alg_test_hash+0x1b4/0x1f4
> alg_test_hash+0x88/0x104
> alg_test.part.6+0x2a8/0x330
> alg_test+0x98/0xa0
> cryptomgr_test+0x24/0x4c
> kthread+0x120/0x130
> ret_from_fork+0x10/0x18
> Code: aa0003e3 b00053e0 91148000 97fc3bf2 (d4210000)
> ---[ end trace d9f3261d50a7f84f ]---
> BUG: sleeping function called from invalid context at
> include/linux/percpu-rwsem.h:34
> in_atomic(): 0, irqs_disabled(): 128, pid: 33, name: cryptomgr_test
> INFO: lockdep is turned off.
> irq event stamp: 262
> hardirqs last enabled at (261): [<ffffff8010157050>]
> console_unlock+0x554/0x560
> hardirqs last disabled at (262): [<ffffff8010081a28>]
> do_debug_exception+0x48/0x13c
> softirqs last enabled at (258): [<ffffff8010081ee4>]
> __do_softirq+0x18c/0x4a0
> softirqs last disabled at (245): [<ffffff80100f3e10>] irq_exit+0xa4/0x100
> CPU: 1 PID: 33 Comm: cryptomgr_test Tainted: G D
> 5.1.0-rc1-salvator-x-01109-gbeb7d6376ecfbf07-dirty #352
> Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
> Call trace:
> dump_backtrace+0x0/0x118
> show_stack+0x14/0x1c
> dump_stack+0xc8/0x118
> ___might_sleep+0x24c/0x25c
> __might_sleep+0x70/0x80
> exit_signals+0x48/0x278
> do_exit+0x10c/0xa30
> die+0x1f4/0x208
> bug_handler+0x4c/0x78
> brk_handler+0x15c/0x188
> do_debug_exception+0xd4/0x13c
> el1_dbg+0x18/0xbc
> usercopy_abort+0x64/0x90
> __check_object_size+0x64/0x464
> build_test_sglist+0x238/0x2c8
> test_hash_vec_cfg+0x130/0x660
> __alg_test_hash+0x1b4/0x1f4
> alg_test_hash+0x88/0x104
> alg_test.part.6+0x2a8/0x330
> alg_test+0x98/0xa0
> cryptomgr_test+0x24/0x4c
> kthread+0x120/0x130
> ret_from_fork+0x10/0x18
>
> Gr{oetje,eeting}s,
>
> Geert
>
Well, this must happen with the new (in 5.1) crypto self-tests implementation
for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
understand why hardened usercopy considers it a bug though, as there's no buffer
overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
buffer that was allocated with __get_free_pages():
__get_free_pages(GFP_KERNEL, 1)
... where 1 means an order-1 allocation.
If it copies to offset=4064 len=42, for example, then hardened usercopy
considers it a bug even though the buffer is 8192 bytes long. Why?
It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
with ITER_KVEC.
- Eric
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-03-19 17:09 ` Eric Biggers
0 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-03-19 17:09 UTC (permalink / raw)
To: Geert Uytterhoeven, Kees Cook
Cc: Linux Kernel Mailing List, linux-security-module, Herbert Xu,
Linux ARM, Linux Crypto Mailing List
On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> When running the sha1-asm crypto selftest on arm with
> CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
>
> usercopy: Kernel memory overwrite attempt detected to spans
> multiple pages (offset 0, size 42)!
> ------------[ cut here ]------------
> kernel BUG at mm/usercopy.c:102!
> Internal error: Oops - BUG: 0 [#1] SMP ARM
> Modules linked in:
> CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
> 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> PC is at usercopy_abort+0x68/0x90
> LR is at usercopy_abort+0x68/0x90
> pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
> sp : ea54bc60 ip : 00000010 fp : cccccccd
> r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
> r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
> r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
> Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> Control: 30c5387d Table: 40003000 DAC: fffffffd
> Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
> Stack: (0xea54bc60 to 0xea54c000)
> bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
> 00000000 c0310060
> bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
> ea54cfe0 ea538c00
> bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
> 0000003a c0427a30
> bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
> 0000002a c081cf80
> bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
> c0e0a408 eb074240
> bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
> ebef7480 eb074200
> bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
> c084061c c0428c38
> bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
> 00000002 eabe4e80
> bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
> 00000006 c09d544c
> bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
> eb074200 00000000
> bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
> 00000000 c081cf70
> bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
> c0e4ee80 443e9884
> bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
> fefefefe fefefefe
> be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
> ea4f7a00 c03062bc
> be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
> 00000002 eabe4e40
> be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
> eb074200 ea538c00
> be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
> c04dace8 00000006
> be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
> ea4f71a8 c0429420
> bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
> 00000400 ffffffff
> bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
> c0e5a2a0 c0e5a2a0
> bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
> c0e5a2a0 c0269470
> bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
> ea537280 0000000e
> bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
> c09c9ed0 ea54bf5c
> bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
> c09c9ed0 ea537200
> bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
> ea4f71a8 c0426524
> bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
> 00000000 00000000
> bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
> 00000000 00000000
> bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
> bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> 00000000 00000000
> [<c030fd60>] (usercopy_abort) from [<c0310060>]
> (__check_object_size+0x2d8/0x448)
> [<c0310060>] (__check_object_size) from [<c0427a30>]
> (build_test_sglist+0x268/0x2d8)
> [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> (test_hash_vec_cfg+0x110/0x694)
> [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> (__alg_test_hash+0x158/0x1b8)
> [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> Exception stack(0xea54bfb0 to 0xea54bff8)
> bfa0: 00000000 00000000
> 00000000 00000000
> bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
> bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
> ---[ end trace 190b3cf48e720f78 ]---
> BUG: sleeping function called from invalid context at
> include/linux/percpu-rwsem.h:34
> in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
> CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
> 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
> [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
> [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
> [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
> [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
> [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
> [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
> [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
> Exception stack(0xea54bc10 to 0xea54bc58)
> bc00: 0000005f 2abb4000
> dd6cd422 dd6cd422
> bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
> 00000000 cccccccd
> bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
> [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
> [<c030fd60>] (usercopy_abort) from [<c0310060>]
> (__check_object_size+0x2d8/0x448)
> [<c0310060>] (__check_object_size) from [<c0427a30>]
> (build_test_sglist+0x268/0x2d8)
> [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> (test_hash_vec_cfg+0x110/0x694)
> [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> (__alg_test_hash+0x158/0x1b8)
> [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> Exception stack(0xea54bfb0 to 0xea54bff8)
> bfa0: 00000000 00000000
> 00000000 00000000
> bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
> bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
>
> A similar trace is seen with sha1-ce on arm64:
>
> usercopy: Kernel memory overwrite attempt detected to spans
> multiple pages (offset 0, size 42)!
> ------------[ cut here ]------------
> kernel BUG at mm/usercopy.c:102!
> Internal error: Oops - BUG: 0 [#1] SMP
> Modules linked in:
> CPU: 1 PID: 33 Comm: cryptomgr_test Not tainted
> 5.1.0-rc1-salvator-x-01109-gbeb7d6376ecfbf07-dirty #352
> Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
> pstate: 60400005 (nZCv daif +PAN -UAO)
> pc : usercopy_abort+0x64/0x90
> lr : usercopy_abort+0x64/0x90
> sp : ffffff8011eb38d0
> x29: ffffff8011eb38e0 x28: 6db6db6db6db6db7
> x27: ffffffbf00000000 x26: 0000000000000038
> x25: ffffffc0778fd009 x24: 0000000000000000
> x23: ffffffc0778fd00a x22: ffffff8010d51000
> x21: 0000000000000000 x20: 000000000000002a
> x19: ffffffc0778fcfe0 x18: 000000000000000a
> x17: 00000000526a1be5 x16: 0000000000000014
> x15: 000000000009f6c2 x14: 0720072007200720
> x13: 0720072007200720 x12: 0720072007200720
> x11: 0720072007200720 x10: 0720072007200720
> x9 : ffffff80110126c8 x8 : 0000000000000000
> x7 : ffffff801015700c x6 : 0000000000000000
> x5 : 0000000000000000 x4 : ffffff8011eb4000
> x3 : 0000000000000080 x2 : a045404094166600
> x1 : 0000000000000000 x0 : 000000000000005f
> Process cryptomgr_test (pid: 33, stack limit = 0x(____ptrval____))
> Call trace:
> usercopy_abort+0x64/0x90
> __check_object_size+0x64/0x464
> build_test_sglist+0x238/0x2c8
> test_hash_vec_cfg+0x130/0x660
> __alg_test_hash+0x1b4/0x1f4
> alg_test_hash+0x88/0x104
> alg_test.part.6+0x2a8/0x330
> alg_test+0x98/0xa0
> cryptomgr_test+0x24/0x4c
> kthread+0x120/0x130
> ret_from_fork+0x10/0x18
> Code: aa0003e3 b00053e0 91148000 97fc3bf2 (d4210000)
> ---[ end trace d9f3261d50a7f84f ]---
> BUG: sleeping function called from invalid context at
> include/linux/percpu-rwsem.h:34
> in_atomic(): 0, irqs_disabled(): 128, pid: 33, name: cryptomgr_test
> INFO: lockdep is turned off.
> irq event stamp: 262
> hardirqs last enabled at (261): [<ffffff8010157050>]
> console_unlock+0x554/0x560
> hardirqs last disabled at (262): [<ffffff8010081a28>]
> do_debug_exception+0x48/0x13c
> softirqs last enabled at (258): [<ffffff8010081ee4>]
> __do_softirq+0x18c/0x4a0
> softirqs last disabled at (245): [<ffffff80100f3e10>] irq_exit+0xa4/0x100
> CPU: 1 PID: 33 Comm: cryptomgr_test Tainted: G D
> 5.1.0-rc1-salvator-x-01109-gbeb7d6376ecfbf07-dirty #352
> Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
> Call trace:
> dump_backtrace+0x0/0x118
> show_stack+0x14/0x1c
> dump_stack+0xc8/0x118
> ___might_sleep+0x24c/0x25c
> __might_sleep+0x70/0x80
> exit_signals+0x48/0x278
> do_exit+0x10c/0xa30
> die+0x1f4/0x208
> bug_handler+0x4c/0x78
> brk_handler+0x15c/0x188
> do_debug_exception+0xd4/0x13c
> el1_dbg+0x18/0xbc
> usercopy_abort+0x64/0x90
> __check_object_size+0x64/0x464
> build_test_sglist+0x238/0x2c8
> test_hash_vec_cfg+0x130/0x660
> __alg_test_hash+0x1b4/0x1f4
> alg_test_hash+0x88/0x104
> alg_test.part.6+0x2a8/0x330
> alg_test+0x98/0xa0
> cryptomgr_test+0x24/0x4c
> kthread+0x120/0x130
> ret_from_fork+0x10/0x18
>
> Gr{oetje,eeting}s,
>
> Geert
>
Well, this must happen with the new (in 5.1) crypto self-tests implementation
for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
understand why hardened usercopy considers it a bug though, as there's no buffer
overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
buffer that was allocated with __get_free_pages():
__get_free_pages(GFP_KERNEL, 1)
... where 1 means an order-1 allocation.
If it copies to offset=4064 len=42, for example, then hardened usercopy
considers it a bug even though the buffer is 8192 bytes long. Why?
It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
with ITER_KVEC.
- Eric
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-03-19 17:09 ` Eric Biggers
@ 2019-03-20 18:57 ` Eric Biggers
-1 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-03-20 18:57 UTC (permalink / raw)
To: Kees Cook
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List
On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > When running the sha1-asm crypto selftest on arm with
> > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> >
> > usercopy: Kernel memory overwrite attempt detected to spans
> > multiple pages (offset 0, size 42)!
> > ------------[ cut here ]------------
> > kernel BUG at mm/usercopy.c:102!
> > Internal error: Oops - BUG: 0 [#1] SMP ARM
> > Modules linked in:
> > CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
> > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > PC is at usercopy_abort+0x68/0x90
> > LR is at usercopy_abort+0x68/0x90
> > pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
> > sp : ea54bc60 ip : 00000010 fp : cccccccd
> > r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
> > r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
> > r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
> > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > Control: 30c5387d Table: 40003000 DAC: fffffffd
> > Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
> > Stack: (0xea54bc60 to 0xea54c000)
> > bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
> > 00000000 c0310060
> > bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
> > ea54cfe0 ea538c00
> > bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
> > 0000003a c0427a30
> > bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
> > 0000002a c081cf80
> > bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
> > c0e0a408 eb074240
> > bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
> > ebef7480 eb074200
> > bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
> > c084061c c0428c38
> > bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
> > 00000002 eabe4e80
> > bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
> > 00000006 c09d544c
> > bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
> > eb074200 00000000
> > bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
> > 00000000 c081cf70
> > bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
> > c0e4ee80 443e9884
> > bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
> > fefefefe fefefefe
> > be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
> > ea4f7a00 c03062bc
> > be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
> > 00000002 eabe4e40
> > be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
> > eb074200 ea538c00
> > be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
> > c04dace8 00000006
> > be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
> > ea4f71a8 c0429420
> > bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
> > 00000400 ffffffff
> > bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
> > c0e5a2a0 c0e5a2a0
> > bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
> > c0e5a2a0 c0269470
> > bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
> > ea537280 0000000e
> > bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
> > c09c9ed0 ea54bf5c
> > bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
> > c09c9ed0 ea537200
> > bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
> > ea4f71a8 c0426524
> > bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
> > 00000000 00000000
> > bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
> > 00000000 00000000
> > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > 00000000 00000000
> > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > 00000000 00000000
> > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > (__check_object_size+0x2d8/0x448)
> > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > (build_test_sglist+0x268/0x2d8)
> > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > (test_hash_vec_cfg+0x110/0x694)
> > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > (__alg_test_hash+0x158/0x1b8)
> > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > Exception stack(0xea54bfb0 to 0xea54bff8)
> > bfa0: 00000000 00000000
> > 00000000 00000000
> > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > 00000000 00000000
> > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
> > ---[ end trace 190b3cf48e720f78 ]---
> > BUG: sleeping function called from invalid context at
> > include/linux/percpu-rwsem.h:34
> > in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
> > CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
> > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
> > [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
> > [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
> > [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
> > [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
> > [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
> > [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
> > [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
> > Exception stack(0xea54bc10 to 0xea54bc58)
> > bc00: 0000005f 2abb4000
> > dd6cd422 dd6cd422
> > bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
> > 00000000 cccccccd
> > bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
> > [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
> > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > (__check_object_size+0x2d8/0x448)
> > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > (build_test_sglist+0x268/0x2d8)
> > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > (test_hash_vec_cfg+0x110/0x694)
> > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > (__alg_test_hash+0x158/0x1b8)
> > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > Exception stack(0xea54bfb0 to 0xea54bff8)
> > bfa0: 00000000 00000000
> > 00000000 00000000
> > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > 00000000 00000000
> > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> >
>
> Well, this must happen with the new (in 5.1) crypto self-tests implementation
> for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> understand why hardened usercopy considers it a bug though, as there's no buffer
> overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> buffer that was allocated with __get_free_pages():
>
> __get_free_pages(GFP_KERNEL, 1)
>
> ... where 1 means an order-1 allocation.
>
> If it copies to offset=4064 len=42, for example, then hardened usercopy
> considers it a bug even though the buffer is 8192 bytes long. Why?
>
> It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> with ITER_KVEC.
>
> - Eric
Kees, any thoughts on why hardened usercopy rejects copies spanning a page
boundary when they seem to be fine?
- Eric
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-03-20 18:57 ` Eric Biggers
0 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-03-20 18:57 UTC (permalink / raw)
To: Kees Cook
Cc: Herbert Xu, Linux Kernel Mailing List, linux-security-module,
Geert Uytterhoeven, Linux Crypto Mailing List, Linux ARM
On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > When running the sha1-asm crypto selftest on arm with
> > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> >
> > usercopy: Kernel memory overwrite attempt detected to spans
> > multiple pages (offset 0, size 42)!
> > ------------[ cut here ]------------
> > kernel BUG at mm/usercopy.c:102!
> > Internal error: Oops - BUG: 0 [#1] SMP ARM
> > Modules linked in:
> > CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
> > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > PC is at usercopy_abort+0x68/0x90
> > LR is at usercopy_abort+0x68/0x90
> > pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
> > sp : ea54bc60 ip : 00000010 fp : cccccccd
> > r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
> > r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
> > r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
> > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > Control: 30c5387d Table: 40003000 DAC: fffffffd
> > Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
> > Stack: (0xea54bc60 to 0xea54c000)
> > bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
> > 00000000 c0310060
> > bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
> > ea54cfe0 ea538c00
> > bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
> > 0000003a c0427a30
> > bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
> > 0000002a c081cf80
> > bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
> > c0e0a408 eb074240
> > bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
> > ebef7480 eb074200
> > bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
> > c084061c c0428c38
> > bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
> > 00000002 eabe4e80
> > bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
> > 00000006 c09d544c
> > bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
> > eb074200 00000000
> > bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
> > 00000000 c081cf70
> > bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
> > c0e4ee80 443e9884
> > bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
> > fefefefe fefefefe
> > be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
> > ea4f7a00 c03062bc
> > be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
> > 00000002 eabe4e40
> > be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
> > eb074200 ea538c00
> > be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
> > c04dace8 00000006
> > be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
> > ea4f71a8 c0429420
> > bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
> > 00000400 ffffffff
> > bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
> > c0e5a2a0 c0e5a2a0
> > bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
> > c0e5a2a0 c0269470
> > bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
> > ea537280 0000000e
> > bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
> > c09c9ed0 ea54bf5c
> > bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
> > c09c9ed0 ea537200
> > bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
> > ea4f71a8 c0426524
> > bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
> > 00000000 00000000
> > bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
> > 00000000 00000000
> > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > 00000000 00000000
> > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > 00000000 00000000
> > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > (__check_object_size+0x2d8/0x448)
> > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > (build_test_sglist+0x268/0x2d8)
> > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > (test_hash_vec_cfg+0x110/0x694)
> > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > (__alg_test_hash+0x158/0x1b8)
> > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > Exception stack(0xea54bfb0 to 0xea54bff8)
> > bfa0: 00000000 00000000
> > 00000000 00000000
> > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > 00000000 00000000
> > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
> > ---[ end trace 190b3cf48e720f78 ]---
> > BUG: sleeping function called from invalid context at
> > include/linux/percpu-rwsem.h:34
> > in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
> > CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
> > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
> > [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
> > [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
> > [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
> > [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
> > [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
> > [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
> > [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
> > Exception stack(0xea54bc10 to 0xea54bc58)
> > bc00: 0000005f 2abb4000
> > dd6cd422 dd6cd422
> > bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
> > 00000000 cccccccd
> > bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
> > [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
> > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > (__check_object_size+0x2d8/0x448)
> > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > (build_test_sglist+0x268/0x2d8)
> > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > (test_hash_vec_cfg+0x110/0x694)
> > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > (__alg_test_hash+0x158/0x1b8)
> > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > Exception stack(0xea54bfb0 to 0xea54bff8)
> > bfa0: 00000000 00000000
> > 00000000 00000000
> > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > 00000000 00000000
> > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> >
>
> Well, this must happen with the new (in 5.1) crypto self-tests implementation
> for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> understand why hardened usercopy considers it a bug though, as there's no buffer
> overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> buffer that was allocated with __get_free_pages():
>
> __get_free_pages(GFP_KERNEL, 1)
>
> ... where 1 means an order-1 allocation.
>
> If it copies to offset=4064 len=42, for example, then hardened usercopy
> considers it a bug even though the buffer is 8192 bytes long. Why?
>
> It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> with ITER_KVEC.
>
> - Eric
Kees, any thoughts on why hardened usercopy rejects copies spanning a page
boundary when they seem to be fine?
- Eric
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-03-20 18:57 ` Eric Biggers
@ 2019-03-21 17:45 ` Kees Cook
-1 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-03-21 17:45 UTC (permalink / raw)
To: Eric Biggers
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List
On Wed, Mar 20, 2019 at 11:57 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> > On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > > When running the sha1-asm crypto selftest on arm with
> > > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> > >
> > > usercopy: Kernel memory overwrite attempt detected to spans
> > > multiple pages (offset 0, size 42)!
> > > ------------[ cut here ]------------
> > > kernel BUG at mm/usercopy.c:102!
> > > Internal error: Oops - BUG: 0 [#1] SMP ARM
> > > Modules linked in:
> > > CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
> > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > PC is at usercopy_abort+0x68/0x90
> > > LR is at usercopy_abort+0x68/0x90
> > > pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
> > > sp : ea54bc60 ip : 00000010 fp : cccccccd
> > > r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
> > > r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
> > > r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
> > > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > > Control: 30c5387d Table: 40003000 DAC: fffffffd
> > > Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
> > > Stack: (0xea54bc60 to 0xea54c000)
> > > bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
> > > 00000000 c0310060
> > > bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
> > > ea54cfe0 ea538c00
> > > bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
> > > 0000003a c0427a30
> > > bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
> > > 0000002a c081cf80
> > > bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
> > > c0e0a408 eb074240
> > > bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
> > > ebef7480 eb074200
> > > bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
> > > c084061c c0428c38
> > > bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
> > > 00000002 eabe4e80
> > > bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
> > > 00000006 c09d544c
> > > bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
> > > eb074200 00000000
> > > bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
> > > 00000000 c081cf70
> > > bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
> > > c0e4ee80 443e9884
> > > bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
> > > fefefefe fefefefe
> > > be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
> > > ea4f7a00 c03062bc
> > > be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
> > > 00000002 eabe4e40
> > > be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
> > > eb074200 ea538c00
> > > be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
> > > c04dace8 00000006
> > > be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
> > > ea4f71a8 c0429420
> > > bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
> > > 00000400 ffffffff
> > > bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
> > > c0e5a2a0 c0e5a2a0
> > > bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
> > > c0e5a2a0 c0269470
> > > bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
> > > ea537280 0000000e
> > > bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
> > > c09c9ed0 ea54bf5c
> > > bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
> > > c09c9ed0 ea537200
> > > bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
> > > ea4f71a8 c0426524
> > > bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
> > > 00000000 00000000
> > > bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
> > > 00000000 00000000
> > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000 00000000
> > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > 00000000 00000000
> > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > (__check_object_size+0x2d8/0x448)
> > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > (build_test_sglist+0x268/0x2d8)
> > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > (test_hash_vec_cfg+0x110/0x694)
> > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > (__alg_test_hash+0x158/0x1b8)
> > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > bfa0: 00000000 00000000
> > > 00000000 00000000
> > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000 00000000
> > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
> > > ---[ end trace 190b3cf48e720f78 ]---
> > > BUG: sleeping function called from invalid context at
> > > include/linux/percpu-rwsem.h:34
> > > in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
> > > CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
> > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
> > > [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
> > > [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
> > > [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
> > > [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
> > > [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
> > > [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
> > > [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
> > > Exception stack(0xea54bc10 to 0xea54bc58)
> > > bc00: 0000005f 2abb4000
> > > dd6cd422 dd6cd422
> > > bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
> > > 00000000 cccccccd
> > > bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
> > > [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
> > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > (__check_object_size+0x2d8/0x448)
> > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > (build_test_sglist+0x268/0x2d8)
> > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > (test_hash_vec_cfg+0x110/0x694)
> > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > (__alg_test_hash+0x158/0x1b8)
> > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > bfa0: 00000000 00000000
> > > 00000000 00000000
> > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000 00000000
> > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > >
> >
> > Well, this must happen with the new (in 5.1) crypto self-tests implementation
> > for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> > understand why hardened usercopy considers it a bug though, as there's no buffer
> > overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> > buffer that was allocated with __get_free_pages():
> >
> > __get_free_pages(GFP_KERNEL, 1)
> >
> > ... where 1 means an order-1 allocation.
> >
> > If it copies to offset=4064 len=42, for example, then hardened usercopy
> > considers it a bug even though the buffer is 8192 bytes long. Why?
> >
> > It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> > with ITER_KVEC.
> >
> > - Eric
>
> Kees, any thoughts on why hardened usercopy rejects copies spanning a page
> boundary when they seem to be fine?
This is due to missing the compound page marking, if I remember
correctly. However, I tend to leave the pagespan test disabled: it
really isn't ready for production use -- there are a lot of missing
annotations still.
--
Kees Cook
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-03-21 17:45 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-03-21 17:45 UTC (permalink / raw)
To: Eric Biggers
Cc: Herbert Xu, Linux Kernel Mailing List, linux-security-module,
Geert Uytterhoeven, Linux Crypto Mailing List, Linux ARM
On Wed, Mar 20, 2019 at 11:57 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> > On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > > When running the sha1-asm crypto selftest on arm with
> > > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> > >
> > > usercopy: Kernel memory overwrite attempt detected to spans
> > > multiple pages (offset 0, size 42)!
> > > ------------[ cut here ]------------
> > > kernel BUG at mm/usercopy.c:102!
> > > Internal error: Oops - BUG: 0 [#1] SMP ARM
> > > Modules linked in:
> > > CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
> > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > PC is at usercopy_abort+0x68/0x90
> > > LR is at usercopy_abort+0x68/0x90
> > > pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
> > > sp : ea54bc60 ip : 00000010 fp : cccccccd
> > > r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
> > > r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
> > > r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
> > > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > > Control: 30c5387d Table: 40003000 DAC: fffffffd
> > > Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
> > > Stack: (0xea54bc60 to 0xea54c000)
> > > bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
> > > 00000000 c0310060
> > > bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
> > > ea54cfe0 ea538c00
> > > bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
> > > 0000003a c0427a30
> > > bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
> > > 0000002a c081cf80
> > > bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
> > > c0e0a408 eb074240
> > > bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
> > > ebef7480 eb074200
> > > bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
> > > c084061c c0428c38
> > > bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
> > > 00000002 eabe4e80
> > > bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
> > > 00000006 c09d544c
> > > bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
> > > eb074200 00000000
> > > bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
> > > 00000000 c081cf70
> > > bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
> > > c0e4ee80 443e9884
> > > bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
> > > fefefefe fefefefe
> > > be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
> > > ea4f7a00 c03062bc
> > > be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
> > > 00000002 eabe4e40
> > > be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
> > > eb074200 ea538c00
> > > be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
> > > c04dace8 00000006
> > > be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
> > > ea4f71a8 c0429420
> > > bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
> > > 00000400 ffffffff
> > > bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
> > > c0e5a2a0 c0e5a2a0
> > > bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
> > > c0e5a2a0 c0269470
> > > bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
> > > ea537280 0000000e
> > > bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
> > > c09c9ed0 ea54bf5c
> > > bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
> > > c09c9ed0 ea537200
> > > bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
> > > ea4f71a8 c0426524
> > > bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
> > > 00000000 00000000
> > > bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
> > > 00000000 00000000
> > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000 00000000
> > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > 00000000 00000000
> > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > (__check_object_size+0x2d8/0x448)
> > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > (build_test_sglist+0x268/0x2d8)
> > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > (test_hash_vec_cfg+0x110/0x694)
> > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > (__alg_test_hash+0x158/0x1b8)
> > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > bfa0: 00000000 00000000
> > > 00000000 00000000
> > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000 00000000
> > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
> > > ---[ end trace 190b3cf48e720f78 ]---
> > > BUG: sleeping function called from invalid context at
> > > include/linux/percpu-rwsem.h:34
> > > in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
> > > CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
> > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
> > > [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
> > > [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
> > > [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
> > > [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
> > > [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
> > > [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
> > > [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
> > > Exception stack(0xea54bc10 to 0xea54bc58)
> > > bc00: 0000005f 2abb4000
> > > dd6cd422 dd6cd422
> > > bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
> > > 00000000 cccccccd
> > > bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
> > > [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
> > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > (__check_object_size+0x2d8/0x448)
> > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > (build_test_sglist+0x268/0x2d8)
> > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > (test_hash_vec_cfg+0x110/0x694)
> > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > (__alg_test_hash+0x158/0x1b8)
> > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > bfa0: 00000000 00000000
> > > 00000000 00000000
> > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000 00000000
> > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > >
> >
> > Well, this must happen with the new (in 5.1) crypto self-tests implementation
> > for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> > understand why hardened usercopy considers it a bug though, as there's no buffer
> > overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> > buffer that was allocated with __get_free_pages():
> >
> > __get_free_pages(GFP_KERNEL, 1)
> >
> > ... where 1 means an order-1 allocation.
> >
> > If it copies to offset=4064 len=42, for example, then hardened usercopy
> > considers it a bug even though the buffer is 8192 bytes long. Why?
> >
> > It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> > with ITER_KVEC.
> >
> > - Eric
>
> Kees, any thoughts on why hardened usercopy rejects copies spanning a page
> boundary when they seem to be fine?
This is due to missing the compound page marking, if I remember
correctly. However, I tend to leave the pagespan test disabled: it
really isn't ready for production use -- there are a lot of missing
annotations still.
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-03-21 17:45 ` Kees Cook
@ 2019-03-21 17:51 ` Eric Biggers
-1 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-03-21 17:51 UTC (permalink / raw)
To: Kees Cook
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List
On Thu, Mar 21, 2019 at 10:45:31AM -0700, Kees Cook wrote:
> On Wed, Mar 20, 2019 at 11:57 AM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> > > On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > > > When running the sha1-asm crypto selftest on arm with
> > > > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> > > >
> > > > usercopy: Kernel memory overwrite attempt detected to spans
> > > > multiple pages (offset 0, size 42)!
> > > > ------------[ cut here ]------------
> > > > kernel BUG at mm/usercopy.c:102!
> > > > Internal error: Oops - BUG: 0 [#1] SMP ARM
> > > > Modules linked in:
> > > > CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
> > > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > > PC is at usercopy_abort+0x68/0x90
> > > > LR is at usercopy_abort+0x68/0x90
> > > > pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
> > > > sp : ea54bc60 ip : 00000010 fp : cccccccd
> > > > r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
> > > > r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
> > > > r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
> > > > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > > > Control: 30c5387d Table: 40003000 DAC: fffffffd
> > > > Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
> > > > Stack: (0xea54bc60 to 0xea54c000)
> > > > bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
> > > > 00000000 c0310060
> > > > bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
> > > > ea54cfe0 ea538c00
> > > > bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
> > > > 0000003a c0427a30
> > > > bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
> > > > 0000002a c081cf80
> > > > bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
> > > > c0e0a408 eb074240
> > > > bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
> > > > ebef7480 eb074200
> > > > bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
> > > > c084061c c0428c38
> > > > bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
> > > > 00000002 eabe4e80
> > > > bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
> > > > 00000006 c09d544c
> > > > bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
> > > > eb074200 00000000
> > > > bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
> > > > 00000000 c081cf70
> > > > bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
> > > > c0e4ee80 443e9884
> > > > bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
> > > > fefefefe fefefefe
> > > > be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
> > > > ea4f7a00 c03062bc
> > > > be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
> > > > 00000002 eabe4e40
> > > > be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
> > > > eb074200 ea538c00
> > > > be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
> > > > c04dace8 00000006
> > > > be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
> > > > ea4f71a8 c0429420
> > > > bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
> > > > 00000400 ffffffff
> > > > bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
> > > > c0e5a2a0 c0e5a2a0
> > > > bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
> > > > c0e5a2a0 c0269470
> > > > bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
> > > > ea537280 0000000e
> > > > bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
> > > > c09c9ed0 ea54bf5c
> > > > bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
> > > > c09c9ed0 ea537200
> > > > bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
> > > > ea4f71a8 c0426524
> > > > bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
> > > > 00000000 00000000
> > > > bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
> > > > 00000000 00000000
> > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > 00000000 00000000
> > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > 00000000 00000000
> > > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > > (__check_object_size+0x2d8/0x448)
> > > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > > (build_test_sglist+0x268/0x2d8)
> > > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > > (test_hash_vec_cfg+0x110/0x694)
> > > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > > (__alg_test_hash+0x158/0x1b8)
> > > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > > bfa0: 00000000 00000000
> > > > 00000000 00000000
> > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > 00000000 00000000
> > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
> > > > ---[ end trace 190b3cf48e720f78 ]---
> > > > BUG: sleeping function called from invalid context at
> > > > include/linux/percpu-rwsem.h:34
> > > > in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
> > > > CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
> > > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > > [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
> > > > [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
> > > > [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
> > > > [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
> > > > [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
> > > > [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
> > > > [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
> > > > [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
> > > > Exception stack(0xea54bc10 to 0xea54bc58)
> > > > bc00: 0000005f 2abb4000
> > > > dd6cd422 dd6cd422
> > > > bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
> > > > 00000000 cccccccd
> > > > bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
> > > > [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
> > > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > > (__check_object_size+0x2d8/0x448)
> > > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > > (build_test_sglist+0x268/0x2d8)
> > > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > > (test_hash_vec_cfg+0x110/0x694)
> > > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > > (__alg_test_hash+0x158/0x1b8)
> > > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > > bfa0: 00000000 00000000
> > > > 00000000 00000000
> > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > 00000000 00000000
> > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > >
> > >
> > > Well, this must happen with the new (in 5.1) crypto self-tests implementation
> > > for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> > > understand why hardened usercopy considers it a bug though, as there's no buffer
> > > overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> > > buffer that was allocated with __get_free_pages():
> > >
> > > __get_free_pages(GFP_KERNEL, 1)
> > >
> > > ... where 1 means an order-1 allocation.
> > >
> > > If it copies to offset=4064 len=42, for example, then hardened usercopy
> > > considers it a bug even though the buffer is 8192 bytes long. Why?
> > >
> > > It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> > > with ITER_KVEC.
> > >
> > > - Eric
> >
> > Kees, any thoughts on why hardened usercopy rejects copies spanning a page
> > boundary when they seem to be fine?
>
> This is due to missing the compound page marking, if I remember
> correctly. However, I tend to leave the pagespan test disabled: it
> really isn't ready for production use -- there are a lot of missing
> annotations still.
>
So do I need to add __GFP_COMP? Is there any actual reason to do so?
Why does hardened usercopy check for it?
- Eric
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-03-21 17:51 ` Eric Biggers
0 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-03-21 17:51 UTC (permalink / raw)
To: Kees Cook
Cc: Herbert Xu, Linux Kernel Mailing List, linux-security-module,
Geert Uytterhoeven, Linux Crypto Mailing List, Linux ARM
On Thu, Mar 21, 2019 at 10:45:31AM -0700, Kees Cook wrote:
> On Wed, Mar 20, 2019 at 11:57 AM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> > > On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > > > When running the sha1-asm crypto selftest on arm with
> > > > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> > > >
> > > > usercopy: Kernel memory overwrite attempt detected to spans
> > > > multiple pages (offset 0, size 42)!
> > > > ------------[ cut here ]------------
> > > > kernel BUG at mm/usercopy.c:102!
> > > > Internal error: Oops - BUG: 0 [#1] SMP ARM
> > > > Modules linked in:
> > > > CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
> > > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > > PC is at usercopy_abort+0x68/0x90
> > > > LR is at usercopy_abort+0x68/0x90
> > > > pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
> > > > sp : ea54bc60 ip : 00000010 fp : cccccccd
> > > > r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
> > > > r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
> > > > r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
> > > > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > > > Control: 30c5387d Table: 40003000 DAC: fffffffd
> > > > Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
> > > > Stack: (0xea54bc60 to 0xea54c000)
> > > > bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
> > > > 00000000 c0310060
> > > > bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
> > > > ea54cfe0 ea538c00
> > > > bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
> > > > 0000003a c0427a30
> > > > bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
> > > > 0000002a c081cf80
> > > > bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
> > > > c0e0a408 eb074240
> > > > bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
> > > > ebef7480 eb074200
> > > > bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
> > > > c084061c c0428c38
> > > > bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
> > > > 00000002 eabe4e80
> > > > bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
> > > > 00000006 c09d544c
> > > > bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
> > > > eb074200 00000000
> > > > bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
> > > > 00000000 c081cf70
> > > > bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
> > > > c0e4ee80 443e9884
> > > > bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
> > > > fefefefe fefefefe
> > > > be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
> > > > ea4f7a00 c03062bc
> > > > be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
> > > > 00000002 eabe4e40
> > > > be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
> > > > eb074200 ea538c00
> > > > be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
> > > > c04dace8 00000006
> > > > be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
> > > > ea4f71a8 c0429420
> > > > bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
> > > > 00000400 ffffffff
> > > > bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
> > > > c0e5a2a0 c0e5a2a0
> > > > bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
> > > > c0e5a2a0 c0269470
> > > > bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
> > > > ea537280 0000000e
> > > > bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
> > > > c09c9ed0 ea54bf5c
> > > > bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
> > > > c09c9ed0 ea537200
> > > > bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
> > > > ea4f71a8 c0426524
> > > > bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
> > > > 00000000 00000000
> > > > bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
> > > > 00000000 00000000
> > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > 00000000 00000000
> > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > 00000000 00000000
> > > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > > (__check_object_size+0x2d8/0x448)
> > > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > > (build_test_sglist+0x268/0x2d8)
> > > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > > (test_hash_vec_cfg+0x110/0x694)
> > > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > > (__alg_test_hash+0x158/0x1b8)
> > > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > > bfa0: 00000000 00000000
> > > > 00000000 00000000
> > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > 00000000 00000000
> > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
> > > > ---[ end trace 190b3cf48e720f78 ]---
> > > > BUG: sleeping function called from invalid context at
> > > > include/linux/percpu-rwsem.h:34
> > > > in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
> > > > CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
> > > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > > [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
> > > > [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
> > > > [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
> > > > [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
> > > > [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
> > > > [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
> > > > [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
> > > > [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
> > > > Exception stack(0xea54bc10 to 0xea54bc58)
> > > > bc00: 0000005f 2abb4000
> > > > dd6cd422 dd6cd422
> > > > bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
> > > > 00000000 cccccccd
> > > > bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
> > > > [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
> > > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > > (__check_object_size+0x2d8/0x448)
> > > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > > (build_test_sglist+0x268/0x2d8)
> > > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > > (test_hash_vec_cfg+0x110/0x694)
> > > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > > (__alg_test_hash+0x158/0x1b8)
> > > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > > bfa0: 00000000 00000000
> > > > 00000000 00000000
> > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > 00000000 00000000
> > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > >
> > >
> > > Well, this must happen with the new (in 5.1) crypto self-tests implementation
> > > for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> > > understand why hardened usercopy considers it a bug though, as there's no buffer
> > > overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> > > buffer that was allocated with __get_free_pages():
> > >
> > > __get_free_pages(GFP_KERNEL, 1)
> > >
> > > ... where 1 means an order-1 allocation.
> > >
> > > If it copies to offset=4064 len=42, for example, then hardened usercopy
> > > considers it a bug even though the buffer is 8192 bytes long. Why?
> > >
> > > It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> > > with ITER_KVEC.
> > >
> > > - Eric
> >
> > Kees, any thoughts on why hardened usercopy rejects copies spanning a page
> > boundary when they seem to be fine?
>
> This is due to missing the compound page marking, if I remember
> correctly. However, I tend to leave the pagespan test disabled: it
> really isn't ready for production use -- there are a lot of missing
> annotations still.
>
So do I need to add __GFP_COMP? Is there any actual reason to do so?
Why does hardened usercopy check for it?
- Eric
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-03-21 17:51 ` Eric Biggers
@ 2019-04-10 3:17 ` Eric Biggers
-1 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-10 3:17 UTC (permalink / raw)
To: Kees Cook
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List
On Thu, Mar 21, 2019 at 10:51:22AM -0700, Eric Biggers wrote:
> On Thu, Mar 21, 2019 at 10:45:31AM -0700, Kees Cook wrote:
> > On Wed, Mar 20, 2019 at 11:57 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > >
> > > On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> > > > On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > > > > When running the sha1-asm crypto selftest on arm with
> > > > > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> > > > >
> > > > > usercopy: Kernel memory overwrite attempt detected to spans
> > > > > multiple pages (offset 0, size 42)!
> > > > > ------------[ cut here ]------------
> > > > > kernel BUG at mm/usercopy.c:102!
> > > > > Internal error: Oops - BUG: 0 [#1] SMP ARM
> > > > > Modules linked in:
> > > > > CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
> > > > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > > > PC is at usercopy_abort+0x68/0x90
> > > > > LR is at usercopy_abort+0x68/0x90
> > > > > pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
> > > > > sp : ea54bc60 ip : 00000010 fp : cccccccd
> > > > > r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
> > > > > r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
> > > > > r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
> > > > > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > > > > Control: 30c5387d Table: 40003000 DAC: fffffffd
> > > > > Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
> > > > > Stack: (0xea54bc60 to 0xea54c000)
> > > > > bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
> > > > > 00000000 c0310060
> > > > > bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
> > > > > ea54cfe0 ea538c00
> > > > > bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
> > > > > 0000003a c0427a30
> > > > > bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
> > > > > 0000002a c081cf80
> > > > > bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
> > > > > c0e0a408 eb074240
> > > > > bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
> > > > > ebef7480 eb074200
> > > > > bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
> > > > > c084061c c0428c38
> > > > > bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
> > > > > 00000002 eabe4e80
> > > > > bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
> > > > > 00000006 c09d544c
> > > > > bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
> > > > > eb074200 00000000
> > > > > bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
> > > > > 00000000 c081cf70
> > > > > bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
> > > > > c0e4ee80 443e9884
> > > > > bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
> > > > > fefefefe fefefefe
> > > > > be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
> > > > > ea4f7a00 c03062bc
> > > > > be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
> > > > > 00000002 eabe4e40
> > > > > be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
> > > > > eb074200 ea538c00
> > > > > be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
> > > > > c04dace8 00000006
> > > > > be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
> > > > > ea4f71a8 c0429420
> > > > > bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
> > > > > 00000400 ffffffff
> > > > > bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
> > > > > c0e5a2a0 c0e5a2a0
> > > > > bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
> > > > > c0e5a2a0 c0269470
> > > > > bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
> > > > > ea537280 0000000e
> > > > > bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
> > > > > c09c9ed0 ea54bf5c
> > > > > bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
> > > > > c09c9ed0 ea537200
> > > > > bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
> > > > > ea4f71a8 c0426524
> > > > > bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
> > > > > 00000000 00000000
> > > > > bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > > 00000000 00000000
> > > > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > > > (__check_object_size+0x2d8/0x448)
> > > > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > > > (build_test_sglist+0x268/0x2d8)
> > > > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > > > (test_hash_vec_cfg+0x110/0x694)
> > > > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > > > (__alg_test_hash+0x158/0x1b8)
> > > > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > > > bfa0: 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > > Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
> > > > > ---[ end trace 190b3cf48e720f78 ]---
> > > > > BUG: sleeping function called from invalid context at
> > > > > include/linux/percpu-rwsem.h:34
> > > > > in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
> > > > > CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
> > > > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > > > [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
> > > > > [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
> > > > > [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
> > > > > [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
> > > > > [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
> > > > > [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
> > > > > [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
> > > > > [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
> > > > > Exception stack(0xea54bc10 to 0xea54bc58)
> > > > > bc00: 0000005f 2abb4000
> > > > > dd6cd422 dd6cd422
> > > > > bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
> > > > > 00000000 cccccccd
> > > > > bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
> > > > > [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
> > > > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > > > (__check_object_size+0x2d8/0x448)
> > > > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > > > (build_test_sglist+0x268/0x2d8)
> > > > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > > > (test_hash_vec_cfg+0x110/0x694)
> > > > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > > > (__alg_test_hash+0x158/0x1b8)
> > > > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > > > bfa0: 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > >
> > > >
> > > > Well, this must happen with the new (in 5.1) crypto self-tests implementation
> > > > for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> > > > understand why hardened usercopy considers it a bug though, as there's no buffer
> > > > overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> > > > buffer that was allocated with __get_free_pages():
> > > >
> > > > __get_free_pages(GFP_KERNEL, 1)
> > > >
> > > > ... where 1 means an order-1 allocation.
> > > >
> > > > If it copies to offset=4064 len=42, for example, then hardened usercopy
> > > > considers it a bug even though the buffer is 8192 bytes long. Why?
> > > >
> > > > It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> > > > with ITER_KVEC.
> > > >
> > > > - Eric
> > >
> > > Kees, any thoughts on why hardened usercopy rejects copies spanning a page
> > > boundary when they seem to be fine?
> >
> > This is due to missing the compound page marking, if I remember
> > correctly. However, I tend to leave the pagespan test disabled: it
> > really isn't ready for production use -- there are a lot of missing
> > annotations still.
> >
>
> So do I need to add __GFP_COMP? Is there any actual reason to do so?
> Why does hardened usercopy check for it?
>
> - Eric
Hi Kees, any answer to this question?
- Eric
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-04-10 3:17 ` Eric Biggers
0 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-10 3:17 UTC (permalink / raw)
To: Kees Cook
Cc: Herbert Xu, Linux Kernel Mailing List, linux-security-module,
Geert Uytterhoeven, Linux Crypto Mailing List, Linux ARM
On Thu, Mar 21, 2019 at 10:51:22AM -0700, Eric Biggers wrote:
> On Thu, Mar 21, 2019 at 10:45:31AM -0700, Kees Cook wrote:
> > On Wed, Mar 20, 2019 at 11:57 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > >
> > > On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> > > > On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > > > > When running the sha1-asm crypto selftest on arm with
> > > > > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> > > > >
> > > > > usercopy: Kernel memory overwrite attempt detected to spans
> > > > > multiple pages (offset 0, size 42)!
> > > > > ------------[ cut here ]------------
> > > > > kernel BUG at mm/usercopy.c:102!
> > > > > Internal error: Oops - BUG: 0 [#1] SMP ARM
> > > > > Modules linked in:
> > > > > CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
> > > > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > > > PC is at usercopy_abort+0x68/0x90
> > > > > LR is at usercopy_abort+0x68/0x90
> > > > > pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
> > > > > sp : ea54bc60 ip : 00000010 fp : cccccccd
> > > > > r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
> > > > > r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
> > > > > r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
> > > > > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > > > > Control: 30c5387d Table: 40003000 DAC: fffffffd
> > > > > Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
> > > > > Stack: (0xea54bc60 to 0xea54c000)
> > > > > bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
> > > > > 00000000 c0310060
> > > > > bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
> > > > > ea54cfe0 ea538c00
> > > > > bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
> > > > > 0000003a c0427a30
> > > > > bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
> > > > > 0000002a c081cf80
> > > > > bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
> > > > > c0e0a408 eb074240
> > > > > bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
> > > > > ebef7480 eb074200
> > > > > bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
> > > > > c084061c c0428c38
> > > > > bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
> > > > > 00000002 eabe4e80
> > > > > bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
> > > > > 00000006 c09d544c
> > > > > bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
> > > > > eb074200 00000000
> > > > > bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
> > > > > 00000000 c081cf70
> > > > > bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
> > > > > c0e4ee80 443e9884
> > > > > bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
> > > > > fefefefe fefefefe
> > > > > be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
> > > > > ea4f7a00 c03062bc
> > > > > be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
> > > > > 00000002 eabe4e40
> > > > > be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
> > > > > eb074200 ea538c00
> > > > > be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
> > > > > c04dace8 00000006
> > > > > be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
> > > > > ea4f71a8 c0429420
> > > > > bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
> > > > > 00000400 ffffffff
> > > > > bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
> > > > > c0e5a2a0 c0e5a2a0
> > > > > bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
> > > > > c0e5a2a0 c0269470
> > > > > bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
> > > > > ea537280 0000000e
> > > > > bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
> > > > > c09c9ed0 ea54bf5c
> > > > > bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
> > > > > c09c9ed0 ea537200
> > > > > bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
> > > > > ea4f71a8 c0426524
> > > > > bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
> > > > > 00000000 00000000
> > > > > bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > > 00000000 00000000
> > > > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > > > (__check_object_size+0x2d8/0x448)
> > > > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > > > (build_test_sglist+0x268/0x2d8)
> > > > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > > > (test_hash_vec_cfg+0x110/0x694)
> > > > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > > > (__alg_test_hash+0x158/0x1b8)
> > > > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > > > bfa0: 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > > Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
> > > > > ---[ end trace 190b3cf48e720f78 ]---
> > > > > BUG: sleeping function called from invalid context at
> > > > > include/linux/percpu-rwsem.h:34
> > > > > in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
> > > > > CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
> > > > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > > > [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
> > > > > [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
> > > > > [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
> > > > > [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
> > > > > [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
> > > > > [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
> > > > > [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
> > > > > [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
> > > > > Exception stack(0xea54bc10 to 0xea54bc58)
> > > > > bc00: 0000005f 2abb4000
> > > > > dd6cd422 dd6cd422
> > > > > bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
> > > > > 00000000 cccccccd
> > > > > bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
> > > > > [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
> > > > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > > > (__check_object_size+0x2d8/0x448)
> > > > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > > > (build_test_sglist+0x268/0x2d8)
> > > > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > > > (test_hash_vec_cfg+0x110/0x694)
> > > > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > > > (__alg_test_hash+0x158/0x1b8)
> > > > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > > > bfa0: 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > >
> > > >
> > > > Well, this must happen with the new (in 5.1) crypto self-tests implementation
> > > > for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> > > > understand why hardened usercopy considers it a bug though, as there's no buffer
> > > > overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> > > > buffer that was allocated with __get_free_pages():
> > > >
> > > > __get_free_pages(GFP_KERNEL, 1)
> > > >
> > > > ... where 1 means an order-1 allocation.
> > > >
> > > > If it copies to offset=4064 len=42, for example, then hardened usercopy
> > > > considers it a bug even though the buffer is 8192 bytes long. Why?
> > > >
> > > > It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> > > > with ITER_KVEC.
> > > >
> > > > - Eric
> > >
> > > Kees, any thoughts on why hardened usercopy rejects copies spanning a page
> > > boundary when they seem to be fine?
> >
> > This is due to missing the compound page marking, if I remember
> > correctly. However, I tend to leave the pagespan test disabled: it
> > really isn't ready for production use -- there are a lot of missing
> > annotations still.
> >
>
> So do I need to add __GFP_COMP? Is there any actual reason to do so?
> Why does hardened usercopy check for it?
>
> - Eric
Hi Kees, any answer to this question?
- Eric
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-04-10 3:17 ` Eric Biggers
@ 2019-04-10 18:30 ` Kees Cook
-1 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-10 18:30 UTC (permalink / raw)
To: Eric Biggers
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List
On Tue, Apr 9, 2019 at 8:17 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Thu, Mar 21, 2019 at 10:51:22AM -0700, Eric Biggers wrote:
> > On Thu, Mar 21, 2019 at 10:45:31AM -0700, Kees Cook wrote:
> > > On Wed, Mar 20, 2019 at 11:57 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > >
> > > > On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> > > > > On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > > > > > When running the sha1-asm crypto selftest on arm with
> > > > > > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> > > > > >
> > > > > > usercopy: Kernel memory overwrite attempt detected to spans
> > > > > > multiple pages (offset 0, size 42)!
> > > > >
> > > > > Well, this must happen with the new (in 5.1) crypto self-tests implementation
> > > > > for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> > > > > understand why hardened usercopy considers it a bug though, as there's no buffer
> > > > > overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> > > > > buffer that was allocated with __get_free_pages():
> > > > >
> > > > > __get_free_pages(GFP_KERNEL, 1)
> > > > >
> > > > > ... where 1 means an order-1 allocation.
> > > > >
> > > > > If it copies to offset=4064 len=42, for example, then hardened usercopy
> > > > > considers it a bug even though the buffer is 8192 bytes long. Why?
> > > > >
> > > > > It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> > > > > with ITER_KVEC.
> > > > >
> > > > > - Eric
> > > >
> > > > Kees, any thoughts on why hardened usercopy rejects copies spanning a page
> > > > boundary when they seem to be fine?
> > >
> > > This is due to missing the compound page marking, if I remember
> > > correctly. However, I tend to leave the pagespan test disabled: it
> > > really isn't ready for production use -- there are a lot of missing
> > > annotations still.
> > >
> >
> > So do I need to add __GFP_COMP? Is there any actual reason to do so?
> > Why does hardened usercopy check for it?
> >
> > - Eric
>
> Hi Kees, any answer to this question?
Hi! Sorry, this got lost in my inbox. Yes, if you can add __GFP_COMP,
that would fix this case. No one has had time lately to track down all
these cases, but avoiding adding new ones would be wonderful. :)
It's in there because it's a state I'd like to get to in the kernel,
but it'll require a lot more work to get there.
--
Kees Cook
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-04-10 18:30 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-10 18:30 UTC (permalink / raw)
To: Eric Biggers
Cc: Herbert Xu, Linux Kernel Mailing List, linux-security-module,
Geert Uytterhoeven, Linux Crypto Mailing List, Linux ARM
On Tue, Apr 9, 2019 at 8:17 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Thu, Mar 21, 2019 at 10:51:22AM -0700, Eric Biggers wrote:
> > On Thu, Mar 21, 2019 at 10:45:31AM -0700, Kees Cook wrote:
> > > On Wed, Mar 20, 2019 at 11:57 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > >
> > > > On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> > > > > On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > > > > > When running the sha1-asm crypto selftest on arm with
> > > > > > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> > > > > >
> > > > > > usercopy: Kernel memory overwrite attempt detected to spans
> > > > > > multiple pages (offset 0, size 42)!
> > > > >
> > > > > Well, this must happen with the new (in 5.1) crypto self-tests implementation
> > > > > for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> > > > > understand why hardened usercopy considers it a bug though, as there's no buffer
> > > > > overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> > > > > buffer that was allocated with __get_free_pages():
> > > > >
> > > > > __get_free_pages(GFP_KERNEL, 1)
> > > > >
> > > > > ... where 1 means an order-1 allocation.
> > > > >
> > > > > If it copies to offset=4064 len=42, for example, then hardened usercopy
> > > > > considers it a bug even though the buffer is 8192 bytes long. Why?
> > > > >
> > > > > It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> > > > > with ITER_KVEC.
> > > > >
> > > > > - Eric
> > > >
> > > > Kees, any thoughts on why hardened usercopy rejects copies spanning a page
> > > > boundary when they seem to be fine?
> > >
> > > This is due to missing the compound page marking, if I remember
> > > correctly. However, I tend to leave the pagespan test disabled: it
> > > really isn't ready for production use -- there are a lot of missing
> > > annotations still.
> > >
> >
> > So do I need to add __GFP_COMP? Is there any actual reason to do so?
> > Why does hardened usercopy check for it?
> >
> > - Eric
>
> Hi Kees, any answer to this question?
Hi! Sorry, this got lost in my inbox. Yes, if you can add __GFP_COMP,
that would fix this case. No one has had time lately to track down all
these cases, but avoiding adding new ones would be wonderful. :)
It's in there because it's a state I'd like to get to in the kernel,
but it'll require a lot more work to get there.
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-04-10 18:30 ` Kees Cook
@ 2019-04-10 19:07 ` Eric Biggers
-1 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-10 19:07 UTC (permalink / raw)
To: Kees Cook
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List
On Wed, Apr 10, 2019 at 11:30:57AM -0700, Kees Cook wrote:
> On Tue, Apr 9, 2019 at 8:17 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Thu, Mar 21, 2019 at 10:51:22AM -0700, Eric Biggers wrote:
> > > On Thu, Mar 21, 2019 at 10:45:31AM -0700, Kees Cook wrote:
> > > > On Wed, Mar 20, 2019 at 11:57 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > > >
> > > > > On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> > > > > > On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > > > > > > When running the sha1-asm crypto selftest on arm with
> > > > > > > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> > > > > > >
> > > > > > > usercopy: Kernel memory overwrite attempt detected to spans
> > > > > > > multiple pages (offset 0, size 42)!
> > > > > >
> > > > > > Well, this must happen with the new (in 5.1) crypto self-tests implementation
> > > > > > for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> > > > > > understand why hardened usercopy considers it a bug though, as there's no buffer
> > > > > > overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> > > > > > buffer that was allocated with __get_free_pages():
> > > > > >
> > > > > > __get_free_pages(GFP_KERNEL, 1)
> > > > > >
> > > > > > ... where 1 means an order-1 allocation.
> > > > > >
> > > > > > If it copies to offset=4064 len=42, for example, then hardened usercopy
> > > > > > considers it a bug even though the buffer is 8192 bytes long. Why?
> > > > > >
> > > > > > It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> > > > > > with ITER_KVEC.
> > > > > >
> > > > > > - Eric
> > > > >
> > > > > Kees, any thoughts on why hardened usercopy rejects copies spanning a page
> > > > > boundary when they seem to be fine?
> > > >
> > > > This is due to missing the compound page marking, if I remember
> > > > correctly. However, I tend to leave the pagespan test disabled: it
> > > > really isn't ready for production use -- there are a lot of missing
> > > > annotations still.
> > > >
> > >
> > > So do I need to add __GFP_COMP? Is there any actual reason to do so?
> > > Why does hardened usercopy check for it?
> > >
> > > - Eric
> >
> > Hi Kees, any answer to this question?
>
> Hi! Sorry, this got lost in my inbox. Yes, if you can add __GFP_COMP,
> that would fix this case. No one has had time lately to track down all
> these cases, but avoiding adding new ones would be wonderful. :)
>
> It's in there because it's a state I'd like to get to in the kernel,
> but it'll require a lot more work to get there.
>
That didn't answer my question. My question is what is the purpose of this? If
there was actual buffer overflow when __GFP_COMP isn't specified that would make
perfect sense, but AFAICS there isn't. So why does hardened usercopy consider
it broken when __GFP_COMP isn't specified?
- Eric
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-04-10 19:07 ` Eric Biggers
0 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-10 19:07 UTC (permalink / raw)
To: Kees Cook
Cc: Herbert Xu, Linux Kernel Mailing List, linux-security-module,
Geert Uytterhoeven, Linux Crypto Mailing List, Linux ARM
On Wed, Apr 10, 2019 at 11:30:57AM -0700, Kees Cook wrote:
> On Tue, Apr 9, 2019 at 8:17 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Thu, Mar 21, 2019 at 10:51:22AM -0700, Eric Biggers wrote:
> > > On Thu, Mar 21, 2019 at 10:45:31AM -0700, Kees Cook wrote:
> > > > On Wed, Mar 20, 2019 at 11:57 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > > >
> > > > > On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> > > > > > On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > > > > > > When running the sha1-asm crypto selftest on arm with
> > > > > > > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> > > > > > >
> > > > > > > usercopy: Kernel memory overwrite attempt detected to spans
> > > > > > > multiple pages (offset 0, size 42)!
> > > > > >
> > > > > > Well, this must happen with the new (in 5.1) crypto self-tests implementation
> > > > > > for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> > > > > > understand why hardened usercopy considers it a bug though, as there's no buffer
> > > > > > overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> > > > > > buffer that was allocated with __get_free_pages():
> > > > > >
> > > > > > __get_free_pages(GFP_KERNEL, 1)
> > > > > >
> > > > > > ... where 1 means an order-1 allocation.
> > > > > >
> > > > > > If it copies to offset=4064 len=42, for example, then hardened usercopy
> > > > > > considers it a bug even though the buffer is 8192 bytes long. Why?
> > > > > >
> > > > > > It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> > > > > > with ITER_KVEC.
> > > > > >
> > > > > > - Eric
> > > > >
> > > > > Kees, any thoughts on why hardened usercopy rejects copies spanning a page
> > > > > boundary when they seem to be fine?
> > > >
> > > > This is due to missing the compound page marking, if I remember
> > > > correctly. However, I tend to leave the pagespan test disabled: it
> > > > really isn't ready for production use -- there are a lot of missing
> > > > annotations still.
> > > >
> > >
> > > So do I need to add __GFP_COMP? Is there any actual reason to do so?
> > > Why does hardened usercopy check for it?
> > >
> > > - Eric
> >
> > Hi Kees, any answer to this question?
>
> Hi! Sorry, this got lost in my inbox. Yes, if you can add __GFP_COMP,
> that would fix this case. No one has had time lately to track down all
> these cases, but avoiding adding new ones would be wonderful. :)
>
> It's in there because it's a state I'd like to get to in the kernel,
> but it'll require a lot more work to get there.
>
That didn't answer my question. My question is what is the purpose of this? If
there was actual buffer overflow when __GFP_COMP isn't specified that would make
perfect sense, but AFAICS there isn't. So why does hardened usercopy consider
it broken when __GFP_COMP isn't specified?
- Eric
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-04-10 19:07 ` Eric Biggers
@ 2019-04-10 21:57 ` Kees Cook
-1 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-10 21:57 UTC (permalink / raw)
To: Eric Biggers
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List,
Laura Abbott, Rik van Riel
On Wed, Apr 10, 2019 at 12:07 PM Eric Biggers <ebiggers@kernel.org> wrote:
> That didn't answer my question. My question is what is the purpose of this? If
> there was actual buffer overflow when __GFP_COMP isn't specified that would make
> perfect sense, but AFAICS there isn't. So why does hardened usercopy consider
> it broken when __GFP_COMP isn't specified?
The goal of CONFIG_HARDENED_USERCOPY_PAGESPAN was to detect copies
across page boundaries in memory allocated by the page allocator.
There appear to be enough cases of allocations that span pages but do
not mark them with __GFP_COMP, so this logic hasn't proven useful in
the real world (which is why no one should use the ..._PAGESPAN config
in production). I'd like to get the kernel to the point where hardened
usercopy can correctly do these checks (right now it's mainly only
useful at checking for overflows in slub and slab), but it'll take
time/focus for a while. No one has had time yet to track all of these
down and fix them. (I defer to Laura and Rik on the design of the
pagespan checks; they did the bulk of the work there.)
Does that help explain it, or am I still missing your question?
--
Kees Cook
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-04-10 21:57 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-10 21:57 UTC (permalink / raw)
To: Eric Biggers
Cc: Herbert Xu, Rik van Riel, Linux Kernel Mailing List,
linux-security-module, Geert Uytterhoeven,
Linux Crypto Mailing List, Laura Abbott, Linux ARM
On Wed, Apr 10, 2019 at 12:07 PM Eric Biggers <ebiggers@kernel.org> wrote:
> That didn't answer my question. My question is what is the purpose of this? If
> there was actual buffer overflow when __GFP_COMP isn't specified that would make
> perfect sense, but AFAICS there isn't. So why does hardened usercopy consider
> it broken when __GFP_COMP isn't specified?
The goal of CONFIG_HARDENED_USERCOPY_PAGESPAN was to detect copies
across page boundaries in memory allocated by the page allocator.
There appear to be enough cases of allocations that span pages but do
not mark them with __GFP_COMP, so this logic hasn't proven useful in
the real world (which is why no one should use the ..._PAGESPAN config
in production). I'd like to get the kernel to the point where hardened
usercopy can correctly do these checks (right now it's mainly only
useful at checking for overflows in slub and slab), but it'll take
time/focus for a while. No one has had time yet to track all of these
down and fix them. (I defer to Laura and Rik on the design of the
pagespan checks; they did the bulk of the work there.)
Does that help explain it, or am I still missing your question?
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-04-10 21:57 ` Kees Cook
@ 2019-04-10 23:11 ` Eric Biggers
-1 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-10 23:11 UTC (permalink / raw)
To: Kees Cook
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List,
Laura Abbott, Rik van Riel
On Wed, Apr 10, 2019 at 02:57:46PM -0700, Kees Cook wrote:
> On Wed, Apr 10, 2019 at 12:07 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > That didn't answer my question. My question is what is the purpose of this? If
> > there was actual buffer overflow when __GFP_COMP isn't specified that would make
> > perfect sense, but AFAICS there isn't. So why does hardened usercopy consider
> > it broken when __GFP_COMP isn't specified?
>
> The goal of CONFIG_HARDENED_USERCOPY_PAGESPAN was to detect copies
> across page boundaries in memory allocated by the page allocator.
> There appear to be enough cases of allocations that span pages but do
> not mark them with __GFP_COMP, so this logic hasn't proven useful in
> the real world (which is why no one should use the ..._PAGESPAN config
> in production). I'd like to get the kernel to the point where hardened
> usercopy can correctly do these checks (right now it's mainly only
> useful at checking for overflows in slub and slab), but it'll take
> time/focus for a while. No one has had time yet to track all of these
> down and fix them. (I defer to Laura and Rik on the design of the
> pagespan checks; they did the bulk of the work there.)
>
> Does that help explain it, or am I still missing your question?
>
> --
> Kees Cook
You've explained *what* it does again, but not *why*. *Why* do you want
hardened usercopy to detect copies across page boundaries, when there is no
actual buffer overflow?
- Eric
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-04-10 23:11 ` Eric Biggers
0 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-10 23:11 UTC (permalink / raw)
To: Kees Cook
Cc: Herbert Xu, Rik van Riel, Linux Kernel Mailing List,
linux-security-module, Geert Uytterhoeven,
Linux Crypto Mailing List, Laura Abbott, Linux ARM
On Wed, Apr 10, 2019 at 02:57:46PM -0700, Kees Cook wrote:
> On Wed, Apr 10, 2019 at 12:07 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > That didn't answer my question. My question is what is the purpose of this? If
> > there was actual buffer overflow when __GFP_COMP isn't specified that would make
> > perfect sense, but AFAICS there isn't. So why does hardened usercopy consider
> > it broken when __GFP_COMP isn't specified?
>
> The goal of CONFIG_HARDENED_USERCOPY_PAGESPAN was to detect copies
> across page boundaries in memory allocated by the page allocator.
> There appear to be enough cases of allocations that span pages but do
> not mark them with __GFP_COMP, so this logic hasn't proven useful in
> the real world (which is why no one should use the ..._PAGESPAN config
> in production). I'd like to get the kernel to the point where hardened
> usercopy can correctly do these checks (right now it's mainly only
> useful at checking for overflows in slub and slab), but it'll take
> time/focus for a while. No one has had time yet to track all of these
> down and fix them. (I defer to Laura and Rik on the design of the
> pagespan checks; they did the bulk of the work there.)
>
> Does that help explain it, or am I still missing your question?
>
> --
> Kees Cook
You've explained *what* it does again, but not *why*. *Why* do you want
hardened usercopy to detect copies across page boundaries, when there is no
actual buffer overflow?
- Eric
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-04-10 23:11 ` Eric Biggers
@ 2019-04-10 23:27 ` Kees Cook
-1 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-10 23:27 UTC (permalink / raw)
To: Eric Biggers
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List,
Laura Abbott, Rik van Riel
On Wed, Apr 10, 2019 at 4:12 PM Eric Biggers <ebiggers@kernel.org> wrote:
> You've explained *what* it does again, but not *why*. *Why* do you want
> hardened usercopy to detect copies across page boundaries, when there is no
> actual buffer overflow?
But that *is* how it determines it was a buffer overflow: "if you
cross page boundaries (of a non-compound allocation), it *is* a buffer
overflow". This assertion, however, is flawed because many contiguous
allocations are not marked as being grouped together when it reality
they were. It was an attempt to get allocation size information out of
the page allocator, similar to how slab can be queries about
allocation size. I'm open to improvements here, since it's obviously
broken in its current state. :)
--
Kees Cook
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-04-10 23:27 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-10 23:27 UTC (permalink / raw)
To: Eric Biggers
Cc: Herbert Xu, Rik van Riel, Linux Kernel Mailing List,
linux-security-module, Geert Uytterhoeven,
Linux Crypto Mailing List, Laura Abbott, Linux ARM
On Wed, Apr 10, 2019 at 4:12 PM Eric Biggers <ebiggers@kernel.org> wrote:
> You've explained *what* it does again, but not *why*. *Why* do you want
> hardened usercopy to detect copies across page boundaries, when there is no
> actual buffer overflow?
But that *is* how it determines it was a buffer overflow: "if you
cross page boundaries (of a non-compound allocation), it *is* a buffer
overflow". This assertion, however, is flawed because many contiguous
allocations are not marked as being grouped together when it reality
they were. It was an attempt to get allocation size information out of
the page allocator, similar to how slab can be queries about
allocation size. I'm open to improvements here, since it's obviously
broken in its current state. :)
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-04-10 23:11 ` Eric Biggers
@ 2019-04-11 1:37 ` Rik van Riel
-1 siblings, 0 replies; 55+ messages in thread
From: Rik van Riel @ 2019-04-11 1:37 UTC (permalink / raw)
To: Eric Biggers, Kees Cook
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List,
Laura Abbott
[-- Attachment #1: Type: text/plain, Size: 563 bytes --]
On Wed, 2019-04-10 at 16:11 -0700, Eric Biggers wrote:
> You've explained *what* it does again, but not *why*. *Why* do you
> want
> hardened usercopy to detect copies across page boundaries, when there
> is no
> actual buffer overflow?
When some subsystem in the kernel allocates multiple
pages without _GFP_COMP, there is no way afterwards
to detect exactly how many pages it allocated.
In other words, there is no way to see how large the
buffer is, nor whether the copy operation in question
would overflow it.
--
All Rights Reversed.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-04-11 1:37 ` Rik van Riel
0 siblings, 0 replies; 55+ messages in thread
From: Rik van Riel @ 2019-04-11 1:37 UTC (permalink / raw)
To: Eric Biggers, Kees Cook
Cc: Herbert Xu, Linux Kernel Mailing List, linux-security-module,
Geert Uytterhoeven, Linux Crypto Mailing List, Laura Abbott,
Linux ARM
[-- Attachment #1.1: Type: text/plain, Size: 563 bytes --]
On Wed, 2019-04-10 at 16:11 -0700, Eric Biggers wrote:
> You've explained *what* it does again, but not *why*. *Why* do you
> want
> hardened usercopy to detect copies across page boundaries, when there
> is no
> actual buffer overflow?
When some subsystem in the kernel allocates multiple
pages without _GFP_COMP, there is no way afterwards
to detect exactly how many pages it allocated.
In other words, there is no way to see how large the
buffer is, nor whether the copy operation in question
would overflow it.
--
All Rights Reversed.
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-04-10 23:27 ` Kees Cook
@ 2019-04-11 17:58 ` Eric Biggers
-1 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-11 17:58 UTC (permalink / raw)
To: Kees Cook
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List,
Laura Abbott, Rik van Riel
On Wed, Apr 10, 2019 at 04:27:28PM -0700, Kees Cook wrote:
> On Wed, Apr 10, 2019 at 4:12 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > You've explained *what* it does again, but not *why*. *Why* do you want
> > hardened usercopy to detect copies across page boundaries, when there is no
> > actual buffer overflow?
>
> But that *is* how it determines it was a buffer overflow: "if you
> cross page boundaries (of a non-compound allocation), it *is* a buffer
> overflow". This assertion, however, is flawed because many contiguous
> allocations are not marked as being grouped together when it reality
> they were. It was an attempt to get allocation size information out of
> the page allocator, similar to how slab can be queries about
> allocation size. I'm open to improvements here, since it's obviously
> broken in its current state. :)
>
> --
> Kees Cook
Well, I'm still at a loss as to whether I'm actually supposed to "fix" this by
adding __GFP_COMP, or whether you're saying the option is broken anyway so I
shouldn't bother doing anything. IIUC, even the kernel stack is still not
marked __GFP_COMP, so copies to/from the stack can trigger this too, despite
this being reported over 2 years ago
(http://lkml.iu.edu/hypermail/linux/kernel/1701.2/01450.html)?
CONFIG_HARDENED_USERCOPY_PAGESPAN is even disabled in syzbot because you already
said the option is broken and should not be used.
I worry that people will enable all the hardened usercopy options "because
security", then when the pagespan check breaks something they will disable all
hardened usercopy options, because they don't understand the individual options.
Providing broken options is actively harmful, IMO.
- Eric
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-04-11 17:58 ` Eric Biggers
0 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-11 17:58 UTC (permalink / raw)
To: Kees Cook
Cc: Herbert Xu, Rik van Riel, Linux Kernel Mailing List,
linux-security-module, Geert Uytterhoeven,
Linux Crypto Mailing List, Laura Abbott, Linux ARM
On Wed, Apr 10, 2019 at 04:27:28PM -0700, Kees Cook wrote:
> On Wed, Apr 10, 2019 at 4:12 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > You've explained *what* it does again, but not *why*. *Why* do you want
> > hardened usercopy to detect copies across page boundaries, when there is no
> > actual buffer overflow?
>
> But that *is* how it determines it was a buffer overflow: "if you
> cross page boundaries (of a non-compound allocation), it *is* a buffer
> overflow". This assertion, however, is flawed because many contiguous
> allocations are not marked as being grouped together when it reality
> they were. It was an attempt to get allocation size information out of
> the page allocator, similar to how slab can be queries about
> allocation size. I'm open to improvements here, since it's obviously
> broken in its current state. :)
>
> --
> Kees Cook
Well, I'm still at a loss as to whether I'm actually supposed to "fix" this by
adding __GFP_COMP, or whether you're saying the option is broken anyway so I
shouldn't bother doing anything. IIUC, even the kernel stack is still not
marked __GFP_COMP, so copies to/from the stack can trigger this too, despite
this being reported over 2 years ago
(http://lkml.iu.edu/hypermail/linux/kernel/1701.2/01450.html)?
CONFIG_HARDENED_USERCOPY_PAGESPAN is even disabled in syzbot because you already
said the option is broken and should not be used.
I worry that people will enable all the hardened usercopy options "because
security", then when the pagespan check breaks something they will disable all
hardened usercopy options, because they don't understand the individual options.
Providing broken options is actively harmful, IMO.
- Eric
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-04-11 17:58 ` Eric Biggers
@ 2019-04-11 18:33 ` Kees Cook
-1 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-11 18:33 UTC (permalink / raw)
To: Eric Biggers, Dmitry Vyukov
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List,
Laura Abbott, Rik van Riel
On Thu, Apr 11, 2019 at 10:58 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Wed, Apr 10, 2019 at 04:27:28PM -0700, Kees Cook wrote:
> > On Wed, Apr 10, 2019 at 4:12 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > > You've explained *what* it does again, but not *why*. *Why* do you want
> > > hardened usercopy to detect copies across page boundaries, when there is no
> > > actual buffer overflow?
> >
> > But that *is* how it determines it was a buffer overflow: "if you
> > cross page boundaries (of a non-compound allocation), it *is* a buffer
> > overflow". This assertion, however, is flawed because many contiguous
> > allocations are not marked as being grouped together when it reality
> > they were. It was an attempt to get allocation size information out of
> > the page allocator, similar to how slab can be queries about
> > allocation size. I'm open to improvements here, since it's obviously
> > broken in its current state. :)
> >
> > --
> > Kees Cook
>
> Well, I'm still at a loss as to whether I'm actually supposed to "fix" this by
> adding __GFP_COMP, or whether you're saying the option is broken anyway so I
I would love it if you could fix it, yes.
> shouldn't bother doing anything. IIUC, even the kernel stack is still not
> marked __GFP_COMP, so copies to/from the stack can trigger this too, despite
> this being reported over 2 years ago
> (http://lkml.iu.edu/hypermail/linux/kernel/1701.2/01450.html)?
> CONFIG_HARDENED_USERCOPY_PAGESPAN is even disabled in syzbot because you already
> said the option is broken and should not be used.
stacks are checked before PAGESPAN, so that particular problem should
no longer be present since commit 7bff3c069973 ("mm/usercopy.c: no
check page span for stack objects").
> I worry that people will enable all the hardened usercopy options "because
> security", then when the pagespan check breaks something they will disable all
> hardened usercopy options, because they don't understand the individual options.
> Providing broken options is actively harmful, IMO.
It's behind EXPERT, default-n, and says:
When a multi-page allocation is done without __GFP_COMP,
hardened usercopy will reject attempts to copy it. There are,
however, several cases of this in the kernel that have not all
been removed. This config is intended to be used only while
trying to find such users.
I'd rather leave it since it's still useful.
Perhaps it could be switched to WARN by default and we reenable it in
syzbot to improve its utility there?
diff --git a/mm/usercopy.c b/mm/usercopy.c
index 14faadcedd06..6e7e28fe062b 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c
@@ -208,8 +208,13 @@ static inline void check_page_span(const void
*ptr, unsigned long n,
*/
is_reserved = PageReserved(page);
is_cma = is_migrate_cma_page(page);
- if (!is_reserved && !is_cma)
- usercopy_abort("spans multiple pages", NULL, to_user, 0, n);
+ if (!is_reserved && !is_cma) {
+ usercopy_warn("spans multiple pages without __GFP_COMP",
+ NULL, to_user,
+ (unsigned long)ptr & (unsigned long)PAGE_MASK,
+ n);
+ return;
+ }
for (ptr += PAGE_SIZE; ptr <= end; ptr += PAGE_SIZE) {
page = virt_to_head_page(ptr);
--
Kees Cook
^ permalink raw reply related [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-04-11 18:33 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-11 18:33 UTC (permalink / raw)
To: Eric Biggers, Dmitry Vyukov
Cc: Herbert Xu, Rik van Riel, Linux Kernel Mailing List,
linux-security-module, Geert Uytterhoeven,
Linux Crypto Mailing List, Laura Abbott, Linux ARM
On Thu, Apr 11, 2019 at 10:58 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Wed, Apr 10, 2019 at 04:27:28PM -0700, Kees Cook wrote:
> > On Wed, Apr 10, 2019 at 4:12 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > > You've explained *what* it does again, but not *why*. *Why* do you want
> > > hardened usercopy to detect copies across page boundaries, when there is no
> > > actual buffer overflow?
> >
> > But that *is* how it determines it was a buffer overflow: "if you
> > cross page boundaries (of a non-compound allocation), it *is* a buffer
> > overflow". This assertion, however, is flawed because many contiguous
> > allocations are not marked as being grouped together when it reality
> > they were. It was an attempt to get allocation size information out of
> > the page allocator, similar to how slab can be queries about
> > allocation size. I'm open to improvements here, since it's obviously
> > broken in its current state. :)
> >
> > --
> > Kees Cook
>
> Well, I'm still at a loss as to whether I'm actually supposed to "fix" this by
> adding __GFP_COMP, or whether you're saying the option is broken anyway so I
I would love it if you could fix it, yes.
> shouldn't bother doing anything. IIUC, even the kernel stack is still not
> marked __GFP_COMP, so copies to/from the stack can trigger this too, despite
> this being reported over 2 years ago
> (http://lkml.iu.edu/hypermail/linux/kernel/1701.2/01450.html)?
> CONFIG_HARDENED_USERCOPY_PAGESPAN is even disabled in syzbot because you already
> said the option is broken and should not be used.
stacks are checked before PAGESPAN, so that particular problem should
no longer be present since commit 7bff3c069973 ("mm/usercopy.c: no
check page span for stack objects").
> I worry that people will enable all the hardened usercopy options "because
> security", then when the pagespan check breaks something they will disable all
> hardened usercopy options, because they don't understand the individual options.
> Providing broken options is actively harmful, IMO.
It's behind EXPERT, default-n, and says:
When a multi-page allocation is done without __GFP_COMP,
hardened usercopy will reject attempts to copy it. There are,
however, several cases of this in the kernel that have not all
been removed. This config is intended to be used only while
trying to find such users.
I'd rather leave it since it's still useful.
Perhaps it could be switched to WARN by default and we reenable it in
syzbot to improve its utility there?
diff --git a/mm/usercopy.c b/mm/usercopy.c
index 14faadcedd06..6e7e28fe062b 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c
@@ -208,8 +208,13 @@ static inline void check_page_span(const void
*ptr, unsigned long n,
*/
is_reserved = PageReserved(page);
is_cma = is_migrate_cma_page(page);
- if (!is_reserved && !is_cma)
- usercopy_abort("spans multiple pages", NULL, to_user, 0, n);
+ if (!is_reserved && !is_cma) {
+ usercopy_warn("spans multiple pages without __GFP_COMP",
+ NULL, to_user,
+ (unsigned long)ptr & (unsigned long)PAGE_MASK,
+ n);
+ return;
+ }
for (ptr += PAGE_SIZE; ptr <= end; ptr += PAGE_SIZE) {
page = virt_to_head_page(ptr);
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-04-11 18:33 ` Kees Cook
@ 2019-04-11 19:26 ` Eric Biggers
-1 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-11 19:26 UTC (permalink / raw)
To: Kees Cook
Cc: Dmitry Vyukov, Geert Uytterhoeven, Herbert Xu,
linux-security-module, Linux ARM, Linux Crypto Mailing List,
Linux Kernel Mailing List, Laura Abbott, Rik van Riel
On Thu, Apr 11, 2019 at 11:33:04AM -0700, Kees Cook wrote:
> On Thu, Apr 11, 2019 at 10:58 AM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Wed, Apr 10, 2019 at 04:27:28PM -0700, Kees Cook wrote:
> > > On Wed, Apr 10, 2019 at 4:12 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > > > You've explained *what* it does again, but not *why*. *Why* do you want
> > > > hardened usercopy to detect copies across page boundaries, when there is no
> > > > actual buffer overflow?
> > >
> > > But that *is* how it determines it was a buffer overflow: "if you
> > > cross page boundaries (of a non-compound allocation), it *is* a buffer
> > > overflow". This assertion, however, is flawed because many contiguous
> > > allocations are not marked as being grouped together when it reality
> > > they were. It was an attempt to get allocation size information out of
> > > the page allocator, similar to how slab can be queries about
> > > allocation size. I'm open to improvements here, since it's obviously
> > > broken in its current state. :)
> > >
> > > --
> > > Kees Cook
> >
> > Well, I'm still at a loss as to whether I'm actually supposed to "fix" this by
> > adding __GFP_COMP, or whether you're saying the option is broken anyway so I
>
> I would love it if you could fix it, yes.
>
> > shouldn't bother doing anything. IIUC, even the kernel stack is still not
> > marked __GFP_COMP, so copies to/from the stack can trigger this too, despite
> > this being reported over 2 years ago
> > (http://lkml.iu.edu/hypermail/linux/kernel/1701.2/01450.html)?
> > CONFIG_HARDENED_USERCOPY_PAGESPAN is even disabled in syzbot because you already
> > said the option is broken and should not be used.
>
> stacks are checked before PAGESPAN, so that particular problem should
> no longer be present since commit 7bff3c069973 ("mm/usercopy.c: no
> check page span for stack objects").
>
> > I worry that people will enable all the hardened usercopy options "because
> > security", then when the pagespan check breaks something they will disable all
> > hardened usercopy options, because they don't understand the individual options.
> > Providing broken options is actively harmful, IMO.
>
> It's behind EXPERT, default-n, and says:
>
> When a multi-page allocation is done without __GFP_COMP,
> hardened usercopy will reject attempts to copy it. There are,
> however, several cases of this in the kernel that have not all
> been removed. This config is intended to be used only while
> trying to find such users.
>
> I'd rather leave it since it's still useful.
>
> Perhaps it could be switched to WARN by default and we reenable it in
> syzbot to improve its utility there?
>
> diff --git a/mm/usercopy.c b/mm/usercopy.c
> index 14faadcedd06..6e7e28fe062b 100644
> --- a/mm/usercopy.c
> +++ b/mm/usercopy.c
> @@ -208,8 +208,13 @@ static inline void check_page_span(const void
> *ptr, unsigned long n,
> */
> is_reserved = PageReserved(page);
> is_cma = is_migrate_cma_page(page);
> - if (!is_reserved && !is_cma)
> - usercopy_abort("spans multiple pages", NULL, to_user, 0, n);
> + if (!is_reserved && !is_cma) {
> + usercopy_warn("spans multiple pages without __GFP_COMP",
> + NULL, to_user,
> + (unsigned long)ptr & (unsigned long)PAGE_MASK,
> + n);
> + return;
> + }
>
> for (ptr += PAGE_SIZE; ptr <= end; ptr += PAGE_SIZE) {
> page = virt_to_head_page(ptr);
>
>
> --
> Kees Cook
Well, I guess I'll just add __GFP_COMP so I at least don't get spammed with
useless bug reports.
But I don't think it's in any way acceptable to change the semantics of the
kernel's page allocator but only under some obscure config option, don't
document it anywhere, ignore the known problems for years, say that the option
is broken anyway so it shouldn't be used, and have to exchange 15 emails to get
anything resembling an explanation.
- Eric
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-04-11 19:26 ` Eric Biggers
0 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-11 19:26 UTC (permalink / raw)
To: Kees Cook
Cc: Herbert Xu, Rik van Riel, Linux Kernel Mailing List,
linux-security-module, Geert Uytterhoeven, Dmitry Vyukov,
Laura Abbott, Linux ARM, Linux Crypto Mailing List
On Thu, Apr 11, 2019 at 11:33:04AM -0700, Kees Cook wrote:
> On Thu, Apr 11, 2019 at 10:58 AM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Wed, Apr 10, 2019 at 04:27:28PM -0700, Kees Cook wrote:
> > > On Wed, Apr 10, 2019 at 4:12 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > > > You've explained *what* it does again, but not *why*. *Why* do you want
> > > > hardened usercopy to detect copies across page boundaries, when there is no
> > > > actual buffer overflow?
> > >
> > > But that *is* how it determines it was a buffer overflow: "if you
> > > cross page boundaries (of a non-compound allocation), it *is* a buffer
> > > overflow". This assertion, however, is flawed because many contiguous
> > > allocations are not marked as being grouped together when it reality
> > > they were. It was an attempt to get allocation size information out of
> > > the page allocator, similar to how slab can be queries about
> > > allocation size. I'm open to improvements here, since it's obviously
> > > broken in its current state. :)
> > >
> > > --
> > > Kees Cook
> >
> > Well, I'm still at a loss as to whether I'm actually supposed to "fix" this by
> > adding __GFP_COMP, or whether you're saying the option is broken anyway so I
>
> I would love it if you could fix it, yes.
>
> > shouldn't bother doing anything. IIUC, even the kernel stack is still not
> > marked __GFP_COMP, so copies to/from the stack can trigger this too, despite
> > this being reported over 2 years ago
> > (http://lkml.iu.edu/hypermail/linux/kernel/1701.2/01450.html)?
> > CONFIG_HARDENED_USERCOPY_PAGESPAN is even disabled in syzbot because you already
> > said the option is broken and should not be used.
>
> stacks are checked before PAGESPAN, so that particular problem should
> no longer be present since commit 7bff3c069973 ("mm/usercopy.c: no
> check page span for stack objects").
>
> > I worry that people will enable all the hardened usercopy options "because
> > security", then when the pagespan check breaks something they will disable all
> > hardened usercopy options, because they don't understand the individual options.
> > Providing broken options is actively harmful, IMO.
>
> It's behind EXPERT, default-n, and says:
>
> When a multi-page allocation is done without __GFP_COMP,
> hardened usercopy will reject attempts to copy it. There are,
> however, several cases of this in the kernel that have not all
> been removed. This config is intended to be used only while
> trying to find such users.
>
> I'd rather leave it since it's still useful.
>
> Perhaps it could be switched to WARN by default and we reenable it in
> syzbot to improve its utility there?
>
> diff --git a/mm/usercopy.c b/mm/usercopy.c
> index 14faadcedd06..6e7e28fe062b 100644
> --- a/mm/usercopy.c
> +++ b/mm/usercopy.c
> @@ -208,8 +208,13 @@ static inline void check_page_span(const void
> *ptr, unsigned long n,
> */
> is_reserved = PageReserved(page);
> is_cma = is_migrate_cma_page(page);
> - if (!is_reserved && !is_cma)
> - usercopy_abort("spans multiple pages", NULL, to_user, 0, n);
> + if (!is_reserved && !is_cma) {
> + usercopy_warn("spans multiple pages without __GFP_COMP",
> + NULL, to_user,
> + (unsigned long)ptr & (unsigned long)PAGE_MASK,
> + n);
> + return;
> + }
>
> for (ptr += PAGE_SIZE; ptr <= end; ptr += PAGE_SIZE) {
> page = virt_to_head_page(ptr);
>
>
> --
> Kees Cook
Well, I guess I'll just add __GFP_COMP so I at least don't get spammed with
useless bug reports.
But I don't think it's in any way acceptable to change the semantics of the
kernel's page allocator but only under some obscure config option, don't
document it anywhere, ignore the known problems for years, say that the option
is broken anyway so it shouldn't be used, and have to exchange 15 emails to get
anything resembling an explanation.
- Eric
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
2019-04-11 19:26 ` Eric Biggers
@ 2019-04-11 19:28 ` Eric Biggers
-1 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-11 19:28 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: Kees Cook, Dmitry Vyukov, Geert Uytterhoeven,
linux-security-module, Linux ARM, Linux Kernel Mailing List,
Laura Abbott, Rik van Riel
From: Eric Biggers <ebiggers@google.com>
This is needed so that CONFIG_HARDENED_USERCOPY_PAGESPAN=y doesn't
incorrectly report a buffer overflow when the destination of
copy_from_iter() spans the page boundary in the 2-page buffer.
Fixes: 3f47a03df6e8 ("crypto: testmgr - add testvec_config struct and helper functions")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/testmgr.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 0f6bfb6ce6a46..3522c0bed2492 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
int i;
for (i = 0; i < XBUFSIZE; i++) {
- buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
+ buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
+ order);
if (!buf[i])
goto err_free_buf;
}
--
2.21.0.392.gf8f6787159e-goog
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
@ 2019-04-11 19:28 ` Eric Biggers
0 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-11 19:28 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: Kees Cook, Rik van Riel, Linux Kernel Mailing List,
linux-security-module, Geert Uytterhoeven, Dmitry Vyukov,
Laura Abbott, Linux ARM
From: Eric Biggers <ebiggers@google.com>
This is needed so that CONFIG_HARDENED_USERCOPY_PAGESPAN=y doesn't
incorrectly report a buffer overflow when the destination of
copy_from_iter() spans the page boundary in the 2-page buffer.
Fixes: 3f47a03df6e8 ("crypto: testmgr - add testvec_config struct and helper functions")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/testmgr.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 0f6bfb6ce6a46..3522c0bed2492 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
int i;
for (i = 0; i < XBUFSIZE; i++) {
- buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
+ buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
+ order);
if (!buf[i])
goto err_free_buf;
}
--
2.21.0.392.gf8f6787159e-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
2019-04-11 19:28 ` Eric Biggers
@ 2019-04-11 20:32 ` Kees Cook
-1 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-11 20:32 UTC (permalink / raw)
To: Eric Biggers, Rik van Riel, Matthew Wilcox
Cc: linux-crypto, Herbert Xu, Kees Cook, Dmitry Vyukov,
Geert Uytterhoeven, linux-security-module, Linux ARM,
Linux Kernel Mailing List, Laura Abbott
On Thu, Apr 11, 2019 at 12:31 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> This is needed so that CONFIG_HARDENED_USERCOPY_PAGESPAN=y doesn't
> incorrectly report a buffer overflow when the destination of
> copy_from_iter() spans the page boundary in the 2-page buffer.
>
> Fixes: 3f47a03df6e8 ("crypto: testmgr - add testvec_config struct and helper functions")
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
> ---
> crypto/testmgr.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index 0f6bfb6ce6a46..3522c0bed2492 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
> int i;
>
> for (i = 0; i < XBUFSIZE; i++) {
> - buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
> + buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
> + order);
Is there a reason __GFP_COMP isn't automatically included in all page
allocations? (Or rather, it seems like the exception is when things
should NOT be considered part of the same allocation, so something
like __GFP_SINGLE should exist?.)
-Kees
> if (!buf[i])
> goto err_free_buf;
> }
> --
> 2.21.0.392.gf8f6787159e-goog
>
--
Kees Cook
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
@ 2019-04-11 20:32 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-11 20:32 UTC (permalink / raw)
To: Eric Biggers, Rik van Riel, Matthew Wilcox
Cc: Linux ARM, Kees Cook, Linux Kernel Mailing List,
linux-security-module, Geert Uytterhoeven, linux-crypto,
Laura Abbott, Dmitry Vyukov, Herbert Xu
On Thu, Apr 11, 2019 at 12:31 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> This is needed so that CONFIG_HARDENED_USERCOPY_PAGESPAN=y doesn't
> incorrectly report a buffer overflow when the destination of
> copy_from_iter() spans the page boundary in the 2-page buffer.
>
> Fixes: 3f47a03df6e8 ("crypto: testmgr - add testvec_config struct and helper functions")
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
> ---
> crypto/testmgr.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index 0f6bfb6ce6a46..3522c0bed2492 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
> int i;
>
> for (i = 0; i < XBUFSIZE; i++) {
> - buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
> + buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
> + order);
Is there a reason __GFP_COMP isn't automatically included in all page
allocations? (Or rather, it seems like the exception is when things
should NOT be considered part of the same allocation, so something
like __GFP_SINGLE should exist?.)
-Kees
> if (!buf[i])
> goto err_free_buf;
> }
> --
> 2.21.0.392.gf8f6787159e-goog
>
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-04-11 19:26 ` Eric Biggers
@ 2019-04-11 20:36 ` Kees Cook
-1 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-11 20:36 UTC (permalink / raw)
To: Eric Biggers
Cc: Dmitry Vyukov, Geert Uytterhoeven, Herbert Xu,
linux-security-module, Linux ARM, Linux Crypto Mailing List,
Linux Kernel Mailing List, Laura Abbott, Rik van Riel
On Thu, Apr 11, 2019 at 12:26 PM Eric Biggers <ebiggers@kernel.org> wrote:
> Well, I guess I'll just add __GFP_COMP so I at least don't get spammed with
> useless bug reports.
Thanks, I appreciate it.
> But I don't think it's in any way acceptable to change the semantics of the
> kernel's page allocator but only under some obscure config option, don't
> document it anywhere, ignore the known problems for years, say that the option
> is broken anyway so it shouldn't be used, and have to exchange 15 emails to get
> anything resembling an explanation.
I understand what you mean, yeah. I'm sorry I wasn't clear about it
earlier. What do you think might help the situation as far as
documentation?
--
Kees Cook
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-04-11 20:36 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-11 20:36 UTC (permalink / raw)
To: Eric Biggers
Cc: Herbert Xu, Rik van Riel, Linux Kernel Mailing List,
linux-security-module, Geert Uytterhoeven, Dmitry Vyukov,
Laura Abbott, Linux ARM, Linux Crypto Mailing List
On Thu, Apr 11, 2019 at 12:26 PM Eric Biggers <ebiggers@kernel.org> wrote:
> Well, I guess I'll just add __GFP_COMP so I at least don't get spammed with
> useless bug reports.
Thanks, I appreciate it.
> But I don't think it's in any way acceptable to change the semantics of the
> kernel's page allocator but only under some obscure config option, don't
> document it anywhere, ignore the known problems for years, say that the option
> is broken anyway so it shouldn't be used, and have to exchange 15 emails to get
> anything resembling an explanation.
I understand what you mean, yeah. I'm sorry I wasn't clear about it
earlier. What do you think might help the situation as far as
documentation?
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
2019-04-11 20:36 ` Kees Cook
@ 2019-04-11 20:56 ` Eric Biggers
-1 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-11 20:56 UTC (permalink / raw)
To: Kees Cook
Cc: Dmitry Vyukov, Geert Uytterhoeven, Herbert Xu,
linux-security-module, Linux ARM, Linux Crypto Mailing List,
Linux Kernel Mailing List, Laura Abbott, Rik van Riel
On Thu, Apr 11, 2019 at 01:36:37PM -0700, Kees Cook wrote:
> On Thu, Apr 11, 2019 at 12:26 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > Well, I guess I'll just add __GFP_COMP so I at least don't get spammed with
> > useless bug reports.
>
> Thanks, I appreciate it.
>
> > But I don't think it's in any way acceptable to change the semantics of the
> > kernel's page allocator but only under some obscure config option, don't
> > document it anywhere, ignore the known problems for years, say that the option
> > is broken anyway so it shouldn't be used, and have to exchange 15 emails to get
> > anything resembling an explanation.
>
> I understand what you mean, yeah. I'm sorry I wasn't clear about it
> earlier. What do you think might help the situation as far as
> documentation?
>
Explanation in Documentation/core-api/memory-allocation.rst of when to use
__GFP_COMP and why. Where "why" includes the real underlying reason why it's
designed the way it is, not just "you now need to provide this flag in order to
stop the hardened usercopy thing from crashing, even though previously it meant
something else, because that's the way it is."
Also a brief, improved explanation of __GFP_COMP in include/linux/gfp.h.
Also get Documentation/security/self-protection.rst up to date with what's
actually in the kernel. Currently it doesn't mention hardened usercopy at all,
and it's unclear about what's supported now vs. what is future work.
And actually fix the known problems with the pagespan check, not ignore them for
years. If not feasible, remove the option.
- Eric
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
@ 2019-04-11 20:56 ` Eric Biggers
0 siblings, 0 replies; 55+ messages in thread
From: Eric Biggers @ 2019-04-11 20:56 UTC (permalink / raw)
To: Kees Cook
Cc: Herbert Xu, Rik van Riel, Linux Kernel Mailing List,
linux-security-module, Geert Uytterhoeven, Dmitry Vyukov,
Laura Abbott, Linux ARM, Linux Crypto Mailing List
On Thu, Apr 11, 2019 at 01:36:37PM -0700, Kees Cook wrote:
> On Thu, Apr 11, 2019 at 12:26 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > Well, I guess I'll just add __GFP_COMP so I at least don't get spammed with
> > useless bug reports.
>
> Thanks, I appreciate it.
>
> > But I don't think it's in any way acceptable to change the semantics of the
> > kernel's page allocator but only under some obscure config option, don't
> > document it anywhere, ignore the known problems for years, say that the option
> > is broken anyway so it shouldn't be used, and have to exchange 15 emails to get
> > anything resembling an explanation.
>
> I understand what you mean, yeah. I'm sorry I wasn't clear about it
> earlier. What do you think might help the situation as far as
> documentation?
>
Explanation in Documentation/core-api/memory-allocation.rst of when to use
__GFP_COMP and why. Where "why" includes the real underlying reason why it's
designed the way it is, not just "you now need to provide this flag in order to
stop the hardened usercopy thing from crashing, even though previously it meant
something else, because that's the way it is."
Also a brief, improved explanation of __GFP_COMP in include/linux/gfp.h.
Also get Documentation/security/self-protection.rst up to date with what's
actually in the kernel. Currently it doesn't mention hardened usercopy at all,
and it's unclear about what's supported now vs. what is future work.
And actually fix the known problems with the pagespan check, not ignore them for
years. If not feasible, remove the option.
- Eric
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
2019-04-11 20:32 ` Kees Cook
@ 2019-04-12 5:38 ` Dmitry Vyukov
-1 siblings, 0 replies; 55+ messages in thread
From: Dmitry Vyukov @ 2019-04-12 5:38 UTC (permalink / raw)
To: Kees Cook
Cc: Eric Biggers, Rik van Riel, Matthew Wilcox, linux-crypto,
Herbert Xu, Geert Uytterhoeven, linux-security-module, Linux ARM,
Linux Kernel Mailing List, Laura Abbott
On Thu, Apr 11, 2019 at 10:32 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Apr 11, 2019 at 12:31 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > From: Eric Biggers <ebiggers@google.com>
> >
> > This is needed so that CONFIG_HARDENED_USERCOPY_PAGESPAN=y doesn't
> > incorrectly report a buffer overflow when the destination of
> > copy_from_iter() spans the page boundary in the 2-page buffer.
> >
> > Fixes: 3f47a03df6e8 ("crypto: testmgr - add testvec_config struct and helper functions")
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> > ---
> > crypto/testmgr.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> > index 0f6bfb6ce6a46..3522c0bed2492 100644
> > --- a/crypto/testmgr.c
> > +++ b/crypto/testmgr.c
> > @@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
> > int i;
> >
> > for (i = 0; i < XBUFSIZE; i++) {
> > - buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
> > + buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
> > + order);
>
> Is there a reason __GFP_COMP isn't automatically included in all page
> allocations? (Or rather, it seems like the exception is when things
> should NOT be considered part of the same allocation, so something
> like __GFP_SINGLE should exist?.)
It would be reasonable if __get_free_pages would automatically mark
consecutive pages as consecutive.
When these should not be considered part of the same allocation? Is it
possible to free them separately? Will that BUG with __GFP_COMP mark?
I understand that there can be a weak "these are actually the same
allocation, but I would like to think about them as separate". But
potentially we can ignore such cases.
> -Kees
>
> > if (!buf[i])
> > goto err_free_buf;
> > }
> > --
> > 2.21.0.392.gf8f6787159e-goog
> >
>
>
> --
> Kees Cook
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
@ 2019-04-12 5:38 ` Dmitry Vyukov
0 siblings, 0 replies; 55+ messages in thread
From: Dmitry Vyukov @ 2019-04-12 5:38 UTC (permalink / raw)
To: Kees Cook
Cc: Herbert Xu, Rik van Riel, Linux Kernel Mailing List,
Matthew Wilcox, Eric Biggers, linux-security-module,
Geert Uytterhoeven, linux-crypto, Laura Abbott, Linux ARM
On Thu, Apr 11, 2019 at 10:32 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Apr 11, 2019 at 12:31 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > From: Eric Biggers <ebiggers@google.com>
> >
> > This is needed so that CONFIG_HARDENED_USERCOPY_PAGESPAN=y doesn't
> > incorrectly report a buffer overflow when the destination of
> > copy_from_iter() spans the page boundary in the 2-page buffer.
> >
> > Fixes: 3f47a03df6e8 ("crypto: testmgr - add testvec_config struct and helper functions")
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> > ---
> > crypto/testmgr.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> > index 0f6bfb6ce6a46..3522c0bed2492 100644
> > --- a/crypto/testmgr.c
> > +++ b/crypto/testmgr.c
> > @@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
> > int i;
> >
> > for (i = 0; i < XBUFSIZE; i++) {
> > - buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
> > + buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
> > + order);
>
> Is there a reason __GFP_COMP isn't automatically included in all page
> allocations? (Or rather, it seems like the exception is when things
> should NOT be considered part of the same allocation, so something
> like __GFP_SINGLE should exist?.)
It would be reasonable if __get_free_pages would automatically mark
consecutive pages as consecutive.
When these should not be considered part of the same allocation? Is it
possible to free them separately? Will that BUG with __GFP_COMP mark?
I understand that there can be a weak "these are actually the same
allocation, but I would like to think about them as separate". But
potentially we can ignore such cases.
> -Kees
>
> > if (!buf[i])
> > goto err_free_buf;
> > }
> > --
> > 2.21.0.392.gf8f6787159e-goog
> >
>
>
> --
> Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
2019-04-11 20:32 ` Kees Cook
@ 2019-04-15 2:24 ` Matthew Wilcox
-1 siblings, 0 replies; 55+ messages in thread
From: Matthew Wilcox @ 2019-04-15 2:24 UTC (permalink / raw)
To: Kees Cook
Cc: Eric Biggers, Rik van Riel, linux-crypto, Herbert Xu,
Dmitry Vyukov, Geert Uytterhoeven, linux-security-module,
Linux ARM, Linux Kernel Mailing List, Laura Abbott, linux-mm
On Thu, Apr 11, 2019 at 01:32:32PM -0700, Kees Cook wrote:
> > @@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
> > int i;
> >
> > for (i = 0; i < XBUFSIZE; i++) {
> > - buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
> > + buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
> > + order);
>
> Is there a reason __GFP_COMP isn't automatically included in all page
> allocations? (Or rather, it seems like the exception is when things
> should NOT be considered part of the same allocation, so something
> like __GFP_SINGLE should exist?.)
The question is not whether or not things should be considered part of the
same allocation. The question is whether the allocation is of a compound
page or of N consecutive pages. Now you're asking what the difference is,
and it's whether you need to be able to be able to call compound_head(),
compound_order(), PageTail() or use a compound_dtor. If you don't, then
you can save some time at allocation & free by not specifying __GFP_COMP.
I'll agree this is not documented well, and maybe most multi-page
allocations do want __GFP_COMP and we should invert that bit, but
__GFP_SINGLE doesn't seem like the right antonym to __GFP_COMP to me.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
@ 2019-04-15 2:24 ` Matthew Wilcox
0 siblings, 0 replies; 55+ messages in thread
From: Matthew Wilcox @ 2019-04-15 2:24 UTC (permalink / raw)
To: Kees Cook
Cc: Linux ARM, Herbert Xu, Rik van Riel, Linux Kernel Mailing List,
Eric Biggers, linux-mm, linux-security-module,
Geert Uytterhoeven, linux-crypto, Laura Abbott, Dmitry Vyukov
On Thu, Apr 11, 2019 at 01:32:32PM -0700, Kees Cook wrote:
> > @@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
> > int i;
> >
> > for (i = 0; i < XBUFSIZE; i++) {
> > - buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
> > + buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
> > + order);
>
> Is there a reason __GFP_COMP isn't automatically included in all page
> allocations? (Or rather, it seems like the exception is when things
> should NOT be considered part of the same allocation, so something
> like __GFP_SINGLE should exist?.)
The question is not whether or not things should be considered part of the
same allocation. The question is whether the allocation is of a compound
page or of N consecutive pages. Now you're asking what the difference is,
and it's whether you need to be able to be able to call compound_head(),
compound_order(), PageTail() or use a compound_dtor. If you don't, then
you can save some time at allocation & free by not specifying __GFP_COMP.
I'll agree this is not documented well, and maybe most multi-page
allocations do want __GFP_COMP and we should invert that bit, but
__GFP_SINGLE doesn't seem like the right antonym to __GFP_COMP to me.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
2019-04-15 2:24 ` Matthew Wilcox
@ 2019-04-15 2:46 ` Herbert Xu
-1 siblings, 0 replies; 55+ messages in thread
From: Herbert Xu @ 2019-04-15 2:46 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Kees Cook, Eric Biggers, Rik van Riel, linux-crypto,
Dmitry Vyukov, Geert Uytterhoeven, linux-security-module,
Linux ARM, Linux Kernel Mailing List, Laura Abbott, linux-mm
On Sun, Apr 14, 2019 at 07:24:12PM -0700, Matthew Wilcox wrote:
> On Thu, Apr 11, 2019 at 01:32:32PM -0700, Kees Cook wrote:
> > > @@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
> > > int i;
> > >
> > > for (i = 0; i < XBUFSIZE; i++) {
> > > - buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
> > > + buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
> > > + order);
> >
> > Is there a reason __GFP_COMP isn't automatically included in all page
> > allocations? (Or rather, it seems like the exception is when things
> > should NOT be considered part of the same allocation, so something
> > like __GFP_SINGLE should exist?.)
>
> The question is not whether or not things should be considered part of the
> same allocation. The question is whether the allocation is of a compound
> page or of N consecutive pages. Now you're asking what the difference is,
> and it's whether you need to be able to be able to call compound_head(),
> compound_order(), PageTail() or use a compound_dtor. If you don't, then
> you can save some time at allocation & free by not specifying __GFP_COMP.
Thanks for clarifying Matthew.
Eric, this means that we should not use __GFP_COMP here just to
silent what is clearly a broken warning.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
@ 2019-04-15 2:46 ` Herbert Xu
0 siblings, 0 replies; 55+ messages in thread
From: Herbert Xu @ 2019-04-15 2:46 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Linux ARM, Kees Cook, Rik van Riel, Linux Kernel Mailing List,
Eric Biggers, linux-mm, linux-security-module,
Geert Uytterhoeven, linux-crypto, Laura Abbott, Dmitry Vyukov
On Sun, Apr 14, 2019 at 07:24:12PM -0700, Matthew Wilcox wrote:
> On Thu, Apr 11, 2019 at 01:32:32PM -0700, Kees Cook wrote:
> > > @@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
> > > int i;
> > >
> > > for (i = 0; i < XBUFSIZE; i++) {
> > > - buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
> > > + buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
> > > + order);
> >
> > Is there a reason __GFP_COMP isn't automatically included in all page
> > allocations? (Or rather, it seems like the exception is when things
> > should NOT be considered part of the same allocation, so something
> > like __GFP_SINGLE should exist?.)
>
> The question is not whether or not things should be considered part of the
> same allocation. The question is whether the allocation is of a compound
> page or of N consecutive pages. Now you're asking what the difference is,
> and it's whether you need to be able to be able to call compound_head(),
> compound_order(), PageTail() or use a compound_dtor. If you don't, then
> you can save some time at allocation & free by not specifying __GFP_COMP.
Thanks for clarifying Matthew.
Eric, this means that we should not use __GFP_COMP here just to
silent what is clearly a broken warning.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
2019-04-15 2:46 ` Herbert Xu
@ 2019-04-16 2:18 ` Matthew Wilcox
-1 siblings, 0 replies; 55+ messages in thread
From: Matthew Wilcox @ 2019-04-16 2:18 UTC (permalink / raw)
To: Herbert Xu
Cc: Kees Cook, Eric Biggers, Rik van Riel, linux-crypto,
Dmitry Vyukov, Geert Uytterhoeven, linux-security-module,
Linux ARM, Linux Kernel Mailing List, Laura Abbott, linux-mm
On Mon, Apr 15, 2019 at 10:46:15AM +0800, Herbert Xu wrote:
> On Sun, Apr 14, 2019 at 07:24:12PM -0700, Matthew Wilcox wrote:
> > On Thu, Apr 11, 2019 at 01:32:32PM -0700, Kees Cook wrote:
> > > > @@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
> > > > int i;
> > > >
> > > > for (i = 0; i < XBUFSIZE; i++) {
> > > > - buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
> > > > + buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
> > > > + order);
> > >
> > > Is there a reason __GFP_COMP isn't automatically included in all page
> > > allocations? (Or rather, it seems like the exception is when things
> > > should NOT be considered part of the same allocation, so something
> > > like __GFP_SINGLE should exist?.)
> >
> > The question is not whether or not things should be considered part of the
> > same allocation. The question is whether the allocation is of a compound
> > page or of N consecutive pages. Now you're asking what the difference is,
> > and it's whether you need to be able to be able to call compound_head(),
> > compound_order(), PageTail() or use a compound_dtor. If you don't, then
> > you can save some time at allocation & free by not specifying __GFP_COMP.
>
> Thanks for clarifying Matthew.
>
> Eric, this means that we should not use __GFP_COMP here just to
> silent what is clearly a broken warning.
I agree; if the crypto code is never going to try to go from the address of
a byte in the allocation back to the head page, then there's no need to
specify GFP_COMP.
But that leaves us in the awkward situation where
HARDENED_USERCOPY_PAGESPAN does need to be able to figure out whether
'ptr + n - 1' lies within the same allocation as ptr. Without using
a compound page, there's no indication in the VM structures that these
two pages were allocated as part of the same allocation.
We could force all multi-page allocations to be compound pages if
HARDENED_USERCOPY_PAGESPAN is enabled, but I worry that could break
something. We could make it catch fewer problems by succeeding if the
page is not compound. I don't know, these all seem like bad choices
to me.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
@ 2019-04-16 2:18 ` Matthew Wilcox
0 siblings, 0 replies; 55+ messages in thread
From: Matthew Wilcox @ 2019-04-16 2:18 UTC (permalink / raw)
To: Herbert Xu
Cc: Linux ARM, Kees Cook, Rik van Riel, Linux Kernel Mailing List,
Eric Biggers, linux-mm, linux-security-module,
Geert Uytterhoeven, linux-crypto, Laura Abbott, Dmitry Vyukov
On Mon, Apr 15, 2019 at 10:46:15AM +0800, Herbert Xu wrote:
> On Sun, Apr 14, 2019 at 07:24:12PM -0700, Matthew Wilcox wrote:
> > On Thu, Apr 11, 2019 at 01:32:32PM -0700, Kees Cook wrote:
> > > > @@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
> > > > int i;
> > > >
> > > > for (i = 0; i < XBUFSIZE; i++) {
> > > > - buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
> > > > + buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
> > > > + order);
> > >
> > > Is there a reason __GFP_COMP isn't automatically included in all page
> > > allocations? (Or rather, it seems like the exception is when things
> > > should NOT be considered part of the same allocation, so something
> > > like __GFP_SINGLE should exist?.)
> >
> > The question is not whether or not things should be considered part of the
> > same allocation. The question is whether the allocation is of a compound
> > page or of N consecutive pages. Now you're asking what the difference is,
> > and it's whether you need to be able to be able to call compound_head(),
> > compound_order(), PageTail() or use a compound_dtor. If you don't, then
> > you can save some time at allocation & free by not specifying __GFP_COMP.
>
> Thanks for clarifying Matthew.
>
> Eric, this means that we should not use __GFP_COMP here just to
> silent what is clearly a broken warning.
I agree; if the crypto code is never going to try to go from the address of
a byte in the allocation back to the head page, then there's no need to
specify GFP_COMP.
But that leaves us in the awkward situation where
HARDENED_USERCOPY_PAGESPAN does need to be able to figure out whether
'ptr + n - 1' lies within the same allocation as ptr. Without using
a compound page, there's no indication in the VM structures that these
two pages were allocated as part of the same allocation.
We could force all multi-page allocations to be compound pages if
HARDENED_USERCOPY_PAGESPAN is enabled, but I worry that could break
something. We could make it catch fewer problems by succeeding if the
page is not compound. I don't know, these all seem like bad choices
to me.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
2019-04-16 2:18 ` Matthew Wilcox
(?)
@ 2019-04-16 3:14 ` Kees Cook
-1 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-16 3:14 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Herbert Xu, Kees Cook, Eric Biggers, Rik van Riel, linux-crypto,
Dmitry Vyukov, Geert Uytterhoeven, linux-security-module,
Linux ARM, Linux Kernel Mailing List, Laura Abbott, Linux-MM
On Mon, Apr 15, 2019 at 9:18 PM Matthew Wilcox <willy@infradead.org> wrote:
> I agree; if the crypto code is never going to try to go from the address of
> a byte in the allocation back to the head page, then there's no need to
> specify GFP_COMP.
>
> But that leaves us in the awkward situation where
> HARDENED_USERCOPY_PAGESPAN does need to be able to figure out whether
> 'ptr + n - 1' lies within the same allocation as ptr. Without using
> a compound page, there's no indication in the VM structures that these
> two pages were allocated as part of the same allocation.
>
> We could force all multi-page allocations to be compound pages if
> HARDENED_USERCOPY_PAGESPAN is enabled, but I worry that could break
> something. We could make it catch fewer problems by succeeding if the
> page is not compound. I don't know, these all seem like bad choices
> to me.
If GFP_COMP is _not_ the correct signal about adjacent pages being
part of the same allocation, then I agree: we need to drop this check
entirely from PAGESPAN. Is there anything else that indicates this
property? (Or where might we be able to store that info?)
There are other pagespan checks, though, so those could stay. But I'd
really love to gain page allocator allocation size checking ...
--
Kees Cook
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
@ 2019-04-16 3:14 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-16 3:14 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Herbert Xu, Kees Cook, Eric Biggers, Rik van Riel, linux-crypto,
Dmitry Vyukov, Geert Uytterhoeven, linux-security-module,
Linux ARM, Linux Kernel Mailing List, Laura Abbott, Linux-MM
On Mon, Apr 15, 2019 at 9:18 PM Matthew Wilcox <willy@infradead.org> wrote:
> I agree; if the crypto code is never going to try to go from the address of
> a byte in the allocation back to the head page, then there's no need to
> specify GFP_COMP.
>
> But that leaves us in the awkward situation where
> HARDENED_USERCOPY_PAGESPAN does need to be able to figure out whether
> 'ptr + n - 1' lies within the same allocation as ptr. Without using
> a compound page, there's no indication in the VM structures that these
> two pages were allocated as part of the same allocation.
>
> We could force all multi-page allocations to be compound pages if
> HARDENED_USERCOPY_PAGESPAN is enabled, but I worry that could break
> something. We could make it catch fewer problems by succeeding if the
> page is not compound. I don't know, these all seem like bad choices
> to me.
If GFP_COMP is _not_ the correct signal about adjacent pages being
part of the same allocation, then I agree: we need to drop this check
entirely from PAGESPAN. Is there anything else that indicates this
property? (Or where might we be able to store that info?)
There are other pagespan checks, though, so those could stay. But I'd
really love to gain page allocator allocation size checking ...
--
Kees Cook
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
@ 2019-04-16 3:14 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2019-04-16 3:14 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Linux ARM, Herbert Xu, Rik van Riel, Linux Kernel Mailing List,
Eric Biggers, Linux-MM, linux-security-module,
Geert Uytterhoeven, linux-crypto, Laura Abbott, Dmitry Vyukov,
Kees Cook
On Mon, Apr 15, 2019 at 9:18 PM Matthew Wilcox <willy@infradead.org> wrote:
> I agree; if the crypto code is never going to try to go from the address of
> a byte in the allocation back to the head page, then there's no need to
> specify GFP_COMP.
>
> But that leaves us in the awkward situation where
> HARDENED_USERCOPY_PAGESPAN does need to be able to figure out whether
> 'ptr + n - 1' lies within the same allocation as ptr. Without using
> a compound page, there's no indication in the VM structures that these
> two pages were allocated as part of the same allocation.
>
> We could force all multi-page allocations to be compound pages if
> HARDENED_USERCOPY_PAGESPAN is enabled, but I worry that could break
> something. We could make it catch fewer problems by succeeding if the
> page is not compound. I don't know, these all seem like bad choices
> to me.
If GFP_COMP is _not_ the correct signal about adjacent pages being
part of the same allocation, then I agree: we need to drop this check
entirely from PAGESPAN. Is there anything else that indicates this
property? (Or where might we be able to store that info?)
There are other pagespan checks, though, so those could stay. But I'd
really love to gain page allocator allocation size checking ...
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
2019-04-16 3:14 ` Kees Cook
@ 2019-04-17 4:08 ` Matthew Wilcox
-1 siblings, 0 replies; 55+ messages in thread
From: Matthew Wilcox @ 2019-04-17 4:08 UTC (permalink / raw)
To: Kees Cook
Cc: Herbert Xu, Eric Biggers, Rik van Riel, linux-crypto,
Dmitry Vyukov, Geert Uytterhoeven, linux-security-module,
Linux ARM, Linux Kernel Mailing List, Laura Abbott, Linux-MM
On Mon, Apr 15, 2019 at 10:14:51PM -0500, Kees Cook wrote:
> On Mon, Apr 15, 2019 at 9:18 PM Matthew Wilcox <willy@infradead.org> wrote:
> > I agree; if the crypto code is never going to try to go from the address of
> > a byte in the allocation back to the head page, then there's no need to
> > specify GFP_COMP.
> >
> > But that leaves us in the awkward situation where
> > HARDENED_USERCOPY_PAGESPAN does need to be able to figure out whether
> > 'ptr + n - 1' lies within the same allocation as ptr. Without using
> > a compound page, there's no indication in the VM structures that these
> > two pages were allocated as part of the same allocation.
> >
> > We could force all multi-page allocations to be compound pages if
> > HARDENED_USERCOPY_PAGESPAN is enabled, but I worry that could break
> > something. We could make it catch fewer problems by succeeding if the
> > page is not compound. I don't know, these all seem like bad choices
> > to me.
>
> If GFP_COMP is _not_ the correct signal about adjacent pages being
> part of the same allocation, then I agree: we need to drop this check
> entirely from PAGESPAN. Is there anything else that indicates this
> property? (Or where might we be able to store that info?)
As far as I know, the page allocator does not store size information
anywhere, unless you use GFP_COMP. That's why you have to pass
the 'order' to free_pages() and __free_pages(). It's also why
alloc_pages_exact() works (follow all the way into split_page()).
> There are other pagespan checks, though, so those could stay. But I'd
> really love to gain page allocator allocation size checking ...
I think that's a great idea, but I'm not sure how you'll be able to
do that.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
@ 2019-04-17 4:08 ` Matthew Wilcox
0 siblings, 0 replies; 55+ messages in thread
From: Matthew Wilcox @ 2019-04-17 4:08 UTC (permalink / raw)
To: Kees Cook
Cc: Linux ARM, Herbert Xu, Rik van Riel, Linux Kernel Mailing List,
Eric Biggers, Linux-MM, linux-security-module,
Geert Uytterhoeven, linux-crypto, Laura Abbott, Dmitry Vyukov
On Mon, Apr 15, 2019 at 10:14:51PM -0500, Kees Cook wrote:
> On Mon, Apr 15, 2019 at 9:18 PM Matthew Wilcox <willy@infradead.org> wrote:
> > I agree; if the crypto code is never going to try to go from the address of
> > a byte in the allocation back to the head page, then there's no need to
> > specify GFP_COMP.
> >
> > But that leaves us in the awkward situation where
> > HARDENED_USERCOPY_PAGESPAN does need to be able to figure out whether
> > 'ptr + n - 1' lies within the same allocation as ptr. Without using
> > a compound page, there's no indication in the VM structures that these
> > two pages were allocated as part of the same allocation.
> >
> > We could force all multi-page allocations to be compound pages if
> > HARDENED_USERCOPY_PAGESPAN is enabled, but I worry that could break
> > something. We could make it catch fewer problems by succeeding if the
> > page is not compound. I don't know, these all seem like bad choices
> > to me.
>
> If GFP_COMP is _not_ the correct signal about adjacent pages being
> part of the same allocation, then I agree: we need to drop this check
> entirely from PAGESPAN. Is there anything else that indicates this
> property? (Or where might we be able to store that info?)
As far as I know, the page allocator does not store size information
anywhere, unless you use GFP_COMP. That's why you have to pass
the 'order' to free_pages() and __free_pages(). It's also why
alloc_pages_exact() works (follow all the way into split_page()).
> There are other pagespan checks, though, so those could stay. But I'd
> really love to gain page allocator allocation size checking ...
I think that's a great idea, but I'm not sure how you'll be able to
do that.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
2019-04-17 4:08 ` Matthew Wilcox
@ 2019-04-17 8:09 ` Russell King - ARM Linux admin
-1 siblings, 0 replies; 55+ messages in thread
From: Russell King - ARM Linux admin @ 2019-04-17 8:09 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Kees Cook, Linux ARM, Herbert Xu, Rik van Riel,
Linux Kernel Mailing List, Eric Biggers, Linux-MM,
linux-security-module, Geert Uytterhoeven, linux-crypto,
Laura Abbott, Dmitry Vyukov
On Tue, Apr 16, 2019 at 09:08:22PM -0700, Matthew Wilcox wrote:
> On Mon, Apr 15, 2019 at 10:14:51PM -0500, Kees Cook wrote:
> > On Mon, Apr 15, 2019 at 9:18 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > I agree; if the crypto code is never going to try to go from the address of
> > > a byte in the allocation back to the head page, then there's no need to
> > > specify GFP_COMP.
> > >
> > > But that leaves us in the awkward situation where
> > > HARDENED_USERCOPY_PAGESPAN does need to be able to figure out whether
> > > 'ptr + n - 1' lies within the same allocation as ptr. Without using
> > > a compound page, there's no indication in the VM structures that these
> > > two pages were allocated as part of the same allocation.
> > >
> > > We could force all multi-page allocations to be compound pages if
> > > HARDENED_USERCOPY_PAGESPAN is enabled, but I worry that could break
> > > something. We could make it catch fewer problems by succeeding if the
> > > page is not compound. I don't know, these all seem like bad choices
> > > to me.
> >
> > If GFP_COMP is _not_ the correct signal about adjacent pages being
> > part of the same allocation, then I agree: we need to drop this check
> > entirely from PAGESPAN. Is there anything else that indicates this
> > property? (Or where might we be able to store that info?)
>
> As far as I know, the page allocator does not store size information
> anywhere, unless you use GFP_COMP. That's why you have to pass
> the 'order' to free_pages() and __free_pages(). It's also why
> alloc_pages_exact() works (follow all the way into split_page()).
>
> > There are other pagespan checks, though, so those could stay. But I'd
> > really love to gain page allocator allocation size checking ...
>
> I think that's a great idea, but I'm not sure how you'll be able to
> do that.
However, we have had code (maybe historically now) that has allocated
a higher order page and then handed back pages that it doesn't need -
for example, when the code requires multiple contiguous pages but does
not require a power-of-2 size of contiguous pages.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
@ 2019-04-17 8:09 ` Russell King - ARM Linux admin
0 siblings, 0 replies; 55+ messages in thread
From: Russell King - ARM Linux admin @ 2019-04-17 8:09 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Kees Cook, Rik van Riel, Linux Kernel Mailing List, Eric Biggers,
Linux-MM, linux-security-module, Geert Uytterhoeven,
linux-crypto, Dmitry Vyukov, Laura Abbott, Linux ARM, Herbert Xu
On Tue, Apr 16, 2019 at 09:08:22PM -0700, Matthew Wilcox wrote:
> On Mon, Apr 15, 2019 at 10:14:51PM -0500, Kees Cook wrote:
> > On Mon, Apr 15, 2019 at 9:18 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > I agree; if the crypto code is never going to try to go from the address of
> > > a byte in the allocation back to the head page, then there's no need to
> > > specify GFP_COMP.
> > >
> > > But that leaves us in the awkward situation where
> > > HARDENED_USERCOPY_PAGESPAN does need to be able to figure out whether
> > > 'ptr + n - 1' lies within the same allocation as ptr. Without using
> > > a compound page, there's no indication in the VM structures that these
> > > two pages were allocated as part of the same allocation.
> > >
> > > We could force all multi-page allocations to be compound pages if
> > > HARDENED_USERCOPY_PAGESPAN is enabled, but I worry that could break
> > > something. We could make it catch fewer problems by succeeding if the
> > > page is not compound. I don't know, these all seem like bad choices
> > > to me.
> >
> > If GFP_COMP is _not_ the correct signal about adjacent pages being
> > part of the same allocation, then I agree: we need to drop this check
> > entirely from PAGESPAN. Is there anything else that indicates this
> > property? (Or where might we be able to store that info?)
>
> As far as I know, the page allocator does not store size information
> anywhere, unless you use GFP_COMP. That's why you have to pass
> the 'order' to free_pages() and __free_pages(). It's also why
> alloc_pages_exact() works (follow all the way into split_page()).
>
> > There are other pagespan checks, though, so those could stay. But I'd
> > really love to gain page allocator allocation size checking ...
>
> I think that's a great idea, but I'm not sure how you'll be able to
> do that.
However, we have had code (maybe historically now) that has allocated
a higher order page and then handed back pages that it doesn't need -
for example, when the code requires multiple contiguous pages but does
not require a power-of-2 size of contiguous pages.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
2019-04-17 8:09 ` Russell King - ARM Linux admin
@ 2019-04-17 9:54 ` Robin Murphy
-1 siblings, 0 replies; 55+ messages in thread
From: Robin Murphy @ 2019-04-17 9:54 UTC (permalink / raw)
To: Russell King - ARM Linux admin, Matthew Wilcox
Cc: Kees Cook, Rik van Riel, Linux Kernel Mailing List, Eric Biggers,
Linux-MM, linux-security-module, Geert Uytterhoeven,
linux-crypto, Dmitry Vyukov, Laura Abbott, Linux ARM, Herbert Xu
On 17/04/2019 09:09, Russell King - ARM Linux admin wrote:
> On Tue, Apr 16, 2019 at 09:08:22PM -0700, Matthew Wilcox wrote:
>> On Mon, Apr 15, 2019 at 10:14:51PM -0500, Kees Cook wrote:
>>> On Mon, Apr 15, 2019 at 9:18 PM Matthew Wilcox <willy@infradead.org> wrote:
>>>> I agree; if the crypto code is never going to try to go from the address of
>>>> a byte in the allocation back to the head page, then there's no need to
>>>> specify GFP_COMP.
>>>>
>>>> But that leaves us in the awkward situation where
>>>> HARDENED_USERCOPY_PAGESPAN does need to be able to figure out whether
>>>> 'ptr + n - 1' lies within the same allocation as ptr. Without using
>>>> a compound page, there's no indication in the VM structures that these
>>>> two pages were allocated as part of the same allocation.
>>>>
>>>> We could force all multi-page allocations to be compound pages if
>>>> HARDENED_USERCOPY_PAGESPAN is enabled, but I worry that could break
>>>> something. We could make it catch fewer problems by succeeding if the
>>>> page is not compound. I don't know, these all seem like bad choices
>>>> to me.
>>>
>>> If GFP_COMP is _not_ the correct signal about adjacent pages being
>>> part of the same allocation, then I agree: we need to drop this check
>>> entirely from PAGESPAN. Is there anything else that indicates this
>>> property? (Or where might we be able to store that info?)
>>
>> As far as I know, the page allocator does not store size information
>> anywhere, unless you use GFP_COMP. That's why you have to pass
>> the 'order' to free_pages() and __free_pages(). It's also why
>> alloc_pages_exact() works (follow all the way into split_page()).
>>
>>> There are other pagespan checks, though, so those could stay. But I'd
>>> really love to gain page allocator allocation size checking ...
>>
>> I think that's a great idea, but I'm not sure how you'll be able to
>> do that.
>
> However, we have had code (maybe historically now) that has allocated
> a higher order page and then handed back pages that it doesn't need -
> for example, when the code requires multiple contiguous pages but does
> not require a power-of-2 size of contiguous pages.
'git grep alloc_pages_exact' suggests it's not historical yet...
Robin.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
@ 2019-04-17 9:54 ` Robin Murphy
0 siblings, 0 replies; 55+ messages in thread
From: Robin Murphy @ 2019-04-17 9:54 UTC (permalink / raw)
To: Russell King - ARM Linux admin, Matthew Wilcox
Cc: Linux ARM, Kees Cook, Rik van Riel, Linux Kernel Mailing List,
Eric Biggers, Linux-MM, linux-security-module,
Geert Uytterhoeven, linux-crypto, Laura Abbott, Dmitry Vyukov,
Herbert Xu
On 17/04/2019 09:09, Russell King - ARM Linux admin wrote:
> On Tue, Apr 16, 2019 at 09:08:22PM -0700, Matthew Wilcox wrote:
>> On Mon, Apr 15, 2019 at 10:14:51PM -0500, Kees Cook wrote:
>>> On Mon, Apr 15, 2019 at 9:18 PM Matthew Wilcox <willy@infradead.org> wrote:
>>>> I agree; if the crypto code is never going to try to go from the address of
>>>> a byte in the allocation back to the head page, then there's no need to
>>>> specify GFP_COMP.
>>>>
>>>> But that leaves us in the awkward situation where
>>>> HARDENED_USERCOPY_PAGESPAN does need to be able to figure out whether
>>>> 'ptr + n - 1' lies within the same allocation as ptr. Without using
>>>> a compound page, there's no indication in the VM structures that these
>>>> two pages were allocated as part of the same allocation.
>>>>
>>>> We could force all multi-page allocations to be compound pages if
>>>> HARDENED_USERCOPY_PAGESPAN is enabled, but I worry that could break
>>>> something. We could make it catch fewer problems by succeeding if the
>>>> page is not compound. I don't know, these all seem like bad choices
>>>> to me.
>>>
>>> If GFP_COMP is _not_ the correct signal about adjacent pages being
>>> part of the same allocation, then I agree: we need to drop this check
>>> entirely from PAGESPAN. Is there anything else that indicates this
>>> property? (Or where might we be able to store that info?)
>>
>> As far as I know, the page allocator does not store size information
>> anywhere, unless you use GFP_COMP. That's why you have to pass
>> the 'order' to free_pages() and __free_pages(). It's also why
>> alloc_pages_exact() works (follow all the way into split_page()).
>>
>>> There are other pagespan checks, though, so those could stay. But I'd
>>> really love to gain page allocator allocation size checking ...
>>
>> I think that's a great idea, but I'm not sure how you'll be able to
>> do that.
>
> However, we have had code (maybe historically now) that has allocated
> a higher order page and then handed back pages that it doesn't need -
> for example, when the code requires multiple contiguous pages but does
> not require a power-of-2 size of contiguous pages.
'git grep alloc_pages_exact' suggests it's not historical yet...
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread
end of thread, other threads:[~2019-04-17 9:54 UTC | newest]
Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-19 11:54 crypto: Kernel memory overwrite attempt detected to spans multiple pages Geert Uytterhoeven
2019-03-19 11:54 ` Geert Uytterhoeven
2019-03-19 17:09 ` Eric Biggers
2019-03-19 17:09 ` Eric Biggers
2019-03-20 18:57 ` Eric Biggers
2019-03-20 18:57 ` Eric Biggers
2019-03-21 17:45 ` Kees Cook
2019-03-21 17:45 ` Kees Cook
2019-03-21 17:51 ` Eric Biggers
2019-03-21 17:51 ` Eric Biggers
2019-04-10 3:17 ` Eric Biggers
2019-04-10 3:17 ` Eric Biggers
2019-04-10 18:30 ` Kees Cook
2019-04-10 18:30 ` Kees Cook
2019-04-10 19:07 ` Eric Biggers
2019-04-10 19:07 ` Eric Biggers
2019-04-10 21:57 ` Kees Cook
2019-04-10 21:57 ` Kees Cook
2019-04-10 23:11 ` Eric Biggers
2019-04-10 23:11 ` Eric Biggers
2019-04-10 23:27 ` Kees Cook
2019-04-10 23:27 ` Kees Cook
2019-04-11 17:58 ` Eric Biggers
2019-04-11 17:58 ` Eric Biggers
2019-04-11 18:33 ` Kees Cook
2019-04-11 18:33 ` Kees Cook
2019-04-11 19:26 ` Eric Biggers
2019-04-11 19:26 ` Eric Biggers
2019-04-11 19:28 ` [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP Eric Biggers
2019-04-11 19:28 ` Eric Biggers
2019-04-11 20:32 ` Kees Cook
2019-04-11 20:32 ` Kees Cook
2019-04-12 5:38 ` Dmitry Vyukov
2019-04-12 5:38 ` Dmitry Vyukov
2019-04-15 2:24 ` Matthew Wilcox
2019-04-15 2:24 ` Matthew Wilcox
2019-04-15 2:46 ` Herbert Xu
2019-04-15 2:46 ` Herbert Xu
2019-04-16 2:18 ` Matthew Wilcox
2019-04-16 2:18 ` Matthew Wilcox
2019-04-16 3:14 ` Kees Cook
2019-04-16 3:14 ` Kees Cook
2019-04-16 3:14 ` Kees Cook
2019-04-17 4:08 ` Matthew Wilcox
2019-04-17 4:08 ` Matthew Wilcox
2019-04-17 8:09 ` Russell King - ARM Linux admin
2019-04-17 8:09 ` Russell King - ARM Linux admin
2019-04-17 9:54 ` Robin Murphy
2019-04-17 9:54 ` Robin Murphy
2019-04-11 20:36 ` crypto: Kernel memory overwrite attempt detected to spans multiple pages Kees Cook
2019-04-11 20:36 ` Kees Cook
2019-04-11 20:56 ` Eric Biggers
2019-04-11 20:56 ` Eric Biggers
2019-04-11 1:37 ` Rik van Riel
2019-04-11 1:37 ` Rik van Riel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.