| Message ID | 1574166746-27197-15-git-send-email-amit.kachhap@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: return address signing | expand |
On Tue, 19 Nov 2019 at 13:33, Amit Daniel Kachhap <amit.kachhap@arm.com> wrote: > > This test is specific for arm64. When in-kernel Pointer Authentication > config is enabled, the return address stored in the stack is signed. This > feature helps in ROP kind of attack. If the matching signature is corrupted > then this will fail in authentication and lead to abort. > > e.g. > echo CORRUPT_PAC > /sys/kernel/debug/provoke-crash/DIRECT > > [ 13.118166] lkdtm: Performing direct entry CORRUPT_PAC > [ 13.118298] lkdtm: Clearing PAC from the return address > [ 13.118466] Unable to handle kernel paging request at virtual address bfff8000108648ec > [ 13.118626] Mem abort info: > [ 13.118666] ESR = 0x86000004 > [ 13.118866] EC = 0x21: IABT (current EL), IL = 32 bits > [ 13.118966] SET = 0, FnV = 0 > [ 13.119117] EA = 0, S1PTW = 0 > > Cc: Kees Cook <keescook@chromium.org> > Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com> > --- > Change since last version: > * New patch > > drivers/misc/lkdtm/bugs.c | 17 +++++++++++++++++ > drivers/misc/lkdtm/core.c | 1 + > drivers/misc/lkdtm/lkdtm.h | 1 + > 3 files changed, 19 insertions(+) > > diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c > index 7284a22..c9bb493 100644 > --- a/drivers/misc/lkdtm/bugs.c > +++ b/drivers/misc/lkdtm/bugs.c > @@ -337,3 +337,20 @@ void lkdtm_UNSET_SMEP(void) > pr_err("FAIL: this test is x86_64-only\n"); > #endif > } > + > +void lkdtm_CORRUPT_PAC(void) > +{ > +#if IS_ENABLED(CONFIG_ARM64_PTR_AUTH) > + u64 ret; > + > + pr_info("Clearing PAC from the return address\n"); > + /* > + * __builtin_return_address masks the PAC bits of return > + * address, so set the same again. > + */ > + ret = (u64)__builtin_return_address(0); > + asm volatile("str %0, [sp, 8]" : : "r" (ret) : "memory"); This looks a bit fragile to me. You are making assumptions about the location of the return address in the stack frame which are not guaranteed to hold. Couldn't you do this test simply by changing the key? > +#else > + pr_err("FAIL: For arm64 pointer authentication capable systems only\n"); > +#endif > +} > diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c > index cbc4c90..b9c9927 100644 > --- a/drivers/misc/lkdtm/core.c > +++ b/drivers/misc/lkdtm/core.c > @@ -116,6 +116,7 @@ static const struct crashtype crashtypes[] = { > CRASHTYPE(STACK_GUARD_PAGE_LEADING), > CRASHTYPE(STACK_GUARD_PAGE_TRAILING), > CRASHTYPE(UNSET_SMEP), > + CRASHTYPE(CORRUPT_PAC), > CRASHTYPE(UNALIGNED_LOAD_STORE_WRITE), > CRASHTYPE(OVERWRITE_ALLOCATION), > CRASHTYPE(WRITE_AFTER_FREE), > diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h > index ab446e0..bf12b68 100644 > --- a/drivers/misc/lkdtm/lkdtm.h > +++ b/drivers/misc/lkdtm/lkdtm.h > @@ -28,6 +28,7 @@ void lkdtm_CORRUPT_USER_DS(void); > void lkdtm_STACK_GUARD_PAGE_LEADING(void); > void lkdtm_STACK_GUARD_PAGE_TRAILING(void); > void lkdtm_UNSET_SMEP(void); > +void lkdtm_CORRUPT_PAC(void); > > /* lkdtm_heap.c */ > void __init lkdtm_heap_init(void); > -- > 2.7.4 >
On 11/21/19 5:39 PM, Ard Biesheuvel wrote: > On Tue, 19 Nov 2019 at 13:33, Amit Daniel Kachhap <amit.kachhap@arm.com> wrote: >> >> This test is specific for arm64. When in-kernel Pointer Authentication >> config is enabled, the return address stored in the stack is signed. This >> feature helps in ROP kind of attack. If the matching signature is corrupted >> then this will fail in authentication and lead to abort. >> >> e.g. >> echo CORRUPT_PAC > /sys/kernel/debug/provoke-crash/DIRECT >> >> [ 13.118166] lkdtm: Performing direct entry CORRUPT_PAC >> [ 13.118298] lkdtm: Clearing PAC from the return address >> [ 13.118466] Unable to handle kernel paging request at virtual address bfff8000108648ec >> [ 13.118626] Mem abort info: >> [ 13.118666] ESR = 0x86000004 >> [ 13.118866] EC = 0x21: IABT (current EL), IL = 32 bits >> [ 13.118966] SET = 0, FnV = 0 >> [ 13.119117] EA = 0, S1PTW = 0 >> >> Cc: Kees Cook <keescook@chromium.org> >> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com> >> --- >> Change since last version: >> * New patch >> >> drivers/misc/lkdtm/bugs.c | 17 +++++++++++++++++ >> drivers/misc/lkdtm/core.c | 1 + >> drivers/misc/lkdtm/lkdtm.h | 1 + >> 3 files changed, 19 insertions(+) >> >> diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c >> index 7284a22..c9bb493 100644 >> --- a/drivers/misc/lkdtm/bugs.c >> +++ b/drivers/misc/lkdtm/bugs.c >> @@ -337,3 +337,20 @@ void lkdtm_UNSET_SMEP(void) >> pr_err("FAIL: this test is x86_64-only\n"); >> #endif >> } >> + >> +void lkdtm_CORRUPT_PAC(void) >> +{ >> +#if IS_ENABLED(CONFIG_ARM64_PTR_AUTH) >> + u64 ret; >> + >> + pr_info("Clearing PAC from the return address\n"); >> + /* >> + * __builtin_return_address masks the PAC bits of return >> + * address, so set the same again. >> + */ >> + ret = (u64)__builtin_return_address(0); >> + asm volatile("str %0, [sp, 8]" : : "r" (ret) : "memory"); > > This looks a bit fragile to me. You are making assumptions about the > location of the return address in the stack frame which are not > guaranteed to hold. Indeed. > Couldn't you do this test simply by changing the key? That, at least, means you don't have to know the stack frame layout. However, there's a chance (1/32767, I think, for the 48-bit vma case w/o TBI) that changing the key will result in the same hash. Even when the stack frame happens to be layed out as Amit guesses, the result is akin to changing the key, such that hash(key, salt, ptr) == 0. While testing this in qemu, I iterate until I find a <key, salt, ptr> tuple that definitely produces a different hash. Usually this loop iterates just once, but the occasional failures that I got without iterating were annoying (with TBI enabled in userspace, the chance drops to 1/127, so much more frequent). r~
Hi, On 11/21/19 11:09 PM, Ard Biesheuvel wrote: > On Tue, 19 Nov 2019 at 13:33, Amit Daniel Kachhap <amit.kachhap@arm.com> wrote: >> >> This test is specific for arm64. When in-kernel Pointer Authentication >> config is enabled, the return address stored in the stack is signed. This >> feature helps in ROP kind of attack. If the matching signature is corrupted >> then this will fail in authentication and lead to abort. >> >> e.g. >> echo CORRUPT_PAC > /sys/kernel/debug/provoke-crash/DIRECT >> >> [ 13.118166] lkdtm: Performing direct entry CORRUPT_PAC >> [ 13.118298] lkdtm: Clearing PAC from the return address >> [ 13.118466] Unable to handle kernel paging request at virtual address bfff8000108648ec >> [ 13.118626] Mem abort info: >> [ 13.118666] ESR = 0x86000004 >> [ 13.118866] EC = 0x21: IABT (current EL), IL = 32 bits >> [ 13.118966] SET = 0, FnV = 0 >> [ 13.119117] EA = 0, S1PTW = 0 >> >> Cc: Kees Cook <keescook@chromium.org> >> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com> >> --- >> Change since last version: >> * New patch >> >> drivers/misc/lkdtm/bugs.c | 17 +++++++++++++++++ >> drivers/misc/lkdtm/core.c | 1 + >> drivers/misc/lkdtm/lkdtm.h | 1 + >> 3 files changed, 19 insertions(+) >> >> diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c >> index 7284a22..c9bb493 100644 >> --- a/drivers/misc/lkdtm/bugs.c >> +++ b/drivers/misc/lkdtm/bugs.c >> @@ -337,3 +337,20 @@ void lkdtm_UNSET_SMEP(void) >> pr_err("FAIL: this test is x86_64-only\n"); >> #endif >> } >> + >> +void lkdtm_CORRUPT_PAC(void) >> +{ >> +#if IS_ENABLED(CONFIG_ARM64_PTR_AUTH) >> + u64 ret; >> + >> + pr_info("Clearing PAC from the return address\n"); >> + /* >> + * __builtin_return_address masks the PAC bits of return >> + * address, so set the same again. >> + */ >> + ret = (u64)__builtin_return_address(0); >> + asm volatile("str %0, [sp, 8]" : : "r" (ret) : "memory"); > > This looks a bit fragile to me. You are making assumptions about the > location of the return address in the stack frame which are not > guaranteed to hold. Yes agreed. > > Couldn't you do this test simply by changing the key? Yes it should be possible. I will update it in my next iteration. Regards, Amit D > >> +#else >> + pr_err("FAIL: For arm64 pointer authentication capable systems only\n"); >> +#endif >> +} >> diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c >> index cbc4c90..b9c9927 100644 >> --- a/drivers/misc/lkdtm/core.c >> +++ b/drivers/misc/lkdtm/core.c >> @@ -116,6 +116,7 @@ static const struct crashtype crashtypes[] = { >> CRASHTYPE(STACK_GUARD_PAGE_LEADING), >> CRASHTYPE(STACK_GUARD_PAGE_TRAILING), >> CRASHTYPE(UNSET_SMEP), >> + CRASHTYPE(CORRUPT_PAC), >> CRASHTYPE(UNALIGNED_LOAD_STORE_WRITE), >> CRASHTYPE(OVERWRITE_ALLOCATION), >> CRASHTYPE(WRITE_AFTER_FREE), >> diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h >> index ab446e0..bf12b68 100644 >> --- a/drivers/misc/lkdtm/lkdtm.h >> +++ b/drivers/misc/lkdtm/lkdtm.h >> @@ -28,6 +28,7 @@ void lkdtm_CORRUPT_USER_DS(void); >> void lkdtm_STACK_GUARD_PAGE_LEADING(void); >> void lkdtm_STACK_GUARD_PAGE_TRAILING(void); >> void lkdtm_UNSET_SMEP(void); >> +void lkdtm_CORRUPT_PAC(void); >> >> /* lkdtm_heap.c */ >> void __init lkdtm_heap_init(void); >> -- >> 2.7.4 >>
On 11/23/19 12:21 AM, Richard Henderson wrote: > On 11/21/19 5:39 PM, Ard Biesheuvel wrote: >> On Tue, 19 Nov 2019 at 13:33, Amit Daniel Kachhap <amit.kachhap@arm.com> wrote: >>> >>> This test is specific for arm64. When in-kernel Pointer Authentication >>> config is enabled, the return address stored in the stack is signed. This >>> feature helps in ROP kind of attack. If the matching signature is corrupted >>> then this will fail in authentication and lead to abort. >>> >>> e.g. >>> echo CORRUPT_PAC > /sys/kernel/debug/provoke-crash/DIRECT >>> >>> [ 13.118166] lkdtm: Performing direct entry CORRUPT_PAC >>> [ 13.118298] lkdtm: Clearing PAC from the return address >>> [ 13.118466] Unable to handle kernel paging request at virtual address bfff8000108648ec >>> [ 13.118626] Mem abort info: >>> [ 13.118666] ESR = 0x86000004 >>> [ 13.118866] EC = 0x21: IABT (current EL), IL = 32 bits >>> [ 13.118966] SET = 0, FnV = 0 >>> [ 13.119117] EA = 0, S1PTW = 0 >>> >>> Cc: Kees Cook <keescook@chromium.org> >>> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com> >>> --- >>> Change since last version: >>> * New patch >>> >>> drivers/misc/lkdtm/bugs.c | 17 +++++++++++++++++ >>> drivers/misc/lkdtm/core.c | 1 + >>> drivers/misc/lkdtm/lkdtm.h | 1 + >>> 3 files changed, 19 insertions(+) >>> >>> diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c >>> index 7284a22..c9bb493 100644 >>> --- a/drivers/misc/lkdtm/bugs.c >>> +++ b/drivers/misc/lkdtm/bugs.c >>> @@ -337,3 +337,20 @@ void lkdtm_UNSET_SMEP(void) >>> pr_err("FAIL: this test is x86_64-only\n"); >>> #endif >>> } >>> + >>> +void lkdtm_CORRUPT_PAC(void) >>> +{ >>> +#if IS_ENABLED(CONFIG_ARM64_PTR_AUTH) >>> + u64 ret; >>> + >>> + pr_info("Clearing PAC from the return address\n"); >>> + /* >>> + * __builtin_return_address masks the PAC bits of return >>> + * address, so set the same again. >>> + */ >>> + ret = (u64)__builtin_return_address(0); >>> + asm volatile("str %0, [sp, 8]" : : "r" (ret) : "memory"); >> >> This looks a bit fragile to me. You are making assumptions about the >> location of the return address in the stack frame which are not >> guaranteed to hold. > > Indeed. > >> Couldn't you do this test simply by changing the key? > > That, at least, means you don't have to know the stack frame layout. However, > there's a chance (1/32767, I think, for the 48-bit vma case w/o TBI) that > changing the key will result in the same hash. > > Even when the stack frame happens to be layed out as Amit guesses, the result > is akin to changing the key, such that hash(key, salt, ptr) == 0. > > While testing this in qemu, I iterate until I find a <key, salt, ptr> tuple > that definitely produces a different hash. Usually this loop iterates just > once, but the occasional failures that I got without iterating were annoying > (with TBI enabled in userspace, the chance drops to 1/127, so much more frequent). Nice suggestion. I will check doing it this way in the next iteration. > > > r~ >
diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c index 7284a22..c9bb493 100644 --- a/drivers/misc/lkdtm/bugs.c +++ b/drivers/misc/lkdtm/bugs.c @@ -337,3 +337,20 @@ void lkdtm_UNSET_SMEP(void) pr_err("FAIL: this test is x86_64-only\n"); #endif } + +void lkdtm_CORRUPT_PAC(void) +{ +#if IS_ENABLED(CONFIG_ARM64_PTR_AUTH) + u64 ret; + + pr_info("Clearing PAC from the return address\n"); + /* + * __builtin_return_address masks the PAC bits of return + * address, so set the same again. + */ + ret = (u64)__builtin_return_address(0); + asm volatile("str %0, [sp, 8]" : : "r" (ret) : "memory"); +#else + pr_err("FAIL: For arm64 pointer authentication capable systems only\n"); +#endif +} diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c index cbc4c90..b9c9927 100644 --- a/drivers/misc/lkdtm/core.c +++ b/drivers/misc/lkdtm/core.c @@ -116,6 +116,7 @@ static const struct crashtype crashtypes[] = { CRASHTYPE(STACK_GUARD_PAGE_LEADING), CRASHTYPE(STACK_GUARD_PAGE_TRAILING), CRASHTYPE(UNSET_SMEP), + CRASHTYPE(CORRUPT_PAC), CRASHTYPE(UNALIGNED_LOAD_STORE_WRITE), CRASHTYPE(OVERWRITE_ALLOCATION), CRASHTYPE(WRITE_AFTER_FREE), diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h index ab446e0..bf12b68 100644 --- a/drivers/misc/lkdtm/lkdtm.h +++ b/drivers/misc/lkdtm/lkdtm.h @@ -28,6 +28,7 @@ void lkdtm_CORRUPT_USER_DS(void); void lkdtm_STACK_GUARD_PAGE_LEADING(void); void lkdtm_STACK_GUARD_PAGE_TRAILING(void); void lkdtm_UNSET_SMEP(void); +void lkdtm_CORRUPT_PAC(void); /* lkdtm_heap.c */ void __init lkdtm_heap_init(void);
This test is specific for arm64. When in-kernel Pointer Authentication config is enabled, the return address stored in the stack is signed. This feature helps in ROP kind of attack. If the matching signature is corrupted then this will fail in authentication and lead to abort. e.g. echo CORRUPT_PAC > /sys/kernel/debug/provoke-crash/DIRECT [ 13.118166] lkdtm: Performing direct entry CORRUPT_PAC [ 13.118298] lkdtm: Clearing PAC from the return address [ 13.118466] Unable to handle kernel paging request at virtual address bfff8000108648ec [ 13.118626] Mem abort info: [ 13.118666] ESR = 0x86000004 [ 13.118866] EC = 0x21: IABT (current EL), IL = 32 bits [ 13.118966] SET = 0, FnV = 0 [ 13.119117] EA = 0, S1PTW = 0 Cc: Kees Cook <keescook@chromium.org> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com> --- Change since last version: * New patch drivers/misc/lkdtm/bugs.c | 17 +++++++++++++++++ drivers/misc/lkdtm/core.c | 1 + drivers/misc/lkdtm/lkdtm.h | 1 + 3 files changed, 19 insertions(+)