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

From: Amit Kachhap <amit.kachhap@arm.com>
To: Dave Martin <Dave.Martin@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Kees Cook <keescook@chromium.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Mark Brown <broonie@kernel.org>,
	James Morse <james.morse@arm.com>,
	Vincenzo Frascino <Vincenzo.Frascino@arm.com>,
	Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 3/4] arm64: kprobe: disable probe of fault prone ptrauth instruction
Date: Mon, 3 Aug 2020 15:46:48 +0530	[thread overview]
Message-ID: <4ab1358f-62b8-751b-7257-3df678ba1201@arm.com> (raw)
In-Reply-To: <20200729104332.GD21941@arm.com>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel