All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/4] arm64: Make NOP handling a whitelist
@ 2020-05-04 13:13 Mark Brown
  2020-05-04 13:13 ` [PATCH v4 1/4] arm64: insn: Add constants for new HINT instruction decode Mark Brown
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Mark Brown @ 2020-05-04 13:13 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: Mark Rutland, Mark Brown, linux-arm-kernel

Currently we default to assuming any unrecognized instruction in the
hint space can be safely handled as a NOP.  This is not robust and any
code that really wants a NOP should be using the explicitly defined NOP
so let's instead invert this and whitelist those instructions which it
is safe to handle as NOPs.

Patch 1 adds defines for the HINTs for BTI landing pads which will be
used by the in-kernel BTI series to generate landing pads in JITed BPF
code so it'd be good if this could be applied on or merged into the BTI
branch.

v4:
 - Add a patch renaming _is_nop() to make the usage clearer.
 - Reorder series and tweak commit logs.
v3:
 - Correct values for more of the HINTs.
 - Add constants for additional HINTs
 - Handle XPACLRI as a NOP along with all the other PAC HINTs.
 - Update commit messages for greater clarity.
 - Tweak the comment about what a NOP.
v2:
 - Fix values for BTI HINTs.
 - Rebase on v5.7-rc3+for-next/bti-user

Mark Brown (4):
  arm64: insn: Add constants for new HINT instruction decode
  arm64: insn: Provide a better name for aarch64_insn_is_nop()
  arm64: insn: Don't assume unrecognized HINTs are skippable
  arm64: insn: Report PAC and BTI instructions as skippable

 arch/arm64/include/asm/insn.h          | 30 +++++++++++++++++++++---
 arch/arm64/kernel/insn.c               | 32 ++++++++++++++++++--------
 arch/arm64/kernel/probes/decode-insn.c |  2 +-
 3 files changed, 50 insertions(+), 14 deletions(-)

-- 
2.20.1


_______________________________________________
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] 10+ messages in thread

* [PATCH v4 1/4] arm64: insn: Add constants for new HINT instruction decode
  2020-05-04 13:13 [PATCH v4 0/4] arm64: Make NOP handling a whitelist Mark Brown
@ 2020-05-04 13:13 ` Mark Brown
  2020-05-04 14:05   ` Mark Rutland
  2020-05-04 13:13 ` [PATCH v4 2/4] arm64: insn: Provide a better name for aarch64_insn_is_nop() Mark Brown
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2020-05-04 13:13 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: Mark Rutland, Mark Brown, linux-arm-kernel

Add constants for decoding newer instructions defined in the HINT space.
Since we are now decoding both the op2 and CRm fields rename the enum as
well; this is compatible with what the existing users are doing.

Signed-off-by: Mark Brown <broonie@kernel.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/insn.h | 28 ++++++++++++++++++++++++++--
 arch/arm64/kernel/insn.c      |  2 +-
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index bb313dde58a4..575675145fe2 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -39,13 +39,37 @@ enum aarch64_insn_encoding_class {
 					 * system instructions */
 };
 
-enum aarch64_insn_hint_op {
+enum aarch64_insn_hint_cr_op {
 	AARCH64_INSN_HINT_NOP	= 0x0 << 5,
 	AARCH64_INSN_HINT_YIELD	= 0x1 << 5,
 	AARCH64_INSN_HINT_WFE	= 0x2 << 5,
 	AARCH64_INSN_HINT_WFI	= 0x3 << 5,
 	AARCH64_INSN_HINT_SEV	= 0x4 << 5,
 	AARCH64_INSN_HINT_SEVL	= 0x5 << 5,
+
+	AARCH64_INSN_HINT_XPACLRI    = 0x07 << 5,
+	AARCH64_INSN_HINT_PACIA_1716 = 0x08 << 5,
+	AARCH64_INSN_HINT_PACIB_1716 = 0x0A << 5,
+	AARCH64_INSN_HINT_AUTIA_1716 = 0x0C << 5,
+	AARCH64_INSN_HINT_AUTIB_1716 = 0x0E << 5,
+	AARCH64_INSN_HINT_PACIAZ     = 0x18 << 5,
+	AARCH64_INSN_HINT_PACIASP    = 0x19 << 5,
+	AARCH64_INSN_HINT_PACIBZ     = 0x1A << 5,
+	AARCH64_INSN_HINT_PACIBSP    = 0x1B << 5,
+	AARCH64_INSN_HINT_AUTIAZ     = 0x1C << 5,
+	AARCH64_INSN_HINT_AUTIASP    = 0x1D << 5,
+	AARCH64_INSN_HINT_AUTIBZ     = 0x1E << 5,
+	AARCH64_INSN_HINT_AUTIBSP    = 0x1F << 5,
+
+	AARCH64_INSN_HINT_ESB  = 0x10 << 5,
+	AARCH64_INSN_HINT_PSB  = 0x11 << 5,
+	AARCH64_INSN_HINT_TSB  = 0x12 << 5,
+	AARCH64_INSN_HINT_CSDB = 0x14 << 5,
+
+	AARCH64_INSN_HINT_BTI   = 0x20 << 5,
+	AARCH64_INSN_HINT_BTIC  = 0x22 << 5,
+	AARCH64_INSN_HINT_BTIJ  = 0x24 << 5,
+	AARCH64_INSN_HINT_BTIJC = 0x26 << 5,
 };
 
 enum aarch64_insn_imm_type {
@@ -370,7 +394,7 @@ u32 aarch64_insn_gen_comp_branch_imm(unsigned long pc, unsigned long addr,
 				     enum aarch64_insn_branch_type type);
 u32 aarch64_insn_gen_cond_branch_imm(unsigned long pc, unsigned long addr,
 				     enum aarch64_insn_condition cond);
-u32 aarch64_insn_gen_hint(enum aarch64_insn_hint_op op);
+u32 aarch64_insn_gen_hint(enum aarch64_insn_hint_cr_op op);
 u32 aarch64_insn_gen_nop(void);
 u32 aarch64_insn_gen_branch_reg(enum aarch64_insn_register reg,
 				enum aarch64_insn_branch_type type);
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index 4a9e773a177f..d63d9cd8b4a2 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -574,7 +574,7 @@ u32 aarch64_insn_gen_cond_branch_imm(unsigned long pc, unsigned long addr,
 					     offset >> 2);
 }
 
-u32 __kprobes aarch64_insn_gen_hint(enum aarch64_insn_hint_op op)
+u32 __kprobes aarch64_insn_gen_hint(enum aarch64_insn_hint_cr_op op)
 {
 	return aarch64_insn_get_hint_value() | op;
 }
-- 
2.20.1


_______________________________________________
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] 10+ messages in thread

* [PATCH v4 2/4] arm64: insn: Provide a better name for aarch64_insn_is_nop()
  2020-05-04 13:13 [PATCH v4 0/4] arm64: Make NOP handling a whitelist Mark Brown
  2020-05-04 13:13 ` [PATCH v4 1/4] arm64: insn: Add constants for new HINT instruction decode Mark Brown
@ 2020-05-04 13:13 ` Mark Brown
  2020-05-04 13:40   ` Mark Rutland
  2020-05-04 13:13 ` [PATCH v4 3/4] arm64: insn: Don't assume unrecognized HINTs are skippable Mark Brown
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2020-05-04 13:13 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: Mark Rutland, Mark Brown, linux-arm-kernel

The current aarch64_insn_is_nop() has exactly one caller which uses it
solely to identify if the instruction is a HINT that can safely be stepped,
requiring us to list things that aren't NOPs and make things more confusing
than they need to be. Rename the function to reflect the actual usage and
make things more clear.

Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/asm/insn.h          | 2 +-
 arch/arm64/kernel/insn.c               | 3 +--
 arch/arm64/kernel/probes/decode-insn.c | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index 575675145fe2..0bc46149e491 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -368,7 +368,7 @@ __AARCH64_INSN_FUNCS(msr_reg,	0xFFF00000, 0xD5100000)
 
 #undef	__AARCH64_INSN_FUNCS
 
-bool aarch64_insn_is_nop(u32 insn);
+bool aarch64_insn_is_steppable_hint(u32 insn);
 bool aarch64_insn_is_branch_imm(u32 insn);
 
 static inline bool aarch64_insn_is_adr_adrp(u32 insn)
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index d63d9cd8b4a2..0829bb5b45ec 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -51,8 +51,7 @@ enum aarch64_insn_encoding_class __kprobes aarch64_get_insn_class(u32 insn)
 	return aarch64_insn_encoding_class[(insn >> 25) & 0xf];
 }
 
-/* NOP is an alias of HINT */
-bool __kprobes aarch64_insn_is_nop(u32 insn)
+bool __kprobes aarch64_insn_is_steppable_hint(u32 insn)
 {
 	if (!aarch64_insn_is_hint(insn))
 		return false;
diff --git a/arch/arm64/kernel/probes/decode-insn.c b/arch/arm64/kernel/probes/decode-insn.c
index b78fac9e546c..263d5fba4c8a 100644
--- a/arch/arm64/kernel/probes/decode-insn.c
+++ b/arch/arm64/kernel/probes/decode-insn.c
@@ -46,7 +46,7 @@ static bool __kprobes aarch64_insn_is_steppable(u32 insn)
 		 * except for the NOP case.
 		 */
 		if (aarch64_insn_is_hint(insn))
-			return aarch64_insn_is_nop(insn);
+			return aarch64_insn_is_steppable_hint(insn);
 
 		return true;
 	}
-- 
2.20.1


_______________________________________________
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] 10+ messages in thread

* [PATCH v4 3/4] arm64: insn: Don't assume unrecognized HINTs are skippable
  2020-05-04 13:13 [PATCH v4 0/4] arm64: Make NOP handling a whitelist Mark Brown
  2020-05-04 13:13 ` [PATCH v4 1/4] arm64: insn: Add constants for new HINT instruction decode Mark Brown
  2020-05-04 13:13 ` [PATCH v4 2/4] arm64: insn: Provide a better name for aarch64_insn_is_nop() Mark Brown
@ 2020-05-04 13:13 ` Mark Brown
  2020-05-04 13:44   ` Mark Rutland
  2020-05-04 13:13 ` [PATCH v4 4/4] arm64: insn: Report PAC and BTI instructions as skippable Mark Brown
  2020-05-04 15:33 ` [PATCH v4 0/4] arm64: Make NOP handling a whitelist Will Deacon
  4 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2020-05-04 13:13 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: Mark Rutland, Mark Brown, linux-arm-kernel

Currently the kernel assumes that any HINT which it does not explicitly
recognise is skippable.  This is not robust as new instructions may be
added which need special handling, and in any case software should only
be using explicit NOP instructions for deliberate NOPs.

This has the effect of rendering PAC and BTI instructions unprobeable
which means that probes can't be inserted on the first instruction of
functions built with those features.

Signed-off-by: Mark Brown <broonie@kernel.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/kernel/insn.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index 0829bb5b45ec..15c3f0643e3b 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -57,14 +57,10 @@ bool __kprobes aarch64_insn_is_steppable_hint(u32 insn)
 		return false;
 
 	switch (insn & 0xFE0) {
-	case AARCH64_INSN_HINT_YIELD:
-	case AARCH64_INSN_HINT_WFE:
-	case AARCH64_INSN_HINT_WFI:
-	case AARCH64_INSN_HINT_SEV:
-	case AARCH64_INSN_HINT_SEVL:
-		return false;
-	default:
+	case AARCH64_INSN_HINT_NOP:
 		return true;
+	default:
+		return false;
 	}
 }
 
-- 
2.20.1


_______________________________________________
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] 10+ messages in thread

* [PATCH v4 4/4] arm64: insn: Report PAC and BTI instructions as skippable
  2020-05-04 13:13 [PATCH v4 0/4] arm64: Make NOP handling a whitelist Mark Brown
                   ` (2 preceding siblings ...)
  2020-05-04 13:13 ` [PATCH v4 3/4] arm64: insn: Don't assume unrecognized HINTs are skippable Mark Brown
@ 2020-05-04 13:13 ` Mark Brown
  2020-05-04 13:43   ` Mark Rutland
  2020-05-04 15:33 ` [PATCH v4 0/4] arm64: Make NOP handling a whitelist Will Deacon
  4 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2020-05-04 13:13 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: Mark Rutland, Mark Brown, linux-arm-kernel

The PAC and BTI instructions can be safely skipped so report them as
such, allowing them to be probed.

Signed-off-by: Mark Brown <broonie@kernel.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/kernel/insn.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index 15c3f0643e3b..6439af794ec4 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -57,6 +57,23 @@ bool __kprobes aarch64_insn_is_steppable_hint(u32 insn)
 		return false;
 
 	switch (insn & 0xFE0) {
+	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:
+	case AARCH64_INSN_HINT_BTIJC:
 	case AARCH64_INSN_HINT_NOP:
 		return true;
 	default:
-- 
2.20.1


_______________________________________________
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] 10+ messages in thread

* Re: [PATCH v4 2/4] arm64: insn: Provide a better name for aarch64_insn_is_nop()
  2020-05-04 13:13 ` [PATCH v4 2/4] arm64: insn: Provide a better name for aarch64_insn_is_nop() Mark Brown
@ 2020-05-04 13:40   ` Mark Rutland
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2020-05-04 13:40 UTC (permalink / raw)
  To: Mark Brown; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel

On Mon, May 04, 2020 at 02:13:24PM +0100, Mark Brown wrote:
> The current aarch64_insn_is_nop() has exactly one caller which uses it
> solely to identify if the instruction is a HINT that can safely be stepped,
> requiring us to list things that aren't NOPs and make things more confusing
> than they need to be. Rename the function to reflect the actual usage and
> make things more clear.
> 
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>

Thanks for this!

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
>  arch/arm64/include/asm/insn.h          | 2 +-
>  arch/arm64/kernel/insn.c               | 3 +--
>  arch/arm64/kernel/probes/decode-insn.c | 2 +-
>  3 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
> index 575675145fe2..0bc46149e491 100644
> --- a/arch/arm64/include/asm/insn.h
> +++ b/arch/arm64/include/asm/insn.h
> @@ -368,7 +368,7 @@ __AARCH64_INSN_FUNCS(msr_reg,	0xFFF00000, 0xD5100000)
>  
>  #undef	__AARCH64_INSN_FUNCS
>  
> -bool aarch64_insn_is_nop(u32 insn);
> +bool aarch64_insn_is_steppable_hint(u32 insn);
>  bool aarch64_insn_is_branch_imm(u32 insn);
>  
>  static inline bool aarch64_insn_is_adr_adrp(u32 insn)
> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
> index d63d9cd8b4a2..0829bb5b45ec 100644
> --- a/arch/arm64/kernel/insn.c
> +++ b/arch/arm64/kernel/insn.c
> @@ -51,8 +51,7 @@ enum aarch64_insn_encoding_class __kprobes aarch64_get_insn_class(u32 insn)
>  	return aarch64_insn_encoding_class[(insn >> 25) & 0xf];
>  }
>  
> -/* NOP is an alias of HINT */
> -bool __kprobes aarch64_insn_is_nop(u32 insn)
> +bool __kprobes aarch64_insn_is_steppable_hint(u32 insn)
>  {
>  	if (!aarch64_insn_is_hint(insn))
>  		return false;
> diff --git a/arch/arm64/kernel/probes/decode-insn.c b/arch/arm64/kernel/probes/decode-insn.c
> index b78fac9e546c..263d5fba4c8a 100644
> --- a/arch/arm64/kernel/probes/decode-insn.c
> +++ b/arch/arm64/kernel/probes/decode-insn.c
> @@ -46,7 +46,7 @@ static bool __kprobes aarch64_insn_is_steppable(u32 insn)
>  		 * except for the NOP case.
>  		 */
>  		if (aarch64_insn_is_hint(insn))
> -			return aarch64_insn_is_nop(insn);
> +			return aarch64_insn_is_steppable_hint(insn);
>  
>  		return true;
>  	}
> -- 
> 2.20.1
> 

_______________________________________________
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] 10+ messages in thread

* Re: [PATCH v4 4/4] arm64: insn: Report PAC and BTI instructions as skippable
  2020-05-04 13:13 ` [PATCH v4 4/4] arm64: insn: Report PAC and BTI instructions as skippable Mark Brown
@ 2020-05-04 13:43   ` Mark Rutland
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2020-05-04 13:43 UTC (permalink / raw)
  To: Mark Brown; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel

On Mon, May 04, 2020 at 02:13:26PM +0100, Mark Brown wrote:
> The PAC and BTI instructions can be safely skipped so report them as
> such, allowing them to be probed.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

These all look safe to step in a XOL area, so:

Acked-by: Mark Rutland <mark.rutland@arm.com>

> ---
>  arch/arm64/kernel/insn.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
> index 15c3f0643e3b..6439af794ec4 100644
> --- a/arch/arm64/kernel/insn.c
> +++ b/arch/arm64/kernel/insn.c
> @@ -57,6 +57,23 @@ bool __kprobes aarch64_insn_is_steppable_hint(u32 insn)
>  		return false;
>  
>  	switch (insn & 0xFE0) {
> +	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:
> +	case AARCH64_INSN_HINT_BTIJC:

Just to check -- do we set the GP flag when creating/mapping the XOL
area? If that's PAGE_KERNEL_EXEC, presumably we do.

Mark.

>  	case AARCH64_INSN_HINT_NOP:
>  		return true;
>  	default:
> -- 
> 2.20.1
> 

_______________________________________________
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] 10+ messages in thread

* Re: [PATCH v4 3/4] arm64: insn: Don't assume unrecognized HINTs are skippable
  2020-05-04 13:13 ` [PATCH v4 3/4] arm64: insn: Don't assume unrecognized HINTs are skippable Mark Brown
@ 2020-05-04 13:44   ` Mark Rutland
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2020-05-04 13:44 UTC (permalink / raw)
  To: Mark Brown; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel

On Mon, May 04, 2020 at 02:13:25PM +0100, Mark Brown wrote:
> Currently the kernel assumes that any HINT which it does not explicitly
> recognise is skippable.  This is not robust as new instructions may be
> added which need special handling, and in any case software should only
> be using explicit NOP instructions for deliberate NOPs.
> 
> This has the effect of rendering PAC and BTI instructions unprobeable
> which means that probes can't be inserted on the first instruction of
> functions built with those features.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
>  arch/arm64/kernel/insn.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
> index 0829bb5b45ec..15c3f0643e3b 100644
> --- a/arch/arm64/kernel/insn.c
> +++ b/arch/arm64/kernel/insn.c
> @@ -57,14 +57,10 @@ bool __kprobes aarch64_insn_is_steppable_hint(u32 insn)
>  		return false;
>  
>  	switch (insn & 0xFE0) {
> -	case AARCH64_INSN_HINT_YIELD:
> -	case AARCH64_INSN_HINT_WFE:
> -	case AARCH64_INSN_HINT_WFI:
> -	case AARCH64_INSN_HINT_SEV:
> -	case AARCH64_INSN_HINT_SEVL:
> -		return false;
> -	default:
> +	case AARCH64_INSN_HINT_NOP:
>  		return true;
> +	default:
> +		return false;
>  	}
>  }
>  
> -- 
> 2.20.1
> 

_______________________________________________
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] 10+ messages in thread

* Re: [PATCH v4 1/4] arm64: insn: Add constants for new HINT instruction decode
  2020-05-04 13:13 ` [PATCH v4 1/4] arm64: insn: Add constants for new HINT instruction decode Mark Brown
@ 2020-05-04 14:05   ` Mark Rutland
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2020-05-04 14:05 UTC (permalink / raw)
  To: Mark Brown; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel

On Mon, May 04, 2020 at 02:13:23PM +0100, Mark Brown wrote:
> Add constants for decoding newer instructions defined in the HINT space.
> Since we are now decoding both the op2 and CRm fields rename the enum as
> well; this is compatible with what the existing users are doing.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

These all look correct to me per the "Architectural hint instructions"
table on page C5-378 of ARM DDI 0487F.a, so:

Acked-by: Mark Rutland <mark.rutland@arm.com>

It looks like DGH was added recently, so we might want to follow up with
that at some point, but it's certianly not necessary now.

Mark.

> ---
>  arch/arm64/include/asm/insn.h | 28 ++++++++++++++++++++++++++--
>  arch/arm64/kernel/insn.c      |  2 +-
>  2 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
> index bb313dde58a4..575675145fe2 100644
> --- a/arch/arm64/include/asm/insn.h
> +++ b/arch/arm64/include/asm/insn.h
> @@ -39,13 +39,37 @@ enum aarch64_insn_encoding_class {
>  					 * system instructions */
>  };
>  
> -enum aarch64_insn_hint_op {
> +enum aarch64_insn_hint_cr_op {
>  	AARCH64_INSN_HINT_NOP	= 0x0 << 5,
>  	AARCH64_INSN_HINT_YIELD	= 0x1 << 5,
>  	AARCH64_INSN_HINT_WFE	= 0x2 << 5,
>  	AARCH64_INSN_HINT_WFI	= 0x3 << 5,
>  	AARCH64_INSN_HINT_SEV	= 0x4 << 5,
>  	AARCH64_INSN_HINT_SEVL	= 0x5 << 5,
> +
> +	AARCH64_INSN_HINT_XPACLRI    = 0x07 << 5,
> +	AARCH64_INSN_HINT_PACIA_1716 = 0x08 << 5,
> +	AARCH64_INSN_HINT_PACIB_1716 = 0x0A << 5,
> +	AARCH64_INSN_HINT_AUTIA_1716 = 0x0C << 5,
> +	AARCH64_INSN_HINT_AUTIB_1716 = 0x0E << 5,
> +	AARCH64_INSN_HINT_PACIAZ     = 0x18 << 5,
> +	AARCH64_INSN_HINT_PACIASP    = 0x19 << 5,
> +	AARCH64_INSN_HINT_PACIBZ     = 0x1A << 5,
> +	AARCH64_INSN_HINT_PACIBSP    = 0x1B << 5,
> +	AARCH64_INSN_HINT_AUTIAZ     = 0x1C << 5,
> +	AARCH64_INSN_HINT_AUTIASP    = 0x1D << 5,
> +	AARCH64_INSN_HINT_AUTIBZ     = 0x1E << 5,
> +	AARCH64_INSN_HINT_AUTIBSP    = 0x1F << 5,
> +
> +	AARCH64_INSN_HINT_ESB  = 0x10 << 5,
> +	AARCH64_INSN_HINT_PSB  = 0x11 << 5,
> +	AARCH64_INSN_HINT_TSB  = 0x12 << 5,
> +	AARCH64_INSN_HINT_CSDB = 0x14 << 5,
> +
> +	AARCH64_INSN_HINT_BTI   = 0x20 << 5,
> +	AARCH64_INSN_HINT_BTIC  = 0x22 << 5,
> +	AARCH64_INSN_HINT_BTIJ  = 0x24 << 5,
> +	AARCH64_INSN_HINT_BTIJC = 0x26 << 5,
>  };
>  
>  enum aarch64_insn_imm_type {
> @@ -370,7 +394,7 @@ u32 aarch64_insn_gen_comp_branch_imm(unsigned long pc, unsigned long addr,
>  				     enum aarch64_insn_branch_type type);
>  u32 aarch64_insn_gen_cond_branch_imm(unsigned long pc, unsigned long addr,
>  				     enum aarch64_insn_condition cond);
> -u32 aarch64_insn_gen_hint(enum aarch64_insn_hint_op op);
> +u32 aarch64_insn_gen_hint(enum aarch64_insn_hint_cr_op op);
>  u32 aarch64_insn_gen_nop(void);
>  u32 aarch64_insn_gen_branch_reg(enum aarch64_insn_register reg,
>  				enum aarch64_insn_branch_type type);
> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
> index 4a9e773a177f..d63d9cd8b4a2 100644
> --- a/arch/arm64/kernel/insn.c
> +++ b/arch/arm64/kernel/insn.c
> @@ -574,7 +574,7 @@ u32 aarch64_insn_gen_cond_branch_imm(unsigned long pc, unsigned long addr,
>  					     offset >> 2);
>  }
>  
> -u32 __kprobes aarch64_insn_gen_hint(enum aarch64_insn_hint_op op)
> +u32 __kprobes aarch64_insn_gen_hint(enum aarch64_insn_hint_cr_op op)
>  {
>  	return aarch64_insn_get_hint_value() | op;
>  }
> -- 
> 2.20.1
> 

_______________________________________________
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] 10+ messages in thread

* Re: [PATCH v4 0/4] arm64: Make NOP handling a whitelist
  2020-05-04 13:13 [PATCH v4 0/4] arm64: Make NOP handling a whitelist Mark Brown
                   ` (3 preceding siblings ...)
  2020-05-04 13:13 ` [PATCH v4 4/4] arm64: insn: Report PAC and BTI instructions as skippable Mark Brown
@ 2020-05-04 15:33 ` Will Deacon
  4 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2020-05-04 15:33 UTC (permalink / raw)
  To: Catalin Marinas, Mark Brown; +Cc: Mark Rutland, Will Deacon, linux-arm-kernel

On Mon, 4 May 2020 14:13:22 +0100, Mark Brown wrote:
> Currently we default to assuming any unrecognized instruction in the
> hint space can be safely handled as a NOP.  This is not robust and any
> code that really wants a NOP should be using the explicitly defined NOP
> so let's instead invert this and whitelist those instructions which it
> is safe to handle as NOPs.
> 
> Patch 1 adds defines for the HINTs for BTI landing pads which will be
> used by the in-kernel BTI series to generate landing pads in JITed BPF
> code so it'd be good if this could be applied on or merged into the BTI
> branch.
> 
> [...]

Applied to arm64 (for-next/insn), thanks!

[1/4] arm64: insn: Add constants for new HINT instruction decode
      https://git.kernel.org/arm64/c/bd507ca2773b
[2/4] arm64: insn: Provide a better name for aarch64_insn_is_nop()
      https://git.kernel.org/arm64/c/07dcd9677c5d
[3/4] arm64: insn: Don't assume unrecognized HINTs are skippable
      https://git.kernel.org/arm64/c/c71052cc9e14
[4/4] arm64: insn: Report PAC and BTI instructions as skippable
      https://git.kernel.org/arm64/c/47d67e4d1918

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev

_______________________________________________
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] 10+ messages in thread

end of thread, other threads:[~2020-05-04 15:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-04 13:13 [PATCH v4 0/4] arm64: Make NOP handling a whitelist Mark Brown
2020-05-04 13:13 ` [PATCH v4 1/4] arm64: insn: Add constants for new HINT instruction decode Mark Brown
2020-05-04 14:05   ` Mark Rutland
2020-05-04 13:13 ` [PATCH v4 2/4] arm64: insn: Provide a better name for aarch64_insn_is_nop() Mark Brown
2020-05-04 13:40   ` Mark Rutland
2020-05-04 13:13 ` [PATCH v4 3/4] arm64: insn: Don't assume unrecognized HINTs are skippable Mark Brown
2020-05-04 13:44   ` Mark Rutland
2020-05-04 13:13 ` [PATCH v4 4/4] arm64: insn: Report PAC and BTI instructions as skippable Mark Brown
2020-05-04 13:43   ` Mark Rutland
2020-05-04 15:33 ` [PATCH v4 0/4] arm64: Make NOP handling a whitelist Will Deacon

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.