[PATCH] arm64: bti: Fix support for userspace only BTI

[PATCH] arm64: bti: Fix support for userspace only BTI

[Mark Brown]

When setting PTE_MAYBE_GP we check system_supports_bti() but this is
true for systems where only CONFIG_BTI is set causing us to enable BTI
on some kernel text. Add an extra check for the kernel mode option,
using an ifdef due to line length.

Fixes: c8027285e366 (arm64: Set GP bit in kernel page tables to enable BTI for the kernel)
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/asm/pgtable-prot.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
index 310690332896..3f07a044bdfb 100644
--- a/arch/arm64/include/asm/pgtable-prot.h
+++ b/arch/arm64/include/asm/pgtable-prot.h
@@ -32,7 +32,11 @@ extern bool arm64_use_ng_mappings;
 #define PTE_MAYBE_NG		(arm64_use_ng_mappings ? PTE_NG : 0)
 #define PMD_MAYBE_NG		(arm64_use_ng_mappings ? PMD_SECT_NG : 0)
 
+#ifdef CONFIG_ARM64_BTI_KERNEL
 #define PTE_MAYBE_GP		(system_supports_bti() ? PTE_GP : 0)
+#else
+#define PTE_MAYBE_GP		0
+#endif
 
 #define PROT_DEFAULT		(_PROT_DEFAULT | PTE_MAYBE_NG)
 #define PROT_SECT_DEFAULT	(_PROT_SECT_DEFAULT | PMD_MAYBE_NG)
-- 
2.20.1


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

Re: [PATCH] arm64: bti: Fix support for userspace only BTI

[Will Deacon]

On Tue, May 12, 2020 at 10:21:55AM +0100, Mark Brown wrote:
> When setting PTE_MAYBE_GP we check system_supports_bti() but this is
> true for systems where only CONFIG_BTI is set causing us to enable BTI
> on some kernel text. Add an extra check for the kernel mode option,
> using an ifdef due to line length.
> 
> Fixes: c8027285e366 (arm64: Set GP bit in kernel page tables to enable BTI for the kernel)
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/pgtable-prot.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
> index 310690332896..3f07a044bdfb 100644
> --- a/arch/arm64/include/asm/pgtable-prot.h
> +++ b/arch/arm64/include/asm/pgtable-prot.h
> @@ -32,7 +32,11 @@ extern bool arm64_use_ng_mappings;
>  #define PTE_MAYBE_NG		(arm64_use_ng_mappings ? PTE_NG : 0)
>  #define PMD_MAYBE_NG		(arm64_use_ng_mappings ? PMD_SECT_NG : 0)
>

Might be worth a comment here in case somebody tries to "clean this up"
later on!

> +#ifdef CONFIG_ARM64_BTI_KERNEL
>  #define PTE_MAYBE_GP		(system_supports_bti() ? PTE_GP : 0)
> +#else
> +#define PTE_MAYBE_GP		0
> +#endif

I also notice we have a funny check in the vdso code:

  if (IS_ENABLED(CONFIG_ARM64_BTI_KERNEL) && system_supports_bti())

but why do we care about CONFIG_ARM64_BTI_KERNEL for the vdso pages?

Will

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

Re: [PATCH] arm64: bti: Fix support for userspace only BTI

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 882 bytes --]

On Tue, May 12, 2020 at 11:39:09AM +0100, Will Deacon wrote:

> Might be worth a comment here in case somebody tries to "clean this up"
> later on!

> > +#ifdef CONFIG_ARM64_BTI_KERNEL
> >  #define PTE_MAYBE_GP		(system_supports_bti() ? PTE_GP : 0)
> > +#else
> > +#define PTE_MAYBE_GP		0
> > +#endif

Sure.

> I also notice we have a funny check in the vdso code:

>   if (IS_ENABLED(CONFIG_ARM64_BTI_KERNEL) && system_supports_bti())

> but why do we care about CONFIG_ARM64_BTI_KERNEL for the vdso pages?

To enable BTI for the vDSO we need the vDSO to be built with BTI
annotations.  Currently the CFLAGS are the same for the vDSO and the
kernel, we could arrange to allow them to differ but since the most
likely reason why the user has userspace but not kernel BTI is that
their compiler isn't suitable it seemed like disproportionate effort
for a most likely small audience.

[-- Attachment #1.2: signature.asc --]
[-- 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

Re: [PATCH] arm64: bti: Fix support for userspace only BTI

[Will Deacon]

On Tue, May 12, 2020 at 12:05:02PM +0100, Mark Brown wrote:
> On Tue, May 12, 2020 at 11:39:09AM +0100, Will Deacon wrote:
> 
> > Might be worth a comment here in case somebody tries to "clean this up"
> > later on!
> 
> > > +#ifdef CONFIG_ARM64_BTI_KERNEL
> > >  #define PTE_MAYBE_GP		(system_supports_bti() ? PTE_GP : 0)
> > > +#else
> > > +#define PTE_MAYBE_GP		0
> > > +#endif
> 
> Sure.
> 
> > I also notice we have a funny check in the vdso code:
> 
> >   if (IS_ENABLED(CONFIG_ARM64_BTI_KERNEL) && system_supports_bti())
> 
> > but why do we care about CONFIG_ARM64_BTI_KERNEL for the vdso pages?
> 
> To enable BTI for the vDSO we need the vDSO to be built with BTI
> annotations.  Currently the CFLAGS are the same for the vDSO and the
> kernel, we could arrange to allow them to differ but since the most
> likely reason why the user has userspace but not kernel BTI is that
> their compiler isn't suitable it seemed like disproportionate effort
> for a most likely small audience.

I was thinking of using CC_HAS_BRANCH_PROT_PAC_RET_BTI instead, but I'd
forgotten about that GCC bug. Oh well.

Will

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

Re: [PATCH] arm64: bti: Fix support for userspace only BTI

[Dave Martin]

On Tue, May 12, 2020 at 12:05:02PM +0100, Mark Brown wrote:
> On Tue, May 12, 2020 at 11:39:09AM +0100, Will Deacon wrote:
> 
> > Might be worth a comment here in case somebody tries to "clean this up"
> > later on!
> 
> > > +#ifdef CONFIG_ARM64_BTI_KERNEL
> > >  #define PTE_MAYBE_GP		(system_supports_bti() ? PTE_GP : 0)
> > > +#else
> > > +#define PTE_MAYBE_GP		0
> > > +#endif
> 
> Sure.
> 
> > I also notice we have a funny check in the vdso code:
> 
> >   if (IS_ENABLED(CONFIG_ARM64_BTI_KERNEL) && system_supports_bti())
> 
> > but why do we care about CONFIG_ARM64_BTI_KERNEL for the vdso pages?
> 
> To enable BTI for the vDSO we need the vDSO to be built with BTI
> annotations.  Currently the CFLAGS are the same for the vDSO and the
> kernel, we could arrange to allow them to differ but since the most
> likely reason why the user has userspace but not kernel BTI is that
> their compiler isn't suitable it seemed like disproportionate effort
> for a most likely small audience.

The situation where the user is stuck on a binary vendor kernel built
using obsolete or wrongly configured tools doesn't sound that unlikely
to me.

When the vdso was all asm, we could easily have solved this by manually
annotating it ... I guess that's the price of progress, unless anyone
has a bright idea :/

Cheers
---Dave

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

Re: [PATCH] arm64: bti: Fix support for userspace only BTI

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1608 bytes --]

On Wed, May 13, 2020 at 03:46:25PM +0100, Dave Martin wrote:
> On Tue, May 12, 2020 at 12:05:02PM +0100, Mark Brown wrote:

> > To enable BTI for the vDSO we need the vDSO to be built with BTI
> > annotations.  Currently the CFLAGS are the same for the vDSO and the
> > kernel, we could arrange to allow them to differ but since the most
> > likely reason why the user has userspace but not kernel BTI is that
> > their compiler isn't suitable it seemed like disproportionate effort
> > for a most likely small audience.

> The situation where the user is stuck on a binary vendor kernel built
> using obsolete or wrongly configured tools doesn't sound that unlikely
> to me.

They'd need to be stuck on a binary vendor kernel which is sufficiently
new to have BTI support but using toolchains that are sufficiently old
to be unable to BTI the kernel (if their vendor went out of their way to
disable BTI then that's another issue).  My guess is that people will
tend to update major versions of these at relatively similar times, and
hopefully if people are going to the trouble of backporting BTI they'll
also grab the toolchain bits since it's substantially more useful if you
can compile userspace.

> When the vdso was all asm, we could easily have solved this by manually
> annotating it ... I guess that's the price of progress, unless anyone
> has a bright idea :/

Which is the other bit - if we don't trust the toolchain to build the
kernel then we probably ought to have concerns about the C bits of the
vDSO as well.  Even if they're fine now some future vDSO change could
cause this to explode.

[-- Attachment #1.2: signature.asc --]
[-- 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

Re: [PATCH] arm64: bti: Fix support for userspace only BTI

[Dave Martin]

On Wed, May 13, 2020 at 04:08:22PM +0100, Mark Brown wrote:
> On Wed, May 13, 2020 at 03:46:25PM +0100, Dave Martin wrote:
> > On Tue, May 12, 2020 at 12:05:02PM +0100, Mark Brown wrote:
> 
> > > To enable BTI for the vDSO we need the vDSO to be built with BTI
> > > annotations.  Currently the CFLAGS are the same for the vDSO and the
> > > kernel, we could arrange to allow them to differ but since the most
> > > likely reason why the user has userspace but not kernel BTI is that
> > > their compiler isn't suitable it seemed like disproportionate effort
> > > for a most likely small audience.
> 
> > The situation where the user is stuck on a binary vendor kernel built
> > using obsolete or wrongly configured tools doesn't sound that unlikely
> > to me.
> 
> They'd need to be stuck on a binary vendor kernel which is sufficiently
> new to have BTI support but using toolchains that are sufficiently old
> to be unable to BTI the kernel (if their vendor went out of their way to
> disable BTI then that's another issue).  My guess is that people will
> tend to update major versions of these at relatively similar times, and
> hopefully if people are going to the trouble of backporting BTI they'll
> also grab the toolchain bits since it's substantially more useful if you
> can compile userspace.

Yeah, just playing devil's advocate.

> > When the vdso was all asm, we could easily have solved this by manually
> > annotating it ... I guess that's the price of progress, unless anyone
> > has a bright idea :/
> 
> Which is the other bit - if we don't trust the toolchain to build the
> kernel then we probably ought to have concerns about the C bits of the
> vDSO as well.  Even if they're fine now some future vDSO change could
> cause this to explode.

Exactly.  So I guess you're right: no kernel BTI, no vDSO BTI.  It's
a bit annoying, but there it is.

I can't argue that the best fix for broken tools isn't to fix the
tools ;)

Cheers
---Dave

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