diff mbox series

[v4,3/4] arm64: kprobe: disable probe of fault prone ptrauth instruction

Message ID 1594368010-4419-4-git-send-email-amit.kachhap@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: add Armv8.3 pointer authentication enhancements | expand

Commit Message

Amit Daniel Kachhap July 10, 2020, 8 a.m. UTC
With the addition of ARMv8.3-FPAC feature, the probe of authenticate
ptrauth instructions (AUT*) may cause ptrauth fault exception in case of
authenticate failure so they cannot be safely single stepped.

Hence the probe of authenticate instructions is disallowed but the
corresponding pac ptrauth instruction (PAC*) is not affected and they can
still be probed. Also AUTH* instructions do not make sense at function
entry points so most realistic probes would be unaffected by this change.

Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>
---
Changes since v3:
 * Commit logs cleanup.
 * Moved comment changes in a separate patch.

 arch/arm64/kernel/insn.c | 6 ------
 1 file changed, 6 deletions(-)

Comments

Dave Martin July 29, 2020, 10:43 a.m. UTC | #1
On Fri, Jul 10, 2020 at 01:30:09PM +0530, Amit Daniel Kachhap wrote:
> With the addition of ARMv8.3-FPAC feature, the probe of authenticate
> ptrauth instructions (AUT*) may cause ptrauth fault exception in case of
> authenticate failure so they cannot be safely single stepped.
> 
> Hence the probe of authenticate instructions is disallowed but the
> corresponding pac ptrauth instruction (PAC*) is not affected and they can
> still be probed. Also AUTH* instructions do not make sense at function
> entry points so most realistic probes would be unaffected by this change.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>

I take it we don't need any special handling of things like RETAA now
that they are allowed to generate ptrauth faults?  IIUC such
instructions are already not simulated and not stepped out-of-line, so
we probably don't need to do anything.  Instructions like this won't
appear at normal function entry points.

Assuming what I've said above is correct:

Reviewed-by: Dave Martin <Dave.Martin@arm.com>

> ---
> Changes since v3:
>  * Commit logs cleanup.
>  * Moved comment changes in a separate patch.
> 
>  arch/arm64/kernel/insn.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
> index a107375005bc..33d53cb46542 100644
> --- a/arch/arm64/kernel/insn.c
> +++ b/arch/arm64/kernel/insn.c
> @@ -60,16 +60,10 @@ bool __kprobes aarch64_insn_is_steppable_hint(u32 insn)
>  	case AARCH64_INSN_HINT_XPACLRI:
>  	case AARCH64_INSN_HINT_PACIA_1716:
>  	case AARCH64_INSN_HINT_PACIB_1716:
> -	case AARCH64_INSN_HINT_AUTIA_1716:
> -	case AARCH64_INSN_HINT_AUTIB_1716:
>  	case AARCH64_INSN_HINT_PACIAZ:
>  	case AARCH64_INSN_HINT_PACIASP:
>  	case AARCH64_INSN_HINT_PACIBZ:
>  	case AARCH64_INSN_HINT_PACIBSP:
> -	case AARCH64_INSN_HINT_AUTIAZ:
> -	case AARCH64_INSN_HINT_AUTIASP:
> -	case AARCH64_INSN_HINT_AUTIBZ:
> -	case AARCH64_INSN_HINT_AUTIBSP:
>  	case AARCH64_INSN_HINT_BTI:
>  	case AARCH64_INSN_HINT_BTIC:
>  	case AARCH64_INSN_HINT_BTIJ:
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Amit Daniel Kachhap Aug. 3, 2020, 10:16 a.m. UTC | #2
Hi,

On 7/29/20 4:13 PM, Dave Martin wrote:
> On Fri, Jul 10, 2020 at 01:30:09PM +0530, Amit Daniel Kachhap wrote:
>> With the addition of ARMv8.3-FPAC feature, the probe of authenticate
>> ptrauth instructions (AUT*) may cause ptrauth fault exception in case of
>> authenticate failure so they cannot be safely single stepped.
>>
>> Hence the probe of authenticate instructions is disallowed but the
>> corresponding pac ptrauth instruction (PAC*) is not affected and they can
>> still be probed. Also AUTH* instructions do not make sense at function
>> entry points so most realistic probes would be unaffected by this change.
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>
> 
> I take it we don't need any special handling of things like RETAA now
> that they are allowed to generate ptrauth faults?  IIUC such
> instructions are already not simulated and not stepped out-of-line, so
> we probably don't need to do anything.  Instructions like this won't
> appear at normal function entry points.

There is an issue currently with retaa(all combined instructions) as 
such branch instructions are not checked and code breaks later. I will 
push a fix as a separate patch.

> 
> Assuming what I've said above is correct:
> 
> Reviewed-by: Dave Martin <Dave.Martin@arm.com>

Thanks for reviewing.

> 
>> ---
>> Changes since v3:
>>   * Commit logs cleanup.
>>   * Moved comment changes in a separate patch.
>>
>>   arch/arm64/kernel/insn.c | 6 ------
>>   1 file changed, 6 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
>> index a107375005bc..33d53cb46542 100644
>> --- a/arch/arm64/kernel/insn.c
>> +++ b/arch/arm64/kernel/insn.c
>> @@ -60,16 +60,10 @@ bool __kprobes aarch64_insn_is_steppable_hint(u32 insn)
>>   	case AARCH64_INSN_HINT_XPACLRI:
>>   	case AARCH64_INSN_HINT_PACIA_1716:
>>   	case AARCH64_INSN_HINT_PACIB_1716:
>> -	case AARCH64_INSN_HINT_AUTIA_1716:
>> -	case AARCH64_INSN_HINT_AUTIB_1716:
>>   	case AARCH64_INSN_HINT_PACIAZ:
>>   	case AARCH64_INSN_HINT_PACIASP:
>>   	case AARCH64_INSN_HINT_PACIBZ:
>>   	case AARCH64_INSN_HINT_PACIBSP:
>> -	case AARCH64_INSN_HINT_AUTIAZ:
>> -	case AARCH64_INSN_HINT_AUTIASP:
>> -	case AARCH64_INSN_HINT_AUTIBZ:
>> -	case AARCH64_INSN_HINT_AUTIBSP:
>>   	case AARCH64_INSN_HINT_BTI:
>>   	case AARCH64_INSN_HINT_BTIC:
>>   	case AARCH64_INSN_HINT_BTIJ:
>> -- 
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox series

Patch

diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index a107375005bc..33d53cb46542 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -60,16 +60,10 @@  bool __kprobes aarch64_insn_is_steppable_hint(u32 insn)
 	case AARCH64_INSN_HINT_XPACLRI:
 	case AARCH64_INSN_HINT_PACIA_1716:
 	case AARCH64_INSN_HINT_PACIB_1716:
-	case AARCH64_INSN_HINT_AUTIA_1716:
-	case AARCH64_INSN_HINT_AUTIB_1716:
 	case AARCH64_INSN_HINT_PACIAZ:
 	case AARCH64_INSN_HINT_PACIASP:
 	case AARCH64_INSN_HINT_PACIBZ:
 	case AARCH64_INSN_HINT_PACIBSP:
-	case AARCH64_INSN_HINT_AUTIAZ:
-	case AARCH64_INSN_HINT_AUTIASP:
-	case AARCH64_INSN_HINT_AUTIBZ:
-	case AARCH64_INSN_HINT_AUTIBSP:
 	case AARCH64_INSN_HINT_BTI:
 	case AARCH64_INSN_HINT_BTIC:
 	case AARCH64_INSN_HINT_BTIJ: