* Re: [PATCH RFT v4 2/5] fork: Add shadow stack support to clone3()
From: Edgecombe, Rick P @ 2023-12-05 0:26 UTC (permalink / raw)
To: dietmar.eggemann@arm.com, broonie@kernel.org,
Szabolcs.Nagy@arm.com, brauner@kernel.org,
dave.hansen@linux.intel.com, debug@rivosinc.com, mgorman@suse.de,
vincent.guittot@linaro.org, fweimer@redhat.com, mingo@redhat.com,
rostedt@goodmis.org, hjl.tools@gmail.com, tglx@linutronix.de,
vschneid@redhat.com, shuah@kernel.org, bristot@redhat.com,
hpa@zytor.com, peterz@infradead.org, bp@alien8.de,
bsegall@google.com, x86@kernel.org, juri.lelli@redhat.com
Cc: keescook@chromium.org, jannh@google.com,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
catalin.marinas@arm.com, linux-api@vger.kernel.org,
will@kernel.org
In-Reply-To: <20231128-clone3-shadow-stack-v4-2-8b28ffe4f676@kernel.org>
On Tue, 2023-11-28 at 18:22 +0000, Mark Brown wrote:
> -unsigned long shstk_alloc_thread_stack(struct task_struct *tsk,
> unsigned long clone_flags,
> - unsigned long stack_size)
> +unsigned long shstk_alloc_thread_stack(struct task_struct *tsk,
> + const struct kernel_clone_args
> *args)
> {
> struct thread_shstk *shstk = &tsk->thread.shstk;
> + unsigned long clone_flags = args->flags;
> unsigned long addr, size;
>
> /*
> * If shadow stack is not enabled on the new thread, skip any
> - * switch to a new shadow stack.
> + * implicit switch to a new shadow stack and reject attempts
> to
> + * explciitly specify one.
> */
> - if (!features_enabled(ARCH_SHSTK_SHSTK))
> - return 0;
> + if (!features_enabled(ARCH_SHSTK_SHSTK)) {
> + if (args->shadow_stack_size)
> + return (unsigned long)ERR_PTR(-EINVAL);
>
> - /*
> - * For CLONE_VFORK the child will share the parents shadow
> stack.
> - * Make sure to clear the internal tracking of the thread
> shadow
> - * stack so the freeing logic run for child knows to leave it
> alone.
> - */
> - if (clone_flags & CLONE_VFORK) {
> - shstk->base = 0;
> - shstk->size = 0;
> return 0;
> }
>
> /*
> - * For !CLONE_VM the child will use a copy of the parents
> shadow
> - * stack.
> + * If the user specified a shadow stack then do some basic
> + * validation and use it, otherwise fall back to a default
> + * shadow stack size if the clone_flags don't indicate an
> + * allocation is unneeded.
> */
> - if (!(clone_flags & CLONE_VM))
> - return 0;
> + if (args->shadow_stack_size) {
> + size = args->shadow_stack_size;
> + } else {
> + /*
> + * For CLONE_VFORK the child will share the parents
> + * shadow stack. Make sure to clear the internal
> + * tracking of the thread shadow stack so the freeing
> + * logic run for child knows to leave it alone.
> + */
> + if (clone_flags & CLONE_VFORK) {
> + shstk->base = 0;
> + shstk->size = 0;
> + return 0;
> + }
> +
> + /*
> + * For !CLONE_VM the child will use a copy of the
> + * parents shadow stack.
> + */
> + if (!(clone_flags & CLONE_VM))
> + return 0;
> +
> + size = args->stack_size;
> +
> + }
>
> - size = adjust_shstk_size(stack_size);
> + size = adjust_shstk_size(size);
> addr = alloc_shstk(0, size, 0, false);
Hmm. I didn't test this, but in the copy_process(), copy_mm() happens
before this point. So the shadow stack would get mapped in current's MM
(i.e. the parent). So in the !CLONE_VM case with shadow_stack_size!=0
the SSP in the child will be updated to an area that is not mapped in
the child. I think we need to pass tsk->mm into alloc_shstk(). But such
an exotic clone usage does give me pause, regarding whether all of this
is premature.
Otherwise it looked ok from the x86/shstk perspective.
> if (IS_ERR_VALUE(addr))
> return addr;
^ permalink raw reply
* Re: [PATCH RFT v4 5/5] kselftest/clone3: Test shadow stack support
From: Edgecombe, Rick P @ 2023-12-05 0:10 UTC (permalink / raw)
To: dietmar.eggemann@arm.com, broonie@kernel.org,
Szabolcs.Nagy@arm.com, brauner@kernel.org,
dave.hansen@linux.intel.com, debug@rivosinc.com, mgorman@suse.de,
vincent.guittot@linaro.org, fweimer@redhat.com, mingo@redhat.com,
rostedt@goodmis.org, hjl.tools@gmail.com, tglx@linutronix.de,
vschneid@redhat.com, shuah@kernel.org, bristot@redhat.com,
hpa@zytor.com, peterz@infradead.org, bp@alien8.de,
bsegall@google.com, x86@kernel.org, juri.lelli@redhat.com
Cc: keescook@chromium.org, jannh@google.com,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
catalin.marinas@arm.com, linux-api@vger.kernel.org,
will@kernel.org
In-Reply-To: <20231128-clone3-shadow-stack-v4-5-8b28ffe4f676@kernel.org>
On Tue, 2023-11-28 at 18:22 +0000, Mark Brown wrote:
> +
> +#define ENABLE_SHADOW_STACK
> +static inline void enable_shadow_stack(void)
> +{
> + int ret = ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK);
> + if (ret == 0)
> + shadow_stack_enabled = true;
> +}
> +
> +#endif
> +
> +#ifndef ENABLE_SHADOW_STACK
> +static void enable_shadow_stack(void)
> +{
> +}
> +#endif
Without this diff, the test crashed for me on a shadow stack system:
diff --git a/tools/testing/selftests/clone3/clone3.c
b/tools/testing/selftests/clone3/clone3.c
index dbe52582573c..3236d97ed261 100644
--- a/tools/testing/selftests/clone3/clone3.c
+++ b/tools/testing/selftests/clone3/clone3.c
@@ -423,7 +423,7 @@ static const struct test tests[] = {
})
#define ENABLE_SHADOW_STACK
-static inline void enable_shadow_stack(void)
+static inline __attribute__((always_inline)) void
enable_shadow_stack(void)
{
int ret = ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK);
if (ret == 0)
The fix works by making sure control flow never returns to before the
point shadow stack was enabled. Otherwise it will underflow the shadow
stack.
But I wonder if the clone3 test should get its shadow stack enabled the
conventional elf bit way. So if it's all there (HW, kernel, glibc) then
the test will run with shadow stack. Otherwise the test will run
without shadow stack.
The other reason is that the shadow stack test in the x86 selftest
manual enabling is designed to work without a shadow stack enabled
glibc and has to be specially crafted to work around the missing
support. I'm not sure the more generic selftests should have to know
how to do this. So what about something like this instead:
diff --git a/tools/testing/selftests/clone3/Makefile
b/tools/testing/selftests/clone3/Makefile
index 84832c369a2e..792bc9685c82 100644
--- a/tools/testing/selftests/clone3/Makefile
+++ b/tools/testing/selftests/clone3/Makefile
@@ -2,6 +2,13 @@
CFLAGS += -g -std=gnu99 $(KHDR_INCLUDES)
LDLIBS += -lcap
+ifeq ($(shell uname -m),x86_64)
+CAN_BUILD_WITH_SHSTK := $(shell ../x86/check_cc.sh gcc
../x86/trivial_program.c -mshstk -fcf-protection)
+ifeq ($(CAN_BUILD_WITH_SHSTK),1)
+CFLAGS += -mshstk -fcf-protection=return
+endif
+endif
+
TEST_GEN_PROGS := clone3 clone3_clear_sighand clone3_set_tid \
clone3_cap_checkpoint_restore
diff --git a/tools/testing/selftests/clone3/clone3.c
b/tools/testing/selftests/clone3/clone3.c
index dbe52582573c..eff5e8d5a5a6 100644
--- a/tools/testing/selftests/clone3/clone3.c
+++ b/tools/testing/selftests/clone3/clone3.c
@@ -23,7 +23,6 @@
#include "clone3_selftests.h"
static bool shadow_stack_enabled;
-static bool shadow_stack_supported;
static size_t max_supported_args_size;
enum test_mode {
@@ -50,36 +49,6 @@ struct test {
filter_function filter;
};
-#ifndef __NR_map_shadow_stack
-#define __NR_map_shadow_stack 453
-#endif
-
-/*
- * We check for shadow stack support by attempting to use
- * map_shadow_stack() since features may have been locked by the
- * dynamic linker resulting in spurious errors when we attempt to
- * enable on startup. We warn if the enable failed.
- */
-static void test_shadow_stack_supported(void)
-{
- long shadow_stack;
-
- shadow_stack = syscall(__NR_map_shadow_stack, 0, getpagesize(),
0);
- if (shadow_stack == -1) {
- ksft_print_msg("map_shadow_stack() not supported\n");
- } else if ((void *)shadow_stack == MAP_FAILED) {
- ksft_print_msg("Failed to map shadow stack\n");
- } else {
- ksft_print_msg("Shadow stack supportd\n");
- shadow_stack_supported = true;
-
- if (!shadow_stack_enabled)
- ksft_print_msg("Mapped but did not enable
shadow stack\n");
-
- munmap((void *)shadow_stack, getpagesize());
- }
-}
-
static int call_clone3(uint64_t flags, size_t size, enum test_mode
test_mode)
{
struct __clone_args args = {
@@ -220,7 +189,7 @@ static bool no_timenamespace(void)
static bool have_shadow_stack(void)
{
- if (shadow_stack_supported) {
+ if (shadow_stack_enabled) {
ksft_print_msg("Shadow stack supported\n");
return true;
}
@@ -230,7 +199,7 @@ static bool have_shadow_stack(void)
static bool no_shadow_stack(void)
{
- if (!shadow_stack_supported) {
+ if (!shadow_stack_enabled) {
ksft_print_msg("Shadow stack not supported\n");
return true;
}
@@ -402,38 +371,18 @@ static const struct test tests[] = {
};
#ifdef __x86_64__
-#define ARCH_SHSTK_ENABLE 0x5001
+#define ARCH_SHSTK_STATUS 0x5005
#define ARCH_SHSTK_SHSTK (1ULL << 0)
-#define ARCH_PRCTL(arg1, arg2) \
-({ \
- long _ret; \
- register long _num asm("eax") = __NR_arch_prctl; \
- register long _arg1 asm("rdi") = (long)(arg1); \
- register long _arg2 asm("rsi") = (long)(arg2); \
- \
- asm volatile ( \
- "syscall\n" \
- : "=a"(_ret) \
- : "r"(_arg1), "r"(_arg2), \
- "0"(_num) \
- : "rcx", "r11", "memory", "cc" \
- ); \
- _ret; \
-})
-
-#define ENABLE_SHADOW_STACK
-static inline void enable_shadow_stack(void)
+static inline __attribute__((always_inline)) void
check_shadow_stack(void)
{
- int ret = ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK);
- if (ret == 0)
- shadow_stack_enabled = true;
+ unsigned long status = 0;
+
+ syscall(SYS_arch_prctl, ARCH_SHSTK_STATUS, &status);
+ shadow_stack_enabled = status & ARCH_SHSTK_SHSTK;
}
-
-#endif
-
-#ifndef ENABLE_SHADOW_STACK
-static void enable_shadow_stack(void)
+#else /* __x86_64__ */
+static void check_shadow_stack(void)
{
}
#endif
@@ -443,12 +392,11 @@ int main(int argc, char *argv[])
size_t size;
int i;
- enable_shadow_stack();
+ check_shadow_stack();
ksft_print_header();
ksft_set_plan(ARRAY_SIZE(tests));
test_clone3_supported();
- test_shadow_stack_supported();
for (i = 0; i < ARRAY_SIZE(tests); i++)
test_clone3(&tests[i]);
^ permalink raw reply related
* Re: [EXT] Re: [RFC PATCH 0/2] Node migration between memory tiers
From: Srinivasulu Thanneeru @ 2023-12-04 19:56 UTC (permalink / raw)
To: Michal Hocko
Cc: aneesh.kumar, linux-cxl, linux-mm, dan.j.williams, hannes,
hasanalmaruf, haowang3, ying.huang, gregory.price, tj,
hezhongkun.hzk, fvdl, john, emirakhur, vtavarespetr,
Ravis.OpenSrc, Jonathan.Cameron, linux-kernel, linux-api
In-Reply-To: <ZW3zl2Fke5FtQCv3@tiehlicka>
On 12/4/2023 9:13 PM, Michal Hocko wrote:
> CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
>
>
> On Fri 01-12-23 03:34:20, sthanneeru.opensrc@micron.com wrote:
>> From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
>>
>> The memory tiers feature allows nodes with similar memory types
>> or performance characteristics to be grouped together in a
>> memory tier. However, there is currently no provision for
>> moving a node from one tier to another on demand.
>
> Could you expand on why this is really needed/necessary? What is the
> actual usecase?
Hi Michal Hock,
Following two use-cases we have observed.
1. It is not accurate to group similar memory types in the same tier,
because even similar memory types may have different speed grades.
2. Some systems boots up with CXL devices and DRAM on the same
memory-tier, we need a way to move the CXL nodes to the correct tier
from the user space.
Regards,
Srini
> --
> Michal Hocko
> SUSE Labs
^ permalink raw reply
* Re: [PATCH 17/21] fs: xfs: iomap atomic write support
From: John Garry @ 2023-12-04 18:06 UTC (permalink / raw)
To: Christoph Hellwig
Cc: axboe, kbusch, sagi, jejb, martin.petersen, djwong, viro, brauner,
chandan.babu, dchinner, linux-block, linux-kernel, linux-nvme,
linux-xfs, linux-fsdevel, tytso, jbongio, linux-api
In-Reply-To: <20231204153912.GA3580@lst.de>
On 04/12/2023 15:39, Christoph Hellwig wrote:
>> So what would you propose as the next step? Would it to be first achieve
>> atomic write support for XFS with HW support + CoW to ensure contiguous
>> extents (and without XFS forcealign)?
> I think the very first priority is just block device support without
> any fs enablement. We just need to make sure the API isn't too limited
> for additional use cases.
Sounds ok
^ permalink raw reply
* [PATCH] ELF: supply userspace with available page shifts (AT_PAGE_SHIFT_LIST)
From: Alexey Dobriyan @ 2023-12-04 17:18 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel, linux-arch, linux-api, x86
Report available page shifts in arch independent manner, so that poor
userspace developers won't have to parse /proc/cpuinfo hunting for
arch-specific flag strings:
unsigned long val = getauxval(AT_PAGE_SHIFT_LIST);
while (val && (val & 255) != 30) {
val >>= 8;
}
if (val) {
page_size_1gib = true;
} else {
page_size_1gib = false;
}
Note!
This is strictly for userspace, if some page size is shutdown due
to kernel command line option or CPU bug workaround, than is must not
be reported in aux vector!
x86_64 machine with 1 GiB pages:
$ hexdump -C /proc/self/auxv
00000030 06 00 00 00 00 00 00 00 00 10 00 00 00 00 00 00
00000040 1d 00 00 00 00 00 00 00 0c 15 1e 00 00 00 00 00
x86_64 machine with 2MiB pages only:
$ hexdump -C /proc/self/auxv
00000030 06 00 00 00 00 00 00 00 00 10 00 00 00 00 00 00
00000040 1d 00 00 00 00 00 00 00 0c 15 00 00 00 00 00 00
AT_PAGESZ is always 4096 which is not much information.
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
arch/x86/include/asm/elf.h | 13 +++++++++++++
fs/binfmt_elf.c | 3 +++
include/uapi/linux/auxvec.h | 17 +++++++++++++++++
3 files changed, 33 insertions(+)
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -358,6 +358,19 @@ else if (IS_ENABLED(CONFIG_IA32_EMULATION)) \
#define COMPAT_ELF_ET_DYN_BASE (TASK_UNMAPPED_BASE + 0x1000000)
+#define ARCH_AT_PAGE_SHIFT_LIST \
+ do { \
+ u32 val = 12; \
+ int s = 0; \
+ if (boot_cpu_has(X86_FEATURE_PSE)) { \
+ val |= 21 << (s += 8); \
+ } \
+ if (boot_cpu_has(X86_FEATURE_GBPAGES)) { \
+ val |= 30 << (s += 8); \
+ } \
+ NEW_AUX_ENT(AT_PAGE_SHIFT_LIST, val); \
+ } while (0)
+
#endif /* !CONFIG_X86_32 */
#define VDSO_CURRENT_BASE ((unsigned long)current->mm->context.vdso)
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -240,6 +240,9 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec,
#endif
NEW_AUX_ENT(AT_HWCAP, ELF_HWCAP);
NEW_AUX_ENT(AT_PAGESZ, ELF_EXEC_PAGESIZE);
+#ifdef ARCH_AT_PAGE_SHIFT_LIST
+ ARCH_AT_PAGE_SHIFT_LIST;
+#endif
NEW_AUX_ENT(AT_CLKTCK, CLOCKS_PER_SEC);
NEW_AUX_ENT(AT_PHDR, phdr_addr);
NEW_AUX_ENT(AT_PHENT, sizeof(struct elf_phdr));
--- a/include/uapi/linux/auxvec.h
+++ b/include/uapi/linux/auxvec.h
@@ -33,6 +33,23 @@
#define AT_RSEQ_FEATURE_SIZE 27 /* rseq supported feature size */
#define AT_RSEQ_ALIGN 28 /* rseq allocation alignment */
+/*
+ * Page sizes available for mmap(2) encoded as 1 page shift per byte in
+ * increasing order.
+ *
+ * Thus 32-bit systems get 4 shifts, 64-bit systems get 8 shifts tops.
+ *
+ * Example:
+ * x86_64 system with "pdpe1gb" reports 4 KiB, 2 MiB and 1 GiB page support.
+ *
+ * $ hexdump -C /proc/self/auxv
+ * 00000030 06 00 00 00 00 00 00 00 00 10 00 00 00 00 00 00
+ * 00000040 1d 00 00 00 00 00 00 00 0c 15 1e 00 00 00 00 00
+ *
+ * For 2^256 hugepage support please contact your Universe sales representative.
+ */
+#define AT_PAGE_SHIFT_LIST 29
+
#define AT_EXECFN 31 /* filename of program */
#ifndef AT_MINSIGSTKSZ
^ permalink raw reply
* Re: [RFC PATCH 0/2] Node migration between memory tiers
From: Michal Hocko @ 2023-12-04 15:43 UTC (permalink / raw)
To: sthanneeru.opensrc
Cc: aneesh.kumar, linux-cxl, linux-mm, dan.j.williams, hannes,
hasanalmaruf, haowang3, ying.huang, gregory.price, tj,
hezhongkun.hzk, fvdl, john, emirakhur, vtavarespetr,
Ravis.OpenSrc, Jonathan.Cameron, linux-kernel, linux-api
In-Reply-To: <20231130220422.2033-1-sthanneeru.opensrc@micron.com>
On Fri 01-12-23 03:34:20, sthanneeru.opensrc@micron.com wrote:
> From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
>
> The memory tiers feature allows nodes with similar memory types
> or performance characteristics to be grouped together in a
> memory tier. However, there is currently no provision for
> moving a node from one tier to another on demand.
Could you expand on why this is really needed/necessary? What is the
actual usecase?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH 17/21] fs: xfs: iomap atomic write support
From: Christoph Hellwig @ 2023-12-04 15:39 UTC (permalink / raw)
To: John Garry
Cc: Christoph Hellwig, axboe, kbusch, sagi, jejb, martin.petersen,
djwong, viro, brauner, chandan.babu, dchinner, linux-block,
linux-kernel, linux-nvme, linux-xfs, linux-fsdevel, tytso,
jbongio, linux-api
In-Reply-To: <a87d48a7-f2a8-40ae-8d9b-e4534ccc29b1@oracle.com>
On Mon, Dec 04, 2023 at 03:19:15PM +0000, John Garry wrote:
> On 04/12/2023 13:45, Christoph Hellwig wrote:
>> On Tue, Nov 28, 2023 at 05:42:10PM +0000, John Garry wrote:
>>> ok, fine, it would not be required for XFS with CoW. Some concerns still:
>>> a. device atomic write boundary, if any
>>> b. other FSes which do not have CoW support. ext4 is already being used for
>>> "atomic writes" in the field - see dubious amazon torn-write prevention.
>>
>> What is the 'dubious amazon torn-write prevention'?
>
> https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/storage-twp.html
>
> AFAICS, this is without any kernel changes, so no guarantee of unwanted
> splitting or merging of bios.
>
> Anyway, there will still be !CoW FSes which people want to support.
Ugg, so they badly reimplement NVMe atomic write support and use it
without software stack enablement. Calling it dubious is way to
gentle..
>> Relying just on the hardware seems very limited, especially as there is
>> plenty of hardware that won't guarantee anything larger than 4k, and
>> plenty of NVMe hardware without has some other small limit like 32k
>> because it doesn't support multiple atomicy mode.
>
> So what would you propose as the next step? Would it to be first achieve
> atomic write support for XFS with HW support + CoW to ensure contiguous
> extents (and without XFS forcealign)?
I think the very first priority is just block device support without
any fs enablement. We just need to make sure the API isn't too limited
for additional use cases.
> Ignoring FSes, then how is this supposed to work for block devices? We just
> always need HW support, right?
Yes.
^ permalink raw reply
* Re: [PATCH 17/21] fs: xfs: iomap atomic write support
From: John Garry @ 2023-12-04 15:19 UTC (permalink / raw)
To: Christoph Hellwig
Cc: axboe, kbusch, sagi, jejb, martin.petersen, djwong, viro, brauner,
chandan.babu, dchinner, linux-block, linux-kernel, linux-nvme,
linux-xfs, linux-fsdevel, tytso, jbongio, linux-api
In-Reply-To: <20231204134509.GA25834@lst.de>
On 04/12/2023 13:45, Christoph Hellwig wrote:
> On Tue, Nov 28, 2023 at 05:42:10PM +0000, John Garry wrote:
>> ok, fine, it would not be required for XFS with CoW. Some concerns still:
>> a. device atomic write boundary, if any
>> b. other FSes which do not have CoW support. ext4 is already being used for
>> "atomic writes" in the field - see dubious amazon torn-write prevention.
>
> What is the 'dubious amazon torn-write prevention'?
https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/storage-twp.html
AFAICS, this is without any kernel changes, so no guarantee of unwanted
splitting or merging of bios.
Anyway, there will still be !CoW FSes which people want to support.
>
>> About b., we could add the pow-of-2 and file offset alignment requirement
>> for other FSes, but then need to add some method to advertise that
>> restriction.
>
> We really need a better way to communicate I/O limitations anyway.
> Something like XFS_IOC_DIOINFO on steroids.
>
>> Sure, but to me it is a concern that we have 2x paths to make robust a.
>> offload via hw, which may involve CoW b. no HW support, i.e. CoW always
>
> Relying just on the hardware seems very limited, especially as there is
> plenty of hardware that won't guarantee anything larger than 4k, and
> plenty of NVMe hardware without has some other small limit like 32k
> because it doesn't support multiple atomicy mode.
So what would you propose as the next step? Would it to be first achieve
atomic write support for XFS with HW support + CoW to ensure contiguous
extents (and without XFS forcealign)?
>
>> And for no HW support, if we don't follow the O_ATOMIC model of committing
>> nothing until a SYNC is issued, would we allocate, write, and later free a
>> new extent for each write, right?
>
> Yes. Then again if you do data journalling you do that anyway, and as
> one little project I'm doing right now shows that data journling is
> often the fastest thing we can do for very small writes.
Ignoring FSes, then how is this supposed to work for block devices? We
just always need HW support, right?
Thanks,
John
^ permalink raw reply
* Re: [PATCH 17/21] fs: xfs: iomap atomic write support
From: Christoph Hellwig @ 2023-12-04 13:45 UTC (permalink / raw)
To: John Garry
Cc: Christoph Hellwig, axboe, kbusch, sagi, jejb, martin.petersen,
djwong, viro, brauner, chandan.babu, dchinner, linux-block,
linux-kernel, linux-nvme, linux-xfs, linux-fsdevel, tytso,
jbongio, linux-api
In-Reply-To: <e4fb6875-e552-45aa-b193-58f15d9a786c@oracle.com>
On Tue, Nov 28, 2023 at 05:42:10PM +0000, John Garry wrote:
> ok, fine, it would not be required for XFS with CoW. Some concerns still:
> a. device atomic write boundary, if any
> b. other FSes which do not have CoW support. ext4 is already being used for
> "atomic writes" in the field - see dubious amazon torn-write prevention.
What is the 'dubious amazon torn-write prevention'?
> About b., we could add the pow-of-2 and file offset alignment requirement
> for other FSes, but then need to add some method to advertise that
> restriction.
We really need a better way to communicate I/O limitations anyway.
Something like XFS_IOC_DIOINFO on steroids.
> Sure, but to me it is a concern that we have 2x paths to make robust a.
> offload via hw, which may involve CoW b. no HW support, i.e. CoW always
Relying just on the hardware seems very limited, especially as there is
plenty of hardware that won't guarantee anything larger than 4k, and
plenty of NVMe hardware without has some other small limit like 32k
because it doesn't support multiple atomicy mode.
> And for no HW support, if we don't follow the O_ATOMIC model of committing
> nothing until a SYNC is issued, would we allocate, write, and later free a
> new extent for each write, right?
Yes. Then again if you do data journalling you do that anyway, and as
one little project I'm doing right now shows that data journling is
often the fastest thing we can do for very small writes.
^ permalink raw reply
* Re: [PATCH 10/21] block: Add fops atomic write support
From: John Garry @ 2023-12-04 13:13 UTC (permalink / raw)
To: Ming Lei
Cc: axboe, kbusch, hch, sagi, jejb, martin.petersen, djwong, viro,
brauner, chandan.babu, dchinner, linux-block, linux-kernel,
linux-nvme, linux-xfs, linux-fsdevel, tytso, jbongio, linux-api
In-Reply-To: <ZW3DracIEH7uTyEA@fedora>
>>
>> I added this here (as opposed to the caller), as I was not really worried
>> about speeding up the failure path. Are you saying to call even earlier in
>> submission path?
> atomic_write_unit_min is one hardware property, and it should be checked
> in blk_queue_atomic_write_unit_min_sectors() from beginning, then you
> can avoid this check every other where.
ok, but we still need to ensure in the submission path that the block
device actually supports atomic writes - this was the initial check.
>
>>>> + if (pos % atomic_write_unit_min_bytes)
>>>> + return false;
>>>> + if (iov_iter_count(iter) % atomic_write_unit_min_bytes)
>>>> + return false;
>>>> + if (!is_power_of_2(iov_iter_count(iter)))
>>>> + return false;
>>>> + if (iov_iter_count(iter) > atomic_write_unit_max_bytes)
>>>> + return false;
>>>> + if (pos % iov_iter_count(iter))
>>>> + return false;
>>> I am a bit confused about relation between atomic_write_unit_max_bytes and
>>> atomic_write_max_bytes.
>> I think that naming could be improved. Or even just drop merging (and
>> atomic_write_max_bytes concept) until we show it to improve performance.
>>
>> So generally atomic_write_unit_max_bytes will be same as
>> atomic_write_max_bytes, however it could be different if:
>> a. request queue nr hw segments or other request queue limits needs to
>> restrict atomic_write_unit_max_bytes
>> b. atomic_write_unit_max_bytes does not need to be a power-of-2 and
>> atomic_write_max_bytes does. So essentially:
>> atomic_write_unit_max_bytes = rounddown_pow_of_2(atomic_write_max_bytes)
>>
> plug merge often improves sequential IO perf, so if the hardware supports
> this way, I think 'atomic_write_max_bytes' should be supported from the
> beginning, such as:
>
> - user space submits sequential N * (4k, 8k, 16k, ...) atomic writes, all can
> be merged to single IO request, which is issued to driver.
>
> Or
>
> - user space submits sequential 4k, 4k, 8k, 16K, 32k, 64k atomic writes, all can
> be merged to single IO request, which is issued to driver.
Right, we do expect userspace to use a fixed block size, but we give
scope in the API to use variable size.
>
> The hardware should recognize unit size by start LBA, and check if length is
> valid, so probably the interface might be relaxed to:
>
> 1) start lba is unit aligned, and this unit is in the supported unit
> range(power_2 in [unit_min, unit_max])
>
> 2) length needs to be:
>
> - N * this_unit_size
> - <= atomic_write_max_bytes
Please note that we also need to consider:
- any atomic write boundary (from NVMe)
- virt boundary (from NVMe)
And, as I mentioned elsewhere, I am still not 100% comfortable that we
don't pay attention to regular max_sectors_kb...
>
>
>>> Here the max IO length is limited to be <= atomic_write_unit_max_bytes,
>>> so looks userspace can only submit IO with write-atomic-unit naturally
>>> aligned IO(such as, 4k, 8k, 16k, 32k, ...),
>> correct
>>
>>> but these user IOs are
>>> allowed to be merged to big one if naturally alignment is respected and
>>> the merged IO size is <= atomic_write_max_bytes.
>> correct, but the resultant merged IO does not have have to be naturally
>> aligned.
>>
>>> Is my understanding right?
>> Yes, but...
>>
>>> If yes, I'd suggest to document the point,
>>> and the last two checks could be change to:
>>>
>>> /* naturally aligned */
>>> if (pos % iov_iter_count(iter))
>>> return false;
>>>
>>> if (iov_iter_count(iter) > atomic_write_max_bytes)
>>> return false;
>> .. we would not be merging at this point as this is just IO submission to
>> the block layer, so atomic_write_max_bytes does not come into play yet. If
>> you check patch 7/21, you will see that we limit IO size to
>> atomic_write_max_bytes, which is relevant merging.
> I know the motivation of atomic_write_max_bytes, and now I am wondering
> atomic_write_max_bytes may be exported to userspace for the sake of
> atomic write performance.
It is available from sysfs for the request queue, but in an earlier
series Dave Chinner suggested doing more to expose to the application
programmer. So here that would mean a statx member. I'm still not
sure... it just didn't seem like a detail which the user would need to
know or be able to do much with.
Thanks,
John
^ permalink raw reply
* Re: [PATCH 10/21] block: Add fops atomic write support
From: Ming Lei @ 2023-12-04 12:18 UTC (permalink / raw)
To: John Garry
Cc: axboe, kbusch, hch, sagi, jejb, martin.petersen, djwong, viro,
brauner, chandan.babu, dchinner, linux-block, linux-kernel,
linux-nvme, linux-xfs, linux-fsdevel, tytso, jbongio, linux-api,
ming.lei
In-Reply-To: <03a87103-0721-412c-92f5-9fd605dc0c74@oracle.com>
On Mon, Dec 04, 2023 at 09:27:00AM +0000, John Garry wrote:
> On 04/12/2023 02:30, Ming Lei wrote:
>
> Hi Ming,
>
> > > +static bool blkdev_atomic_write_valid(struct block_device *bdev, loff_t pos,
> > > + struct iov_iter *iter)
> > > +{
> > > + unsigned int atomic_write_unit_min_bytes =
> > > + queue_atomic_write_unit_min_bytes(bdev_get_queue(bdev));
> > > + unsigned int atomic_write_unit_max_bytes =
> > > + queue_atomic_write_unit_max_bytes(bdev_get_queue(bdev));
> > > +
> > > + if (!atomic_write_unit_min_bytes)
> > > + return false;
> > The above check should have be moved to limit setting code path.
>
> Sorry, I didn't fully understand your point.
>
> I added this here (as opposed to the caller), as I was not really worried
> about speeding up the failure path. Are you saying to call even earlier in
> submission path?
atomic_write_unit_min is one hardware property, and it should be checked
in blk_queue_atomic_write_unit_min_sectors() from beginning, then you
can avoid this check every other where.
>
> >
> > > + if (pos % atomic_write_unit_min_bytes)
> > > + return false;
> > > + if (iov_iter_count(iter) % atomic_write_unit_min_bytes)
> > > + return false;
> > > + if (!is_power_of_2(iov_iter_count(iter)))
> > > + return false;
> > > + if (iov_iter_count(iter) > atomic_write_unit_max_bytes)
> > > + return false;
> > > + if (pos % iov_iter_count(iter))
> > > + return false;
> > I am a bit confused about relation between atomic_write_unit_max_bytes and
> > atomic_write_max_bytes.
>
> I think that naming could be improved. Or even just drop merging (and
> atomic_write_max_bytes concept) until we show it to improve performance.
>
> So generally atomic_write_unit_max_bytes will be same as
> atomic_write_max_bytes, however it could be different if:
> a. request queue nr hw segments or other request queue limits needs to
> restrict atomic_write_unit_max_bytes
> b. atomic_write_unit_max_bytes does not need to be a power-of-2 and
> atomic_write_max_bytes does. So essentially:
> atomic_write_unit_max_bytes = rounddown_pow_of_2(atomic_write_max_bytes)
>
plug merge often improves sequential IO perf, so if the hardware supports
this way, I think 'atomic_write_max_bytes' should be supported from the
beginning, such as:
- user space submits sequential N * (4k, 8k, 16k, ...) atomic writes, all can
be merged to single IO request, which is issued to driver.
Or
- user space submits sequential 4k, 4k, 8k, 16K, 32k, 64k atomic writes, all can
be merged to single IO request, which is issued to driver.
The hardware should recognize unit size by start LBA, and check if length is
valid, so probably the interface might be relaxed to:
1) start lba is unit aligned, and this unit is in the supported unit
range(power_2 in [unit_min, unit_max])
2) length needs to be:
- N * this_unit_size
- <= atomic_write_max_bytes
> >
> > Here the max IO length is limited to be <= atomic_write_unit_max_bytes,
> > so looks userspace can only submit IO with write-atomic-unit naturally
> > aligned IO(such as, 4k, 8k, 16k, 32k, ...),
>
> correct
>
> > but these user IOs are
> > allowed to be merged to big one if naturally alignment is respected and
> > the merged IO size is <= atomic_write_max_bytes.
>
> correct, but the resultant merged IO does not have have to be naturally
> aligned.
>
> >
> > Is my understanding right?
>
> Yes, but...
>
> > If yes, I'd suggest to document the point,
> > and the last two checks could be change to:
> >
> > /* naturally aligned */
> > if (pos % iov_iter_count(iter))
> > return false;
> >
> > if (iov_iter_count(iter) > atomic_write_max_bytes)
> > return false;
>
> .. we would not be merging at this point as this is just IO submission to
> the block layer, so atomic_write_max_bytes does not come into play yet. If
> you check patch 7/21, you will see that we limit IO size to
> atomic_write_max_bytes, which is relevant merging.
I know the motivation of atomic_write_max_bytes, and now I am wondering
atomic_write_max_bytes may be exported to userspace for the sake of
atomic write performance.
Thanks,
Ming
^ permalink raw reply
* Re: [PATCH 02/21] block: Limit atomic writes according to bio and queue limits
From: John Garry @ 2023-12-04 9:35 UTC (permalink / raw)
To: Ming Lei
Cc: axboe, kbusch, hch, sagi, jejb, martin.petersen, djwong, viro,
brauner, chandan.babu, dchinner, linux-block, linux-kernel,
linux-nvme, linux-xfs, linux-fsdevel, tytso, jbongio, linux-api
In-Reply-To: <ZW1NxiEh2x82SOai@fedora>
On 04/12/2023 03:55, Ming Lei wrote:
Hi Ming,
> On Mon, Dec 04, 2023 at 11:19:20AM +0800, Ming Lei wrote:
>> On Fri, Sep 29, 2023 at 10:27:07AM +0000, John Garry wrote:
>>> We rely the block layer always being able to send a bio of size
>>> atomic_write_unit_max without being required to split it due to request
>>> queue or other bio limits.
>>>
>>> A bio may contain min(BIO_MAX_VECS, limits->max_segments) vectors,
>>> and each vector is at worst case the device logical block size from
>>> direct IO alignment requirement.
>> Both unit_max and unit_min are applied to FS bio, which is built over
>> single userspace buffer, so only the 1st and last vector can include
> Actually it isn't true for pwritev, and sorry for the noise.
Yeah, I think that it should be:
(max_segments - 2) * PAGE_SIZE
And we need to enforce that any middle vectors are PAGE-aligned.
Thanks,
John
^ permalink raw reply
* Re: [PATCH 10/21] block: Add fops atomic write support
From: John Garry @ 2023-12-04 9:27 UTC (permalink / raw)
To: Ming Lei
Cc: axboe, kbusch, hch, sagi, jejb, martin.petersen, djwong, viro,
brauner, chandan.babu, dchinner, linux-block, linux-kernel,
linux-nvme, linux-xfs, linux-fsdevel, tytso, jbongio, linux-api
In-Reply-To: <ZW05th/c0sNbM2Zf@fedora>
On 04/12/2023 02:30, Ming Lei wrote:
Hi Ming,
>> +static bool blkdev_atomic_write_valid(struct block_device *bdev, loff_t pos,
>> + struct iov_iter *iter)
>> +{
>> + unsigned int atomic_write_unit_min_bytes =
>> + queue_atomic_write_unit_min_bytes(bdev_get_queue(bdev));
>> + unsigned int atomic_write_unit_max_bytes =
>> + queue_atomic_write_unit_max_bytes(bdev_get_queue(bdev));
>> +
>> + if (!atomic_write_unit_min_bytes)
>> + return false;
> The above check should have be moved to limit setting code path.
Sorry, I didn't fully understand your point.
I added this here (as opposed to the caller), as I was not really
worried about speeding up the failure path. Are you saying to call even
earlier in submission path?
>
>> + if (pos % atomic_write_unit_min_bytes)
>> + return false;
>> + if (iov_iter_count(iter) % atomic_write_unit_min_bytes)
>> + return false;
>> + if (!is_power_of_2(iov_iter_count(iter)))
>> + return false;
>> + if (iov_iter_count(iter) > atomic_write_unit_max_bytes)
>> + return false;
>> + if (pos % iov_iter_count(iter))
>> + return false;
> I am a bit confused about relation between atomic_write_unit_max_bytes and
> atomic_write_max_bytes.
I think that naming could be improved. Or even just drop merging (and
atomic_write_max_bytes concept) until we show it to improve performance.
So generally atomic_write_unit_max_bytes will be same as
atomic_write_max_bytes, however it could be different if:
a. request queue nr hw segments or other request queue limits needs to
restrict atomic_write_unit_max_bytes
b. atomic_write_unit_max_bytes does not need to be a power-of-2 and
atomic_write_max_bytes does. So essentially:
atomic_write_unit_max_bytes = rounddown_pow_of_2(atomic_write_max_bytes)
>
> Here the max IO length is limited to be <= atomic_write_unit_max_bytes,
> so looks userspace can only submit IO with write-atomic-unit naturally
> aligned IO(such as, 4k, 8k, 16k, 32k, ...),
correct
> but these user IOs are
> allowed to be merged to big one if naturally alignment is respected and
> the merged IO size is <= atomic_write_max_bytes.
correct, but the resultant merged IO does not have have to be naturally
aligned.
>
> Is my understanding right?
Yes, but...
> If yes, I'd suggest to document the point,
> and the last two checks could be change to:
>
> /* naturally aligned */
> if (pos % iov_iter_count(iter))
> return false;
>
> if (iov_iter_count(iter) > atomic_write_max_bytes)
> return false;
.. we would not be merging at this point as this is just IO submission
to the block layer, so atomic_write_max_bytes does not come into play
yet. If you check patch 7/21, you will see that we limit IO size to
atomic_write_max_bytes, which is relevant merging.
Thanks,
John
^ permalink raw reply
* Re: [RFC PATCH 0/2] Node migration between memory tiers
From: Huang, Ying @ 2023-12-04 8:52 UTC (permalink / raw)
To: sthanneeru.opensrc
Cc: aneesh.kumar, hannes, hasanalmaruf, haowang3, gregory.price,
dan.j.williams, mhocko, tj, hezhongkun.hzk, fvdl, john, mirakhur,
vtavarespetr, Ravis.OpenSrc, Jonathan.Cameron, linux-kernel,
linux-api, Johannes Weiner
In-Reply-To: <20231130214858.1887-1-sthanneeru.opensrc@micron.com>
<sthanneeru.opensrc@micron.com> writes:
> From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
>
> The memory tiers feature allows nodes with similar memory types
> or performance characteristics to be grouped together in a
> memory tier. However, there is currently no provision for
> moving a node from one tier to another on demand.
>
> This patch series aims to support node migration between tiers
> on demand by sysadmin/root user using the provided sysfs for
> node migration. Each tier has a start abstract distance(adistance)
> and range.
We have discussed migrating nodes (in fact nodes of a memory type)
between tiers by sysadmin/root before. The basic idea comes from
Johannes. It is summarized in page 11 of [1],
[1] https://lpc.events/event/16/contributions/1209/attachments/1042/1995/Live%20In%20a%20World%20With%20Multiple%20Memory%20Types.pdf
The abstract distance of a memory type (e.g., GPU HBM) can be adjusted
via a sysfs knob (<memory_type>/abstract_distance_offset).
I still think that the memory type is better to be used to change
the abstract distance of nodes. Do you agree?
--
Best Regards,
Huang, Ying
> To migrate a node to a tier, the corresponding node’s sysfs
> adistance_offset is written with a value corresponding to
> the tier’s adistance.
>
> Example: Move node2 to memory tier5 from its default tier(i.e 4)
>
> 1. Check default values:
> $cat /sys/devices/virtual/memory_tiering/memory_tier4/nodelist
> 0-2
>
> $cat /sys/devices/system/node/node0/adistance_offset
> 0
> $cat /sys/devices/system/node/node1/adistance_offset
> 0
> $cat /sys/devices/system/node/node2/adistance_offset
> 0
>
> 2. Move node2 to tier5:
>
> To move node2 from emory_tier4 (adistance=512) to
> emory_tier5 (abstract=640), set the `adistance_offset` of
> node 2 to 128 (i.e., 512 + 128 = 640).
>
> Tier4 adistance start can be derved from tier-id
> (i.e for tier4, 4 << 7 = 512).
>
> $echo 128 > /sys/devices/system/node/node2/adistance_offset
> $cat /sys/devices/system/node/node2/adistance_offset
> 128
>
> 3. Verify node2's tier id:
>
> $cat /sys/devices/virtual/memory_tiering/memory_tier5/nodelist
> 2
> $cat /sys/devices/virtual/memory_tiering/memory_tier4/nodelist
> 0-1
>
> Srinivasulu Thanneeru (2):
> base/node: Add sysfs for adistance_offset
> memory tier: Support node migration between tiers
>
> drivers/base/node.c | 51 +++++++++++++++++++++++
> include/linux/memory-tiers.h | 11 +++++
> include/linux/node.h | 6 +++
> mm/memory-tiers.c | 79 ++++++++++++++++++++----------------
> 4 files changed, 113 insertions(+), 34 deletions(-)
^ permalink raw reply
* Re: [PATCH 02/21] block: Limit atomic writes according to bio and queue limits
From: Ming Lei @ 2023-12-04 3:55 UTC (permalink / raw)
To: John Garry
Cc: axboe, kbusch, hch, sagi, jejb, martin.petersen, djwong, viro,
brauner, chandan.babu, dchinner, linux-block, linux-kernel,
linux-nvme, linux-xfs, linux-fsdevel, tytso, jbongio, linux-api
In-Reply-To: <ZW1FOFWsUGUNLajE@fedora>
On Mon, Dec 04, 2023 at 11:19:20AM +0800, Ming Lei wrote:
> On Fri, Sep 29, 2023 at 10:27:07AM +0000, John Garry wrote:
> > We rely the block layer always being able to send a bio of size
> > atomic_write_unit_max without being required to split it due to request
> > queue or other bio limits.
> >
> > A bio may contain min(BIO_MAX_VECS, limits->max_segments) vectors,
> > and each vector is at worst case the device logical block size from
> > direct IO alignment requirement.
>
> Both unit_max and unit_min are applied to FS bio, which is built over
> single userspace buffer, so only the 1st and last vector can include
Actually it isn't true for pwritev, and sorry for the noise.
Thanks,
Ming
^ permalink raw reply
* Re: [PATCH 02/21] block: Limit atomic writes according to bio and queue limits
From: Ming Lei @ 2023-12-04 3:19 UTC (permalink / raw)
To: John Garry
Cc: axboe, kbusch, hch, sagi, jejb, martin.petersen, djwong, viro,
brauner, chandan.babu, dchinner, linux-block, linux-kernel,
linux-nvme, linux-xfs, linux-fsdevel, tytso, jbongio, linux-api,
ming.lei
In-Reply-To: <20230929102726.2985188-3-john.g.garry@oracle.com>
On Fri, Sep 29, 2023 at 10:27:07AM +0000, John Garry wrote:
> We rely the block layer always being able to send a bio of size
> atomic_write_unit_max without being required to split it due to request
> queue or other bio limits.
>
> A bio may contain min(BIO_MAX_VECS, limits->max_segments) vectors,
> and each vector is at worst case the device logical block size from
> direct IO alignment requirement.
Both unit_max and unit_min are applied to FS bio, which is built over
single userspace buffer, so only the 1st and last vector can include
partial page, and the other vectors should always cover whole page,
then the minimal size could be:
(max_segments - 2) * PAGE_SIZE + 2 * queue_logical_block_size(q)
Thanks,
Ming
^ permalink raw reply
* Re: [PATCH 10/21] block: Add fops atomic write support
From: Ming Lei @ 2023-12-04 2:30 UTC (permalink / raw)
To: John Garry
Cc: axboe, kbusch, hch, sagi, jejb, martin.petersen, djwong, viro,
brauner, chandan.babu, dchinner, linux-block, linux-kernel,
linux-nvme, linux-xfs, linux-fsdevel, tytso, jbongio, linux-api,
ming.lei
In-Reply-To: <20230929102726.2985188-11-john.g.garry@oracle.com>
On Fri, Sep 29, 2023 at 10:27:15AM +0000, John Garry wrote:
> Add support for atomic writes, as follows:
> - Ensure that the IO follows all the atomic writes rules, like must be
> naturally aligned
> - Set REQ_ATOMIC
>
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
> block/fops.c | 42 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/block/fops.c b/block/fops.c
> index acff3d5d22d4..516669ad69e5 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -41,6 +41,29 @@ static bool blkdev_dio_unaligned(struct block_device *bdev, loff_t pos,
> !bdev_iter_is_aligned(bdev, iter);
> }
>
> +static bool blkdev_atomic_write_valid(struct block_device *bdev, loff_t pos,
> + struct iov_iter *iter)
> +{
> + unsigned int atomic_write_unit_min_bytes =
> + queue_atomic_write_unit_min_bytes(bdev_get_queue(bdev));
> + unsigned int atomic_write_unit_max_bytes =
> + queue_atomic_write_unit_max_bytes(bdev_get_queue(bdev));
> +
> + if (!atomic_write_unit_min_bytes)
> + return false;
The above check should have be moved to limit setting code path.
> + if (pos % atomic_write_unit_min_bytes)
> + return false;
> + if (iov_iter_count(iter) % atomic_write_unit_min_bytes)
> + return false;
> + if (!is_power_of_2(iov_iter_count(iter)))
> + return false;
> + if (iov_iter_count(iter) > atomic_write_unit_max_bytes)
> + return false;
> + if (pos % iov_iter_count(iter))
> + return false;
I am a bit confused about relation between atomic_write_unit_max_bytes and
atomic_write_max_bytes.
Here the max IO length is limited to be <= atomic_write_unit_max_bytes,
so looks userspace can only submit IO with write-atomic-unit naturally
aligned IO(such as, 4k, 8k, 16k, 32k, ...), but these user IOs are
allowed to be merged to big one if naturally alignment is respected and
the merged IO size is <= atomic_write_max_bytes.
Is my understanding right? If yes, I'd suggest to document the point,
and the last two checks could be change to:
/* naturally aligned */
if (pos % iov_iter_count(iter))
return false;
if (iov_iter_count(iter) > atomic_write_max_bytes)
return false;
Thanks,
Ming
^ permalink raw reply
* Re: [PATCH RFT v4 0/5] fork: Support shadow stacks in clone3()
From: Mark Brown @ 2023-12-01 18:28 UTC (permalink / raw)
To: Catalin Marinas
Cc: Rick P. Edgecombe, Deepak Gupta, Szabolcs Nagy, H.J. Lu,
Florian Weimer, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Peter Zijlstra, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Daniel Bristot de Oliveira, Valentin Schneider,
Christian Brauner, Shuah Khan, linux-kernel, Will Deacon,
Kees Cook, jannh, linux-kselftest, linux-api, David Hildenbrand
In-Reply-To: <ZWoYLs2STGA1LZLU@arm.com>
[-- Attachment #1: Type: text/plain, Size: 1937 bytes --]
On Fri, Dec 01, 2023 at 05:30:22PM +0000, Catalin Marinas wrote:
> Another concern I had was that map_shadow_stack() currently takes
> a flags arg (though only one flag) while the clone/clone3() allocate the
> shadow stack with an implicit configuration (other than size). Would
> map_shadow_stack() ever get new flags that we may also need to set on
> the default thread shadow stack (e.g. a new permission type)? At that
> point it would be better if clone3() allowed a shadow stack pointer so
> that any specific attributes would be limited to map_shadow_stack().
The flags argument currently only lets you specify if a stack switch
token should be written (which is not relevant for the clone3() case)
and if a top of stack marker should be included (which since the top of
stack marker is NULL for arm64 only has perceptible effect if a token is
being written). I'm not particularly anticipating any further additions,
though never say never.
> If that's only theoretical, I'm fine to go ahead with a size-only
> argument for clone3(). We could also add the pointer now and allocate
> the stack if NULL or reuse it if not, maybe with some prctl to allow
> this. It might be overengineering and we'd never use such feature
> though.
Yeah, it seems like a bunch of work and interface to test that I'm not
convinced anyone would actually use.
> > As well as the actual configuration of the size the other thing that we
> > gain is that as well as relying on heuristics to determine if we need to
> > allocate a new shadow stack for the new thread we allow userspace to
> > explicitly request a new shadow stack.
> But the reverse is not true - we can't use clone3() to create a thread
> without a shadow stack AFAICT.
Right. Given the existing implicit allocation only x86 ABI we'd need to
retrofit that by adding an explicit "no shadow stack" flag. That is
possible though I'm having a hard time seeing the use case for it.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH RFT v4 0/5] fork: Support shadow stacks in clone3()
From: Catalin Marinas @ 2023-12-01 17:30 UTC (permalink / raw)
To: Mark Brown
Cc: Rick P. Edgecombe, Deepak Gupta, Szabolcs Nagy, H.J. Lu,
Florian Weimer, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Peter Zijlstra, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Daniel Bristot de Oliveira, Valentin Schneider,
Christian Brauner, Shuah Khan, linux-kernel, Will Deacon,
Kees Cook, jannh, linux-kselftest, linux-api, David Hildenbrand
In-Reply-To: <fce4c169-5d19-40e8-bc32-0abec9bb008e@sirena.org.uk>
Thanks all for the clarification.
On Thu, Nov 30, 2023 at 09:51:04PM +0000, Mark Brown wrote:
> On Thu, Nov 30, 2023 at 07:00:58PM +0000, Catalin Marinas wrote:
> > My hope when looking at the arm64 patches was that we can completely
> > avoid the kernel allocation/deallocation of the shadow stack since it
> > doesn't need to do this for the normal stack either. Could someone
> > please summarise why we dropped the shadow stack pointer after v1? IIUC
> > there was a potential security argument but I don't think it was a very
> > strong one. Also what's the threat model for this feature? I thought
> > it's mainly mitigating stack corruption. If some rogue code can do
> > syscalls, we have bigger problems than clone3() taking a shadow stack
> > pointer.
>
> As well as preventing/detecting corruption of the in memory stack shadow
> stacks are also ensuring that any return instructions are unwinding a
> prior call instruction, and that the returns are done in opposite order
> to the calls. This forces usage of the stack - any value we attempt to
> RET to is going to be checked against the top of the shadow stack which
> makes chaining returns together as a substitute for branches harder.
>
> The concern Rick raised was that allowing user to pick the exact shadow
> stack pointer would allow userspace to corrupt or reuse the stack of an
> existing thread by starting a new thread with the shadow stack pointing
> into the existing shadow stack of that thread. While in isolation
> that's not too much more than what userspace could just do directly
> anyway it might compose with other issues to something more "interesting"
> (eg, I'd be a bit concerned about overlap with pkeys/POE though I've not
> thought through potential uses in detail).
Another concern I had was that map_shadow_stack() currently takes
a flags arg (though only one flag) while the clone/clone3() allocate the
shadow stack with an implicit configuration (other than size). Would
map_shadow_stack() ever get new flags that we may also need to set on
the default thread shadow stack (e.g. a new permission type)? At that
point it would be better if clone3() allowed a shadow stack pointer so
that any specific attributes would be limited to map_shadow_stack().
If that's only theoretical, I'm fine to go ahead with a size-only
argument for clone3(). We could also add the pointer now and allocate
the stack if NULL or reuse it if not, maybe with some prctl to allow
this. It might be overengineering and we'd never use such feature
though.
> > I'm not against clone3() getting a shadow_stack_size argument but asking
> > some more questions. If we won't pass a pointer as well, is there any
> > advantage in expanding this syscall vs a specific prctl() option? Do we
> > need a different size per thread or do all threads have the same shadow
> > stack size? A new RLIMIT doesn't seem to map well though, it is more
> > like an upper limit rather than a fixed/default size (glibc I think uses
> > it for thread stacks but bionic or musl don't AFAIK).
>
> I don't know what the userspace patterns are likely to be here, it's
> possible a single value for each process might be fine but I couldn't
> say that confidently. I agree that a RLIMIT does seem like a poor fit.
Szabolcs clarified that there are cases where we need the size per
thread.
> As well as the actual configuration of the size the other thing that we
> gain is that as well as relying on heuristics to determine if we need to
> allocate a new shadow stack for the new thread we allow userspace to
> explicitly request a new shadow stack.
But the reverse is not true - we can't use clone3() to create a thread
without a shadow stack AFAICT.
> > Another dumb question on arm64 - is GCSPR_EL0 writeable by the user? If
> > yes, can the libc wrapper for threads allocate a shadow stack via
> > map_shadow_stack() and set it up in the thread initialisation handler
> > before invoking the thread function?
>
> No, GCSPR_EL0 can only be changed by EL0 through BL, RET and the
> new GCS instructions (push/pop and stack switch). Push is optional -
> userspace has to explicitly request that it be enabled and this could be
> prevented through seccomp or some other LSM. The stack switch
> instructions require a token at the destination address which must
> either be written by a higher EL or will be written in the process of
> switching away from a stack so you can switch back. Unless I've missed
> one every mechanism for userspace to update GCSPR_EL0 will do a GCS
> memory access so providing guard pages have been allocated wrapping to a
> different stack will be prevented.
>
> We would need a syscall to allow GCSPR_EL0 to be written.
Good point, I thought I must be missing something.
--
Catalin
^ permalink raw reply
* Re: [RFC 1/3] pidfd: allow pidfd_open() on non-thread-group leaders
From: Tycho Andersen @ 2023-12-01 16:31 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Christian Brauner, Eric W . Biederman, linux-kernel, linux-api,
Tycho Andersen
In-Reply-To: <ZWjM6trZ6uw6yBza@tycho.pizza>
On Thu, Nov 30, 2023 at 10:57:01AM -0700, Tycho Andersen wrote:
> On Thu, Nov 30, 2023 at 06:39:39PM +0100, Oleg Nesterov wrote:
> > I think that wake_up_all(wait_pidfd) should have a single caller,
> > do_notify_pidfd(). This probably means it should be shiftef from
> > do_notify_parent() to exit_notify(), I am not sure...
Indeed, below passes the tests without issue and is much less ugly.
I'll respin with that later next week sometime.
Thanks,
Tycho
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 3499c1a8b929..04c4423ebed0 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -332,6 +332,7 @@ extern int kill_pid_usb_asyncio(int sig, int errno, sigval_t addr, struct pid *,
extern int kill_pgrp(struct pid *pid, int sig, int priv);
extern int kill_pid(struct pid *pid, int sig, int priv);
extern __must_check bool do_notify_parent(struct task_struct *, int);
+extern void do_notify_pidfd(struct task_struct *);
extern void __wake_up_parent(struct task_struct *p, struct task_struct *parent);
extern void force_sig(int);
extern void force_fatal_sig(int);
diff --git a/kernel/exit.c b/kernel/exit.c
index 34eeefc7ee21..fd6048c20c48 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -769,6 +769,8 @@ static void exit_notify(struct task_struct *tsk, int group_dead)
wake_up_process(tsk->signal->group_exec_task);
write_unlock_irq(&tasklist_lock);
+ do_notify_pidfd(tsk);
+
list_for_each_entry_safe(p, n, &dead, ptrace_entry) {
list_del_init(&p->ptrace_entry);
release_task(p);
diff --git a/kernel/signal.c b/kernel/signal.c
index 47a7602dfe8d..7b3a1e147225 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2028,7 +2028,7 @@ int send_sigqueue(struct sigqueue *q, struct pid *pid, enum pid_type type)
return ret;
}
-static void do_notify_pidfd(struct task_struct *task)
+void do_notify_pidfd(struct task_struct *task)
{
struct pid *pid;
@@ -2060,9 +2060,6 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
WARN_ON_ONCE(!tsk->ptrace &&
(tsk->group_leader != tsk || !thread_group_empty(tsk)));
- /* Wake up all pidfd waiters */
- do_notify_pidfd(tsk);
-
if (sig != SIGCHLD) {
/*
* This is only possible if parent == real_parent.
^ permalink raw reply related
* Re: [PATCH RFT v4 0/5] fork: Support shadow stacks in clone3()
From: Mark Brown @ 2023-12-01 14:00 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: catalin.marinas@arm.com, dietmar.eggemann@arm.com,
keescook@chromium.org, Szabolcs.Nagy@arm.com, brauner@kernel.org,
dave.hansen@linux.intel.com, debug@rivosinc.com, mgorman@suse.de,
vincent.guittot@linaro.org, fweimer@redhat.com,
linux-kernel@vger.kernel.org, mingo@redhat.com,
rostedt@goodmis.org, hjl.tools@gmail.com, tglx@linutronix.de,
linux-api@vger.kernel.org, vschneid@redhat.com, shuah@kernel.org,
bristot@redhat.com, will@kernel.org, hpa@zytor.com,
peterz@infradead.org, jannh@google.com, bp@alien8.de,
bsegall@google.com, linux-kselftest@vger.kernel.org,
david@redhat.com, x86@kernel.org, juri.lelli@redhat.com
In-Reply-To: <881e1b6d89d61cef4e71c6be688635fc47bb2b8e.camel@intel.com>
[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]
On Thu, Nov 30, 2023 at 11:37:42PM +0000, Edgecombe, Rick P wrote:
> On Thu, 2023-11-30 at 21:51 +0000, Mark Brown wrote:
> > On Thu, Nov 30, 2023 at 07:00:58PM +0000, Catalin Marinas wrote:
> > explicitly request a new shadow stack. There was some corner case
> > with
> > IIRC posix_nspawn() mentioned where the heuristics aren't what we
> > want
> > for example.
> Can't posix_spawn() pass in a shadow stack size into clone3 to get a
> new shadow stack after this series?
Yes, the above was addressing Catalin's suggestion that we add stack
size control separately to clone3() instead - doing that would remove
the ability to explicitly request a new stack unless we add a flag to
clone3() at which point we're back to modifying clone3() anyway.
> > > Another dumb question on arm64 - is GCSPR_EL0 writeable by the
> > > user? If
> > > yes, can the libc wrapper for threads allocate a shadow stack via
> > > map_shadow_stack() and set it up in the thread initialisation
> > > handler
> > > before invoking the thread function?
> > We would need a syscall to allow GCSPR_EL0 to be written.
> I think the problem with doing this is signals. If a signal is
> delivered to the new thread, then it could push to the old shadow stack
> before userspace gets a chance to switch. So the thread needs to start
> on a new shadow/stack.
That's an issue, plus using a syscall just wouldn't work with a security
model that locked down writes to the pointer which does seem like
something people would reasonably want to deploy.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH] mm/mbind: Introduce process_mbind() syscall for external memory binding
From: Zhongkun He @ 2023-12-01 13:53 UTC (permalink / raw)
To: Gregory Price
Cc: Vinicius Petrucci, akpm, linux-mm, linux-cxl, linux-kernel,
linux-arch, linux-api, minchan, dave.hansen, x86,
Jonathan.Cameron, aneesh.kumar, ying.huang, dan.j.williams, fvdl,
surenb, rientjes, hannes, mhocko, Hasan.Maruf, jgroves,
ravis.opensrc, sthanneeru, emirakhur, vtavarespetr
In-Reply-To: <ZWizUEd/rsxSc0fW@memverge.com>
>
> Hi ZhongKun!
>
> I actually just sent out a more general RFC to mempolicy updates that
> discuss this more completely:
>
> https://lore.kernel.org/linux-mm/ZWezcQk+BYEq%2FWiI@memverge.com/
>
OK.
> and another post on even more issues with pidfd modifications to vma
> mempolicies:
>
> https://lore.kernel.org/linux-mm/ZWYsth2CtC4Ilvoz@memverge.com/
>
> We may have to slow-walk the changes to vma policies due to there being
> many more hidden accesses to (current) than expected. It's a rather
> nasty rats nest of mempolicy-vma-cpusets-shmem callbacks that obscure
> these current-task accesses, it will take time to work through.
>
Got it, thanks. It's more complicated than I thought.
> As for hot-path reference counting - we may need to change the way
> mempolicy is managed, possibly we could leverage RCU to manage mempolicy
> references in the hot path, rather than using locks. In this scenario,
> we would likely need to change the way the default policy is applied
> (maybe not, I haven't fully explored it).
>
RCU may have a long time in the read-side critical section.
We should probably replace the atomic_t refcnt with percpu_ref in
mempolicy(also suggested by Michal), but refactoring work involves
a lot of code.
A simple way is to use task_work to release the mempolicy which may
be used by alloc_pages(). But it doesn't have a direct result.
> Do you have thoughts on this? Would very much like additional comments
> before I go through the refactor work.
>
> Regards,
> Gregory
^ permalink raw reply
* Re: [PATCH RFT v4 0/5] fork: Support shadow stacks in clone3()
From: Mark Brown @ 2023-12-01 13:47 UTC (permalink / raw)
To: Szabolcs Nagy
Cc: Catalin Marinas, Rick P. Edgecombe, Deepak Gupta, H.J. Lu,
Florian Weimer, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Peter Zijlstra, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Daniel Bristot de Oliveira, Valentin Schneider,
Christian Brauner, Shuah Khan, linux-kernel, Will Deacon,
Kees Cook, jannh, linux-kselftest, linux-api, David Hildenbrand
In-Reply-To: <ZWnIgQgUW0Eyfcw9@arm.com>
[-- Attachment #1: Type: text/plain, Size: 676 bytes --]
On Fri, Dec 01, 2023 at 11:50:25AM +0000, Szabolcs Nagy wrote:
> The 11/30/2023 21:51, Mark Brown wrote:
> > The concern Rick raised was that allowing user to pick the exact shadow
> > stack pointer would allow userspace to corrupt or reuse the stack of an
> > existing thread by starting a new thread with the shadow stack pointing
> > into the existing shadow stack of that thread. While in isolation
> note that this can be prevented by map_shadow_stack adding
> a token that clone3 verifies.
That would make it impossible to reuse the shadow stack once the token
is overwritten which does move the needle more towards making doing the
mapping separately pure overhead.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH RFT v4 0/5] fork: Support shadow stacks in clone3()
From: Szabolcs Nagy @ 2023-12-01 11:50 UTC (permalink / raw)
To: Mark Brown, Catalin Marinas
Cc: Rick P. Edgecombe, Deepak Gupta, H.J. Lu, Florian Weimer,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Daniel Bristot de Oliveira, Valentin Schneider, Christian Brauner,
Shuah Khan, linux-kernel, Will Deacon, Kees Cook, jannh,
linux-kselftest, linux-api, David Hildenbrand
In-Reply-To: <fce4c169-5d19-40e8-bc32-0abec9bb008e@sirena.org.uk>
The 11/30/2023 21:51, Mark Brown wrote:
> The concern Rick raised was that allowing user to pick the exact shadow
> stack pointer would allow userspace to corrupt or reuse the stack of an
> existing thread by starting a new thread with the shadow stack pointing
> into the existing shadow stack of that thread. While in isolation
note that this can be prevented by map_shadow_stack adding
a token that clone3 verifies.
> that's not too much more than what userspace could just do directly
> anyway it might compose with other issues to something more "interesting"
> (eg, I'd be a bit concerned about overlap with pkeys/POE though I've not
> thought through potential uses in detail).
>
> > I'm not against clone3() getting a shadow_stack_size argument but asking
> > some more questions. If we won't pass a pointer as well, is there any
> > advantage in expanding this syscall vs a specific prctl() option? Do we
> > need a different size per thread or do all threads have the same shadow
> > stack size? A new RLIMIT doesn't seem to map well though, it is more
> > like an upper limit rather than a fixed/default size (glibc I think uses
> > it for thread stacks but bionic or musl don't AFAIK).
>
> I don't know what the userspace patterns are likely to be here, it's
> possible a single value for each process might be fine but I couldn't
> say that confidently. I agree that a RLIMIT does seem like a poor fit.
user code can control the thread stack size per thread
and different size per thread happens in practice (even
in the libc e.g. timer_create with SIGEV_THREAD uses
different stack size than the dns resolver helper thread).
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply
* Re: [PATCH RFT v4 0/5] fork: Support shadow stacks in clone3()
From: Edgecombe, Rick P @ 2023-11-30 23:37 UTC (permalink / raw)
To: broonie@kernel.org, catalin.marinas@arm.com
Cc: dietmar.eggemann@arm.com, keescook@chromium.org,
Szabolcs.Nagy@arm.com, brauner@kernel.org,
dave.hansen@linux.intel.com, debug@rivosinc.com, mgorman@suse.de,
vincent.guittot@linaro.org, fweimer@redhat.com,
linux-kernel@vger.kernel.org, mingo@redhat.com,
rostedt@goodmis.org, hjl.tools@gmail.com, tglx@linutronix.de,
linux-api@vger.kernel.org, vschneid@redhat.com, shuah@kernel.org,
bristot@redhat.com, will@kernel.org, hpa@zytor.com,
peterz@infradead.org, jannh@google.com, bp@alien8.de,
bsegall@google.com, linux-kselftest@vger.kernel.org,
david@redhat.com, x86@kernel.org, juri.lelli@redhat.com
In-Reply-To: <fce4c169-5d19-40e8-bc32-0abec9bb008e@sirena.org.uk>
On Thu, 2023-11-30 at 21:51 +0000, Mark Brown wrote:
> On Thu, Nov 30, 2023 at 07:00:58PM +0000, Catalin Marinas wrote:
>
> > My hope when looking at the arm64 patches was that we can
> > completely
> > avoid the kernel allocation/deallocation of the shadow stack since
> > it
> > doesn't need to do this for the normal stack either. Could someone
> > please summarise why we dropped the shadow stack pointer after v1?
> > IIUC
> > there was a potential security argument but I don't think it was a
> > very
> > strong one. Also what's the threat model for this feature? I
> > thought
> > it's mainly mitigating stack corruption. If some rogue code can do
> > syscalls, we have bigger problems than clone3() taking a shadow
> > stack
> > pointer.
>
> As well as preventing/detecting corruption of the in memory stack
> shadow
> stacks are also ensuring that any return instructions are unwinding a
> prior call instruction, and that the returns are done in opposite
> order
> to the calls. This forces usage of the stack - any value we attempt
> to
> RET to is going to be checked against the top of the shadow stack
> which
> makes chaining returns together as a substitute for branches harder.
>
> The concern Rick raised was that allowing user to pick the exact
> shadow
> stack pointer would allow userspace to corrupt or reuse the stack of
> an
> existing thread by starting a new thread with the shadow stack
> pointing
> into the existing shadow stack of that thread. While in isolation
> that's not too much more than what userspace could just do directly
> anyway it might compose with other issues to something more
> "interesting"
> (eg, I'd be a bit concerned about overlap with pkeys/POE though I've
> not
> thought through potential uses in detail).
I think it is open for userspace customization. The kernel tries to
leave the option to lock things down as much as it can (partly because
it's not clear how all the userspace tradeoffs will shake out).
In the past, we had talked about allowing a set SSP (GCSPR) prctl() to
help with some of the compatibility gaps (longjmp() between stacks,
etc). If we loosened things up a bit this could help there, but it kind
of defeats the purpose a little, of the token checking stuff built into
these features at the HW level. A super-stack-canary mode might be nice
for people who just want to flip a switch on existing apps without
checking them, or people who want to do tracing and don't care about
security. But, I also wouldn't be surprised if some high security
applications decide to block map_shadow_stack all together to lock
threads to their own shadow stacks.
So I kind of like leaning towards leaving the option to lock things
down more when we can. Like Mark was getting at, we don't know all the
ways shadow stacks will get attacked yet. So turning it around, why not
let the shadow stack get allocated by the kernel? It makes the kernel
code/complexity smaller, are there any other benefits?
>
> > I'm not against clone3() getting a shadow_stack_size argument but
> > asking
> > some more questions. If we won't pass a pointer as well, is there
> > any
> > advantage in expanding this syscall vs a specific prctl() option?
> > Do we
> > need a different size per thread or do all threads have the same
> > shadow
> > stack size? A new RLIMIT doesn't seem to map well though, it is
> > more
> > like an upper limit rather than a fixed/default size (glibc I think
> > uses
> > it for thread stacks but bionic or musl don't AFAIK).
>
> I don't know what the userspace patterns are likely to be here, it's
> possible a single value for each process might be fine but I couldn't
> say that confidently. I agree that a RLIMIT does seem like a poor
> fit.
>
> As well as the actual configuration of the size the other thing that
> we
> gain is that as well as relying on heuristics to determine if we need
> to
> allocate a new shadow stack for the new thread we allow userspace to
> explicitly request a new shadow stack. There was some corner case
> with
> IIRC posix_nspawn() mentioned where the heuristics aren't what we
> want
> for example.
Can't posix_spawn() pass in a shadow stack size into clone3 to get a
new shadow stack after this series?
>
> > Another dumb question on arm64 - is GCSPR_EL0 writeable by the
> > user? If
> > yes, can the libc wrapper for threads allocate a shadow stack via
> > map_shadow_stack() and set it up in the thread initialisation
> > handler
> > before invoking the thread function?
>
> No, GCSPR_EL0 can only be changed by EL0 through BL, RET and the
> new GCS instructions (push/pop and stack switch). Push is optional -
> userspace has to explicitly request that it be enabled and this could
> be
> prevented through seccomp or some other LSM. The stack switch
> instructions require a token at the destination address which must
> either be written by a higher EL or will be written in the process of
> switching away from a stack so you can switch back. Unless I've
> missed
> one every mechanism for userspace to update GCSPR_EL0 will do a GCS
> memory access so providing guard pages have been allocated wrapping
> to a
> different stack will be prevented.
>
> We would need a syscall to allow GCSPR_EL0 to be written.
I think the problem with doing this is signals. If a signal is
delivered to the new thread, then it could push to the old shadow stack
before userspace gets a chance to switch. So the thread needs to start
on a new shadow/stack.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox