Message-ID: <f7ffadc4-16ed-4d11-8baf-ea2887305d18@linuxfoundation.org>
Date: Mon, 5 Aug 2024 15:29:45 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Mark Brown <broonie@...nel.org>,
 "Rick P. Edgecombe" <rick.p.edgecombe@...el.com>,
 Deepak Gupta <debug@...osinc.com>, Szabolcs Nagy <Szabolcs.Nagy@....com>,
 "H.J. Lu" <hjl.tools@...il.com>, Florian Weimer <fweimer@...hat.com>,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
 Peter Zijlstra <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>,
 Vincent Guittot <vincent.guittot@...aro.org>,
 Dietmar Eggemann <dietmar.eggemann@....com>,
 Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>,
 Mel Gorman <mgorman@...e.de>, Daniel Bristot de Oliveira
 <bristot@...hat.com>, Valentin Schneider <vschneid@...hat.com>,
 Christian Brauner <brauner@...nel.org>, Shuah Khan <shuah@...nel.org>
Cc: linux-kernel@...r.kernel.org, Catalin Marinas <catalin.marinas@....com>,
 Will Deacon <will@...nel.org>, jannh@...gle.com,
 linux-kselftest@...r.kernel.org, linux-api@...r.kernel.org,
 Kees Cook <kees@...nel.org>, David Hildenbrand <david@...hat.com>,
 Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH RFT v7 0/9] fork: Support shadow stacks in clone3()

On 7/31/24 06:14, Mark Brown wrote:
> The kernel has recently added support for shadow stacks, currently
> x86 only using their CET feature but both arm64 and RISC-V have
> equivalent features (GCS and Zicfiss respectively), I am actively
> working on GCS[1].  With shadow stacks the hardware maintains an
> additional stack containing only the return addresses for branch
> instructions which is not generally writeable by userspace and ensures
> that any returns are to the recorded addresses.  This provides some
> protection against ROP attacks and making it easier to collect call
> stacks.  These shadow stacks are allocated in the address space of the
> userspace process.
> 
> Our API for shadow stacks does not currently offer userspace any
> flexiblity for managing the allocation of shadow stacks for newly
> created threads, instead the kernel allocates a new shadow stack with
> the same size as the normal stack whenever a thread is created with the
> feature enabled.  The stacks allocated in this way are freed by the
> kernel when the thread exits or shadow stacks are disabled for the
> thread.  This lack of flexibility and control isn't ideal, in the vast
> majority of cases the shadow stack will be over allocated and the
> implicit allocation and deallocation is not consistent with other
> interfaces.  As far as I can tell the interface is done in this manner
> mainly because the shadow stack patches were in development since before
> clone3() was implemented.
> 
> Since clone3() is readily extensible let's add support for specifying a
> shadow stack when creating a new thread or process in a similar manner
> to how the normal stack is specified, keeping the current implicit
> allocation behaviour if one is not specified either with clone3() or
> through the use of clone().  The user must provide a shadow stack
> address and size, this must point to memory mapped for use as a shadow
> stackby map_shadow_stack() with a shadow stack token at the top of the
> stack.
> 
> Please note that the x86 portions of this code are build tested only, I
> don't appear to have a system that can run CET avaible to me, I have
> done testing with an integration into my pending work for GCS.  There is
> some possibility that the arm64 implementation may require the use of
> clone3() and explicit userspace allocation of shadow stacks, this is
> still under discussion.
> 
> Please further note that the token consumption done by clone3() is not
> currently implemented in an atomic fashion, Rick indicated that he would
> look into fixing this if people are OK with the implementation.
> 
> A new architecture feature Kconfig option for shadow stacks is added as
> here, this was suggested as part of the review comments for the arm64
> GCS series and since we need to detect if shadow stacks are supported it
> seemed sensible to roll it in here.
> 
> [1] https://lore.kernel.org/r/20231009-arm64-gcs-v6-0-78e55deaa4dd@kernel.org/
> 
> Signed-off-by: Mark Brown <broonie@...nel.org>
> ---
> Changes in v7:
> - Rebase onto v6.11-rc1.
> - Typo fixes.
> - Link to v6: https://lore.kernel.org/r/20240623-clone3-shadow-stack-v6-0-9ee7783b1fb9@kernel.org
> 
> Changes in v6:
> - Rebase onto v6.10-rc3.
> - Ensure we don't try to free the parent shadow stack in error paths of
>    x86 arch code.
> - Spelling fixes in userspace API document.
> - Additional cleanups and improvements to the clone3() tests to support
>    the shadow stack tests.
> - Link to v5: https://lore.kernel.org/r/20240203-clone3-shadow-stack-v5-0-322c69598e4b@kernel.org
> 
> Changes in v5:
> - Rebase onto v6.8-rc2.
> - Rework ABI to have the user allocate the shadow stack memory with
>    map_shadow_stack() and a token.
> - Force inlining of the x86 shadow stack enablement.
> - Move shadow stack enablement out into a shared header for reuse by
>    other tests.
> - Link to v4: https://lore.kernel.org/r/20231128-clone3-shadow-stack-v4-0-8b28ffe4f676@kernel.org
> 
> Changes in v4:
> - Formatting changes.
> - Use a define for minimum shadow stack size and move some basic
>    validation to fork.c.
> - Link to v3: https://lore.kernel.org/r/20231120-clone3-shadow-stack-v3-0-a7b8ed3e2acc@kernel.org
> 
> Changes in v3:
> - Rebase onto v6.7-rc2.
> - Remove stale shadow_stack in internal kargs.
> - If a shadow stack is specified unconditionally use it regardless of
>    CLONE_ parameters.
> - Force enable shadow stacks in the selftest.
> - Update changelogs for RISC-V feature rename.
> - Link to v2: https://lore.kernel.org/r/20231114-clone3-shadow-stack-v2-0-b613f8681155@kernel.org
> 
> Changes in v2:
> - Rebase onto v6.7-rc1.
> - Remove ability to provide preallocated shadow stack, just specify the
>    desired size.
> - Link to v1: https://lore.kernel.org/r/20231023-clone3-shadow-stack-v1-0-d867d0b5d4d0@kernel.org
> 
> ---
> Mark Brown (9):
>        Documentation: userspace-api: Add shadow stack API documentation
>        selftests: Provide helper header for shadow stack testing
>        mm: Introduce ARCH_HAS_USER_SHADOW_STACK
>        fork: Add shadow stack support to clone3()
>        selftests/clone3: Remove redundant flushes of output streams
>        selftests/clone3: Factor more of main loop into test_clone3()
>        selftests/clone3: Explicitly handle child exits due to signals
>        selftests/clone3: Allow tests to flag if -E2BIG is a valid error code
>        selftests/clone3: Test shadow stack support
> 
>   Documentation/userspace-api/index.rst             |   1 +
>   Documentation/userspace-api/shadow_stack.rst      |  41 ++++
>   arch/x86/Kconfig                                  |   1 +
>   arch/x86/include/asm/shstk.h                      |  11 +-
>   arch/x86/kernel/process.c                         |   2 +-
>   arch/x86/kernel/shstk.c                           | 104 +++++++---
>   fs/proc/task_mmu.c                                |   2 +-
>   include/linux/mm.h                                |   2 +-
>   include/linux/sched/task.h                        |  13 ++
>   include/uapi/linux/sched.h                        |  13 +-
>   kernel/fork.c                                     |  76 ++++++--
>   mm/Kconfig                                        |   6 +
>   tools/testing/selftests/clone3/clone3.c           | 224 ++++++++++++++++++----
>   tools/testing/selftests/clone3/clone3_selftests.h |  40 +++-
>   tools/testing/selftests/ksft_shstk.h              |  63 ++++++
>   15 files changed, 511 insertions(+), 88 deletions(-)
> ---
> base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
> change-id: 20231019-clone3-shadow-stack-15d40d2bf536
> 
> Best regards,


For selftests:

Acked-by: Shuah Khan <skhan@...uxfoundation.org>

thanks,
-- Shuah

Powered by blists - more mailing lists