linux-kernel - Re: [PATCH] PR_SPEC_DISABLE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAN_oZf29J-DMF04EhQf=29CzV5zrPT+L6HchZcG-CDt=MjFf4A@mail.gmail.com>
Date:   Sun, 20 Sep 2020 03:25:23 -0700
From:   Anthony Steinhauser <asteinhauser@...gle.com>
To:     Will Deacon <will@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>, catalin.marinas@....com,
        maz@...nel.org
Subject: Re: [PATCH] PR_SPEC_DISABLE_NOEXEC support for arm64.

> 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