Re: [PATCH] PR_SPEC_DISABLE_NOEXEC support for arm64.

From: Anthony Steinhauser
Date: Sun Sep 20 2020 - 06:25:44 EST


> As a heads up: I'm currently reworking most of this, and hope to post
> something within the next two weeks.

Sure. Let me know whether you want to implement the
PR_SPEC_DISABLE_NOEXEC support directly or whether this patch would be
relevant even after your rework.
>
> > diff --git a/arch/arm64/include/asm/ssbd.h b/arch/arm64/include/asm/ssbd.h
> > new file mode 100644
> > index 000000000000..68c716dc5811
> > --- /dev/null
> > +++ b/arch/arm64/include/asm/ssbd.h
...
> > +}
>
> I'd prefer to keep these where they are and have an out-of-line call if
> necessary. We should try to keep the SSBD stuff in one place.

OK.
>
> > +
> > +#endif /* __ASM_SSBD_H */
> > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> > index 6089638c7d43..ad3c67c86c4c 100644
> > --- a/arch/arm64/kernel/process.c
> > +++ b/arch/arm64/kernel/process.c
> > @@ -54,6 +54,7 @@
> > #include <asm/mmu_context.h>
> > #include <asm/processor.h>
> > #include <asm/pointer_auth.h>
> > +#include <asm/ssbd.h>
> > #include <asm/stacktrace.h>
> >
> > #if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK)
> > @@ -588,6 +589,18 @@ void arch_setup_new_exec(void)
> > current->mm->context.flags = is_compat_task() ? MMCF_AARCH32 : 0;
> >
> > ptrauth_thread_init_user(current);
> > +
> > + /*
> > + * Don't inherit TIF_SSBD across exec boundary when
> > + * PR_SPEC_DISABLE_NOEXEC is used.
> > + */
> > + if (test_thread_flag(TIF_SSBD) &&
> > + task_spec_ssb_noexec(current)) {
> > + clear_thread_flag(TIF_SSBD);
> > + task_clear_spec_ssb_disable(current);
> > + task_clear_spec_ssb_noexec(current);
> > + ssbd_ssbs_enable(current);
> > + }
>
> How is this supposed to work with CPUs that expose SSBS directly to
> userspace? I suppose we should be using PR_SPEC_DISABLE_NOEXEC to decide
> what we set the SSBS bit to on exec, but the logic here requires TIF_SSBD
> to be set and so won't trigger afaict.
>

You're right. The SSBS support is incomplete. I guess
"test_thread_flag(TIF_SSBD)" can be replaced just with
"arm64_get_ssbd_state() == ARM64_SSBD_KERNEL".
Thanks,
Anthony