The clone3 syscall is currently broken when used with CLONE_SETTLS on all architectures that don't have an implementation of copy_thread_tls. The old copy_thread function handles CLONE_SETTLS by reading the new TLS value from pt_regs containing the clone syscall parameters. Since clone3 passes the TLS value in clone_args, this results in the TLS register being initialized to a garbage value.
This patch series implements copy_thread_tls on all architectures that currently define __ARCH_WANT_SYS_CLONE3 and adds a compile-time check to ensure that any architecture that enables clone3 in the future also implements copy_thread_tls.
I have also included a minor fix for the arm64 uapi headers which caused __NR_clone3 to be missing from the exported user headers.
I have only tested this on arm64, but the copy_thread_tls implementations for the various architectures are fairly straightforward.
Amanieu d'Antras (7): arm64: Move __ARCH_WANT_SYS_CLONE3 definition to uapi headers arm64: Implement copy_thread_tls arm: Implement copy_thread_tls parisc: Implement copy_thread_tls riscv: Implement copy_thread_tls xtensa: Implement copy_thread_tls clone3: ensure copy_thread_tls is implemented
arch/arm/Kconfig | 1 + arch/arm/kernel/process.c | 6 +++--- arch/arm64/Kconfig | 1 + arch/arm64/include/asm/unistd.h | 1 - arch/arm64/include/uapi/asm/unistd.h | 1 + arch/arm64/kernel/process.c | 10 +++++----- arch/parisc/Kconfig | 1 + arch/parisc/kernel/process.c | 8 ++++---- arch/riscv/Kconfig | 1 + arch/riscv/kernel/process.c | 6 +++--- arch/xtensa/Kconfig | 1 + arch/xtensa/kernel/process.c | 8 ++++---- kernel/fork.c | 10 ++++++++++ 13 files changed, 35 insertions(+), 20 deletions(-)
Previously this was only defined in the internal headers which resulted in __NR_clone3 not being defined in the user headers.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x --- arch/arm64/include/asm/unistd.h | 1 - arch/arm64/include/uapi/asm/unistd.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h index 2629a68b8724..5af82587909e 100644 --- a/arch/arm64/include/asm/unistd.h +++ b/arch/arm64/include/asm/unistd.h @@ -42,7 +42,6 @@ #endif
#define __ARCH_WANT_SYS_CLONE -#define __ARCH_WANT_SYS_CLONE3
#ifndef __COMPAT_SYSCALL_NR #include <uapi/asm/unistd.h> diff --git a/arch/arm64/include/uapi/asm/unistd.h b/arch/arm64/include/uapi/asm/unistd.h index 4703d218663a..f83a70e07df8 100644 --- a/arch/arm64/include/uapi/asm/unistd.h +++ b/arch/arm64/include/uapi/asm/unistd.h @@ -19,5 +19,6 @@ #define __ARCH_WANT_NEW_STAT #define __ARCH_WANT_SET_GET_RLIMIT #define __ARCH_WANT_TIME32_SYSCALLS +#define __ARCH_WANT_SYS_CLONE3
#include <asm-generic/unistd.h>
[Cc Arnd. I'd like his Ack on this]
On Thu, Jan 02, 2020 at 06:24:07PM +0100, Amanieu d'Antras wrote:
Previously this was only defined in the internal headers which resulted in __NR_clone3 not being defined in the user headers.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x
arch/arm64/include/asm/unistd.h | 1 - arch/arm64/include/uapi/asm/unistd.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h index 2629a68b8724..5af82587909e 100644 --- a/arch/arm64/include/asm/unistd.h +++ b/arch/arm64/include/asm/unistd.h @@ -42,7 +42,6 @@ #endif #define __ARCH_WANT_SYS_CLONE -#define __ARCH_WANT_SYS_CLONE3 #ifndef __COMPAT_SYSCALL_NR #include <uapi/asm/unistd.h> diff --git a/arch/arm64/include/uapi/asm/unistd.h b/arch/arm64/include/uapi/asm/unistd.h index 4703d218663a..f83a70e07df8 100644 --- a/arch/arm64/include/uapi/asm/unistd.h +++ b/arch/arm64/include/uapi/asm/unistd.h @@ -19,5 +19,6 @@ #define __ARCH_WANT_NEW_STAT #define __ARCH_WANT_SET_GET_RLIMIT #define __ARCH_WANT_TIME32_SYSCALLS +#define __ARCH_WANT_SYS_CLONE3
#include <asm-generic/unistd.h>
2.24.1
On Thu, Jan 2, 2020 at 6:50 PM Christian Brauner christian.brauner@ubuntu.com wrote:
On Thu, Jan 02, 2020 at 06:24:07PM +0100, Amanieu d'Antras wrote:
Previously this was only defined in the internal headers which resulted in __NR_clone3 not being defined in the user headers.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x
arch/arm64/include/asm/unistd.h | 1 - arch/arm64/include/uapi/asm/unistd.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-)
Good catch, this is clearly needed, but please make the patch change every copy of asm/unistd.h that defines this, not just the arm64 one.
Arnd
On Thu, Jan 2, 2020 at 8:25 PM Arnd Bergmann arnd@arndb.de wrote:
On Thu, Jan 2, 2020 at 6:50 PM Christian Brauner christian.brauner@ubuntu.com wrote:
On Thu, Jan 02, 2020 at 06:24:07PM +0100, Amanieu d'Antras wrote:
Previously this was only defined in the internal headers which resulted in __NR_clone3 not being defined in the user headers.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x
arch/arm64/include/asm/unistd.h | 1 - arch/arm64/include/uapi/asm/unistd.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-)
Good catch, this is clearly needed, but please make the patch change every copy of asm/unistd.h that defines this, not just the arm64 one.
Actually __ARCH_WANT_SYS_CLONE3 only needs to be in the uapi headers for architectures that use the asm-generic/unistd.h header, which uses it to guard the definition of __NR_clone3. Architectures not using the asm-generic header don't need this define to export __NR_clone3. The only other architecture with clone3 that uses the asm-generic header is riscv, which already defines __ARCH_WANT_SYS_CLONE3 in the uapi headers.
On Thu, Jan 2, 2020 at 8:33 PM Amanieu d'Antras amanieu@gmail.com wrote:
On Thu, Jan 2, 2020 at 8:25 PM Arnd Bergmann arnd@arndb.de wrote:
On Thu, Jan 2, 2020 at 6:50 PM Christian Brauner christian.brauner@ubuntu.com wrote:
On Thu, Jan 02, 2020 at 06:24:07PM +0100, Amanieu d'Antras wrote:
Previously this was only defined in the internal headers which resulted in __NR_clone3 not being defined in the user headers.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x
arch/arm64/include/asm/unistd.h | 1 - arch/arm64/include/uapi/asm/unistd.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-)
Good catch, this is clearly needed, but please make the patch change every copy of asm/unistd.h that defines this, not just the arm64 one.
Actually __ARCH_WANT_SYS_CLONE3 only needs to be in the uapi headers for architectures that use the asm-generic/unistd.h header, which uses it to guard the definition of __NR_clone3. Architectures not using the asm-generic header don't need this define to export __NR_clone3. The only other architecture with clone3 that uses the asm-generic header is riscv, which already defines __ARCH_WANT_SYS_CLONE3 in the uapi headers.
Ah, of course. The patch looks fine to me then.
Reviewed-by: Arnd Bergmann arnd@arndb.de
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x --- arch/arm64/Kconfig | 1 + arch/arm64/kernel/process.c | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index b1b4476ddb83..e688dfad0b72 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -138,6 +138,7 @@ config ARM64 select HAVE_CMPXCHG_DOUBLE select HAVE_CMPXCHG_LOCAL select HAVE_CONTEXT_TRACKING + select HAVE_COPY_THREAD_TLS select HAVE_DEBUG_BUGVERBOSE select HAVE_DEBUG_KMEMLEAK select HAVE_DMA_CONTIGUOUS diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 71f788cd2b18..d54586d5b031 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -360,8 +360,8 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
asmlinkage void ret_from_fork(void) asm("ret_from_fork");
-int copy_thread(unsigned long clone_flags, unsigned long stack_start, - unsigned long stk_sz, struct task_struct *p) +int copy_thread_tls(unsigned long clone_flags, unsigned long stack_start, + unsigned long stk_sz, struct task_struct *p, unsigned long tls) { struct pt_regs *childregs = task_pt_regs(p);
@@ -394,11 +394,11 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start, }
/* - * If a TLS pointer was passed to clone (4th argument), use it - * for the new thread. + * If a TLS pointer was passed to clone, use it for the new + * thread. */ if (clone_flags & CLONE_SETTLS) - p->thread.uw.tp_value = childregs->regs[3]; + p->thread.uw.tp_value = tls; } else { memset(childregs, 0, sizeof(struct pt_regs)); childregs->pstate = PSR_MODE_EL1h;
On Thu, Jan 02, 2020 at 06:24:08PM +0100, Amanieu d'Antras wrote:
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x
This looks sane to me but I'd like an ack from someone who knows his arm from his arse before taking this. :) Acked-by: Christian Brauner christian.brauner@ubuntu.com
arch/arm64/Kconfig | 1 + arch/arm64/kernel/process.c | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index b1b4476ddb83..e688dfad0b72 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -138,6 +138,7 @@ config ARM64 select HAVE_CMPXCHG_DOUBLE select HAVE_CMPXCHG_LOCAL select HAVE_CONTEXT_TRACKING
- select HAVE_COPY_THREAD_TLS select HAVE_DEBUG_BUGVERBOSE select HAVE_DEBUG_KMEMLEAK select HAVE_DMA_CONTIGUOUS
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 71f788cd2b18..d54586d5b031 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -360,8 +360,8 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) asmlinkage void ret_from_fork(void) asm("ret_from_fork"); -int copy_thread(unsigned long clone_flags, unsigned long stack_start,
unsigned long stk_sz, struct task_struct *p)+int copy_thread_tls(unsigned long clone_flags, unsigned long stack_start,
unsigned long stk_sz, struct task_struct *p, unsigned long tls){ struct pt_regs *childregs = task_pt_regs(p); @@ -394,11 +394,11 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start, } /*
* If a TLS pointer was passed to clone (4th argument), use it* for the new thread.
* If a TLS pointer was passed to clone, use it for the new* thread.
p->thread.uw.tp_value = childregs->regs[3];
p->thread.uw.tp_value = tls;-- 2.24.1
On Thu, Jan 02, 2020 at 07:01:33PM +0100, Christian Brauner wrote:
On Thu, Jan 02, 2020 at 06:24:08PM +0100, Amanieu d'Antras wrote:
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x
This looks sane to me but I'd like an ack from someone who knows his arm from his arse before taking this. :)
That's *ME*! Code looks fine:
Acked-by: Will Deacon will@kernel.org
I also ran the native and compat selftests but, unfortunately, they all pass even without this patch. Do you reckon it would be possible to update them to check the tls pointer?
Will
On Mon, Jan 6, 2020 at 6:39 PM Will Deacon will@kernel.org wrote:
I also ran the native and compat selftests but, unfortunately, they all pass even without this patch. Do you reckon it would be possible to update them to check the tls pointer?
Here's the program I used for testing on arm64. I considered adding it to the selftests but there is no portable way of reading the TLS register on all architectures.
#include <sys/syscall.h> #include <unistd.h> #include <stdio.h> #include <stdint.h>
#define __NR_clone3 435 struct clone_args { uint64_t flags; uint64_t pidfd; uint64_t child_tid; uint64_t parent_tid; uint64_t exit_signal; uint64_t stack; uint64_t stack_size; uint64_t tls; };
#define USE_CLONE3
int main() { printf("Before fork: tp = %p\n", __builtin_thread_pointer()); #ifdef USE_CLONE3 struct clone_args args = { .flags = CLONE_SETTLS, .tls = (uint64_t)__builtin_thread_pointer(), }; int ret = syscall(__NR_clone3, &args, sizeof(args)); #else int ret = syscall(__NR_clone, CLONE_SETTLS, 0, 0, __builtin_thread_pointer(), 0); #endif printf("Fork returned %d, tp = %p\n", ret, __builtin_thread_pointer()); }
[Cc Kees in case he knows something about where arch specific tests live or whether we have a framework for this]
On Mon, Jan 06, 2020 at 07:03:32PM +0100, Amanieu d'Antras wrote:
On Mon, Jan 6, 2020 at 6:39 PM Will Deacon will@kernel.org wrote:
I also ran the native and compat selftests but, unfortunately, they all pass even without this patch. Do you reckon it would be possible to update them to check the tls pointer?
Here's the program I used for testing on arm64. I considered adding it to the selftests but there is no portable way of reading the TLS register on all architectures.
I'm not saying you need to do this right now. It feels like we must've run into the "this is architecture specific"-and-we-want-to-test-this issue before... Do we have a place where architecture specific selftests live?
#include <sys/syscall.h> #include <unistd.h> #include <stdio.h> #include <stdint.h>
#define __NR_clone3 435 struct clone_args { uint64_t flags; uint64_t pidfd; uint64_t child_tid; uint64_t parent_tid; uint64_t exit_signal; uint64_t stack; uint64_t stack_size; uint64_t tls; };
#define USE_CLONE3
int main() { printf("Before fork: tp = %p\n", __builtin_thread_pointer()); #ifdef USE_CLONE3 struct clone_args args = { .flags = CLONE_SETTLS, .tls = (uint64_t)__builtin_thread_pointer(), }; int ret = syscall(__NR_clone3, &args, sizeof(args)); #else int ret = syscall(__NR_clone, CLONE_SETTLS, 0, 0, __builtin_thread_pointer(), 0); #endif printf("Fork returned %d, tp = %p\n", ret, __builtin_thread_pointer()); }
On Tue, Jan 07, 2020 at 10:02:27AM +0100, Christian Brauner wrote:
[Cc Kees in case he knows something about where arch specific tests live or whether we have a framework for this]
On Mon, Jan 06, 2020 at 07:03:32PM +0100, Amanieu d'Antras wrote:
On Mon, Jan 6, 2020 at 6:39 PM Will Deacon will@kernel.org wrote:
I also ran the native and compat selftests but, unfortunately, they all pass even without this patch. Do you reckon it would be possible to update them to check the tls pointer?
Here's the program I used for testing on arm64. I considered adding it to the selftests but there is no portable way of reading the TLS register on all architectures.
I'm not saying you need to do this right now.
Agreed, these patches should be merged in their current state and my ack stands for that.
It feels like we must've run into the "this is architecture specific"-and-we-want-to-test-this issue before... Do we have a place where architecture specific selftests live?
For arch-specific selftests there are tools/testing/selftests/$ARCH directories, although in this case maybe it's better to have an #ifdef in a header so that architectures with __builtin_thread_pointer can use that.
Will
On Tue, Jan 07, 2020 at 05:45:09PM +0000, Will Deacon wrote:
On Tue, Jan 07, 2020 at 10:02:27AM +0100, Christian Brauner wrote:
[Cc Kees in case he knows something about where arch specific tests live or whether we have a framework for this] [...] It feels like we must've run into the "this is architecture specific"-and-we-want-to-test-this issue before... Do we have a place where architecture specific selftests live?
For arch-specific selftests there are tools/testing/selftests/$ARCH directories, although in this case maybe it's better to have an #ifdef in a header so that architectures with __builtin_thread_pointer can use that.
Yup, I agree: that's the current best-practice for arch-specific selftests.
On Tue, Jan 07, 2020 at 10:12:39AM -0800, Kees Cook wrote:
On Tue, Jan 07, 2020 at 05:45:09PM +0000, Will Deacon wrote:
On Tue, Jan 07, 2020 at 10:02:27AM +0100, Christian Brauner wrote:
[Cc Kees in case he knows something about where arch specific tests live or whether we have a framework for this] [...] It feels like we must've run into the "this is architecture specific"-and-we-want-to-test-this issue before... Do we have a place where architecture specific selftests live?
For arch-specific selftests there are tools/testing/selftests/$ARCH directories, although in this case maybe it's better to have an #ifdef in a header so that architectures with __builtin_thread_pointer can use that.
Yup, I agree: that's the current best-practice for arch-specific selftests.
Thanks! I think using #ifdef in this case with __builtin_thread_pointer sounds good. So the tests can be moved into the clone3() test-suite for those architectures.
Christian
On Tue, Jan 07, 2020 at 05:45:09PM +0000, Will Deacon wrote:
On Tue, Jan 07, 2020 at 10:02:27AM +0100, Christian Brauner wrote:
[Cc Kees in case he knows something about where arch specific tests live or whether we have a framework for this]
On Mon, Jan 06, 2020 at 07:03:32PM +0100, Amanieu d'Antras wrote:
On Mon, Jan 6, 2020 at 6:39 PM Will Deacon will@kernel.org wrote:
I also ran the native and compat selftests but, unfortunately, they all pass even without this patch. Do you reckon it would be possible to update them to check the tls pointer?
Here's the program I used for testing on arm64. I considered adding it to the selftests but there is no portable way of reading the TLS register on all architectures.
I'm not saying you need to do this right now.
Agreed, these patches should be merged in their current state and my ack stands for that.
Oh yeah, that's how I took your Ack. Thanks! :)
It feels like we must've run into the "this is architecture specific"-and-we-want-to-test-this issue before... Do we have a place where architecture specific selftests live?
For arch-specific selftests there are tools/testing/selftests/$ARCH directories, although in this case maybe it's better to have an #ifdef in a header so that architectures with __builtin_thread_pointer can use that.
Yeah, I think the #ifdef approach might make the most sense.
Christian
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x --- arch/arm/Kconfig | 1 + arch/arm/kernel/process.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index ba75e3661a41..96dab76da3b3 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -72,6 +72,7 @@ config ARM select HAVE_ARM_SMCCC if CPU_V7 select HAVE_EBPF_JIT if !CPU_ENDIAN_BE32 select HAVE_CONTEXT_TRACKING + select HAVE_COPY_THREAD_TLS select HAVE_C_RECORDMCOUNT select HAVE_DEBUG_KMEMLEAK select HAVE_DMA_CONTIGUOUS if MMU diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c index cea1c27c29cb..46e478fb5ea2 100644 --- a/arch/arm/kernel/process.c +++ b/arch/arm/kernel/process.c @@ -226,8 +226,8 @@ void release_thread(struct task_struct *dead_task) asmlinkage void ret_from_fork(void) __asm__("ret_from_fork");
int -copy_thread(unsigned long clone_flags, unsigned long stack_start, - unsigned long stk_sz, struct task_struct *p) +copy_thread_tls(unsigned long clone_flags, unsigned long stack_start, + unsigned long stk_sz, struct task_struct *p, unsigned long tls) { struct thread_info *thread = task_thread_info(p); struct pt_regs *childregs = task_pt_regs(p); @@ -261,7 +261,7 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start, clear_ptrace_hw_breakpoint(p);
if (clone_flags & CLONE_SETTLS) - thread->tp_value[0] = childregs->ARM_r3; + thread->tp_value[0] = tls; thread->tp_value[1] = get_tpuser();
thread_notify(THREAD_NOTIFY_COPY, thread);
On Thu, Jan 02, 2020 at 06:24:09PM +0100, Amanieu d'Antras wrote:
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x
Again, looks good to me but I'd like an ack from someone closer to the architecture itself. Acked-by: Christian Brauner christian.brauner@ubuntu.com
arch/arm/Kconfig | 1 + arch/arm/kernel/process.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index ba75e3661a41..96dab76da3b3 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -72,6 +72,7 @@ config ARM select HAVE_ARM_SMCCC if CPU_V7 select HAVE_EBPF_JIT if !CPU_ENDIAN_BE32 select HAVE_CONTEXT_TRACKING
- select HAVE_COPY_THREAD_TLS select HAVE_C_RECORDMCOUNT select HAVE_DEBUG_KMEMLEAK select HAVE_DMA_CONTIGUOUS if MMU
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c index cea1c27c29cb..46e478fb5ea2 100644 --- a/arch/arm/kernel/process.c +++ b/arch/arm/kernel/process.c @@ -226,8 +226,8 @@ void release_thread(struct task_struct *dead_task) asmlinkage void ret_from_fork(void) __asm__("ret_from_fork"); int -copy_thread(unsigned long clone_flags, unsigned long stack_start,
unsigned long stk_sz, struct task_struct *p)+copy_thread_tls(unsigned long clone_flags, unsigned long stack_start,
unsigned long stk_sz, struct task_struct *p, unsigned long tls){ struct thread_info *thread = task_thread_info(p); struct pt_regs *childregs = task_pt_regs(p); @@ -261,7 +261,7 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start, clear_ptrace_hw_breakpoint(p); if (clone_flags & CLONE_SETTLS)
thread->tp_value[0] = childregs->ARM_r3;
thread->tp_value[0] = tls;thread_notify(THREAD_NOTIFY_COPY, thread); -- 2.24.1
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-parisc@vger.kernel.org Cc: stable@vger.kernel.org # 5.3.x --- arch/parisc/Kconfig | 1 + arch/parisc/kernel/process.c | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig index b16237c95ea3..0c29d6cb2c8d 100644 --- a/arch/parisc/Kconfig +++ b/arch/parisc/Kconfig @@ -62,6 +62,7 @@ config PARISC select HAVE_FTRACE_MCOUNT_RECORD if HAVE_DYNAMIC_FTRACE select HAVE_KPROBES_ON_FTRACE select HAVE_DYNAMIC_FTRACE_WITH_REGS + select HAVE_COPY_THREAD_TLS
help The PA-RISC microprocessor is designed by Hewlett-Packard and used diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c index ecc5c2771208..230a6422b99f 100644 --- a/arch/parisc/kernel/process.c +++ b/arch/parisc/kernel/process.c @@ -208,8 +208,8 @@ arch_initcall(parisc_idle_init); * Copy architecture-specific thread state */ int -copy_thread(unsigned long clone_flags, unsigned long usp, - unsigned long kthread_arg, struct task_struct *p) +copy_thread_tls(unsigned long clone_flags, unsigned long usp, + unsigned long kthread_arg, struct task_struct *p, unsigned long tls) { struct pt_regs *cregs = &(p->thread.regs); void *stack = task_stack_page(p); @@ -254,9 +254,9 @@ copy_thread(unsigned long clone_flags, unsigned long usp, cregs->ksp = (unsigned long)stack + THREAD_SZ_ALGN + FRAME_SIZE; cregs->kpc = (unsigned long) &child_return;
- /* Setup thread TLS area from the 4th parameter in clone */ + /* Setup thread TLS area */ if (clone_flags & CLONE_SETTLS) - cregs->cr27 = cregs->gr[23]; + cregs->cr27 = tls; }
return 0;
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-riscv@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x --- arch/riscv/Kconfig | 1 + arch/riscv/kernel/process.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index d8efbaa78d67..d6c3d44f96b5 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -64,6 +64,7 @@ config RISCV select SPARSEMEM_STATIC if 32BIT select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU select HAVE_ARCH_MMAP_RND_BITS if MMU + select HAVE_COPY_THREAD_TLS
config ARCH_MMAP_RND_BITS_MIN default 18 if 64BIT diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c index 95a3031e5c7c..817cf7b0974c 100644 --- a/arch/riscv/kernel/process.c +++ b/arch/riscv/kernel/process.c @@ -99,8 +99,8 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) return 0; }
-int copy_thread(unsigned long clone_flags, unsigned long usp, - unsigned long arg, struct task_struct *p) +int copy_thread_tls(unsigned long clone_flags, unsigned long usp, + unsigned long arg, struct task_struct *p, unsigned long tls) { struct pt_regs *childregs = task_pt_regs(p);
@@ -121,7 +121,7 @@ int copy_thread(unsigned long clone_flags, unsigned long usp, if (usp) /* User fork */ childregs->sp = usp; if (clone_flags & CLONE_SETTLS) - childregs->tp = childregs->a5; + childregs->tp = tls; childregs->a0 = 0; /* Return value of fork() */ p->thread.ra = (unsigned long)ret_from_fork; }
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-xtensa@linux-xtensa.org Cc: stable@vger.kernel.org # 5.3.x --- arch/xtensa/Kconfig | 1 + arch/xtensa/kernel/process.c | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig index 4a3fa295d8fe..296c5324dace 100644 --- a/arch/xtensa/Kconfig +++ b/arch/xtensa/Kconfig @@ -24,6 +24,7 @@ config XTENSA select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL select HAVE_ARCH_KASAN if MMU && !XIP_KERNEL select HAVE_ARCH_TRACEHOOK + select HAVE_COPY_THREAD_TLS select HAVE_DEBUG_KMEMLEAK select HAVE_DMA_CONTIGUOUS select HAVE_EXIT_THREAD diff --git a/arch/xtensa/kernel/process.c b/arch/xtensa/kernel/process.c index 9e1c49134c07..3edecc41ef8c 100644 --- a/arch/xtensa/kernel/process.c +++ b/arch/xtensa/kernel/process.c @@ -202,8 +202,9 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) * involved. Much simpler to just not copy those live frames across. */
-int copy_thread(unsigned long clone_flags, unsigned long usp_thread_fn, - unsigned long thread_fn_arg, struct task_struct *p) +int copy_thread_tls(unsigned long clone_flags, unsigned long usp_thread_fn, + unsigned long thread_fn_arg, struct task_struct *p, + unsigned long tls) { struct pt_regs *childregs = task_pt_regs(p);
@@ -266,9 +267,8 @@ int copy_thread(unsigned long clone_flags, unsigned long usp_thread_fn,
childregs->syscall = regs->syscall;
- /* The thread pointer is passed in the '4th argument' (= a5) */ if (clone_flags & CLONE_SETTLS) - childregs->threadptr = childregs->areg[5]; + childregs->threadptr = tls; } else { p->thread.ra = MAKE_RA_FOR_CALL( (unsigned long)ret_from_kernel_thread, 1);
copy_thread implementations handle CLONE_SETTLS by reading the TLS value from the registers containing the syscall arguments for clone. This doesn't work with clone3 since the TLS value is passed in clone_args instead.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: stable@vger.kernel.org # 5.3.x --- kernel/fork.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/kernel/fork.c b/kernel/fork.c index 2508a4f238a3..080809560072 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -2578,6 +2578,16 @@ SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp, #endif
#ifdef __ARCH_WANT_SYS_CLONE3 + +/* + * copy_thread implementations handle CLONE_SETTLS by reading the TLS value from + * the registers containing the syscall arguments for clone. This doesn't work + * with clone3 since the TLS value is passed in clone_args instead. + */ +#ifndef CONFIG_HAVE_COPY_THREAD_TLS +#error clone3 requires copy_thread_tls support in arch +#endif + noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs, struct clone_args __user *uargs, size_t usize)
On Thu, Jan 02, 2020 at 06:24:13PM +0100, Amanieu d'Antras wrote:
copy_thread implementations handle CLONE_SETTLS by reading the TLS value from the registers containing the syscall arguments for clone. This doesn't work with clone3 since the TLS value is passed in clone_args instead.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: stable@vger.kernel.org # 5.3.x
I'm in favor of this change. But we need to make sure that any arch which now has ARCH_WANTS_SYS_CLONE3 set but doesn't implement copy_thread_tls() is fixed.
Once all architectures have clone3() support - and there are just a few by now (IA64 comes to mind) this means we should also be able to get rid of of copy_thread() completely. That seems desirable to me as it makes the codepaths easier to follow.
Reviewed-by: Christian Brauner christian.brauner@ubuntu.com
On Thu, Jan 2, 2020 at 7:09 PM Christian Brauner christian.brauner@ubuntu.com wrote:
I'm in favor of this change. But we need to make sure that any arch which now has ARCH_WANTS_SYS_CLONE3 set but doesn't implement copy_thread_tls() is fixed.
Once all architectures have clone3() support - and there are just a few by now (IA64 comes to mind) this means we should also be able to get rid of of copy_thread() completely. That seems desirable to me as it makes the codepaths easier to follow.
I've already implemented copy_thread_tls for all arches that currently have ARCH_WANTS_SYS_CLONE3 in the previous 5 patches. The #error is there so that any future arches that wire up clone3 don't forget to implement copy_thread_tls as well.
On Thu, Jan 02, 2020 at 07:19:11PM +0100, Amanieu d'Antras wrote:
On Thu, Jan 2, 2020 at 7:09 PM Christian Brauner christian.brauner@ubuntu.com wrote:
I'm in favor of this change. But we need to make sure that any arch which now has ARCH_WANTS_SYS_CLONE3 set but doesn't implement copy_thread_tls() is fixed.
Once all architectures have clone3() support - and there are just a few by now (IA64 comes to mind) this means we should also be able to get rid of of copy_thread() completely. That seems desirable to me as it makes the codepaths easier to follow.
I've already implemented copy_thread_tls for all arches that currently have ARCH_WANTS_SYS_CLONE3 in the previous 5 patches. The #error is there so that any future arches that wire up clone3 don't forget to implement copy_thread_tls as well.
Right, my point is: Once _all_ arches have ARCH_WANT_SYS_CLONE3 they also must implement copy_thread_tls at which point we can remove: - ARCH_WANT_SYS_CLONE3 ifdefines - copy_thread() We can't do this right now because e.g. IA64 does not set ARCH_WANT_SYS_CLONE3 and also does not select HAVE_COPY_THREAD_TLS.
Christian
On Thu, Jan 02, 2020 at 06:24:06PM +0100, Amanieu d'Antras wrote:
The clone3 syscall is currently broken when used with CLONE_SETTLS on all architectures that don't have an implementation of copy_thread_tls. The old copy_thread function handles CLONE_SETTLS by reading the new TLS value from pt_regs containing the clone syscall parameters. Since clone3 passes the TLS value in clone_args, this results in the TLS register being initialized to a garbage value.
This patch series implements copy_thread_tls on all architectures that currently define __ARCH_WANT_SYS_CLONE3 and adds a compile-time check to ensure that any architecture that enables clone3 in the future also implements copy_thread_tls.
I have also included a minor fix for the arm64 uapi headers which caused __NR_clone3 to be missing from the exported user headers.
I have only tested this on arm64, but the copy_thread_tls implementations for the various architectures are fairly straightforward.
The series looks straightforward to me but I'd like a few Acks from some of the arch maintainers before taking this.
Acked-by: Christian Brauner christian.brauner@ubuntu.com
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-um@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x --- arch/um/Kconfig | 1 + arch/um/include/asm/ptrace-generic.h | 2 +- arch/um/kernel/process.c | 6 +++--- arch/x86/um/tls_32.c | 6 ++---- arch/x86/um/tls_64.c | 7 +++---- 5 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/arch/um/Kconfig b/arch/um/Kconfig index 2a6d04fcb3e9..6f0edd0c0220 100644 --- a/arch/um/Kconfig +++ b/arch/um/Kconfig @@ -14,6 +14,7 @@ config UML select HAVE_FUTEX_CMPXCHG if FUTEX select HAVE_DEBUG_KMEMLEAK select HAVE_DEBUG_BUGVERBOSE + select HAVE_COPY_THREAD_TLS select GENERIC_IRQ_SHOW select GENERIC_CPU_DEVICES select GENERIC_CLOCKEVENTS diff --git a/arch/um/include/asm/ptrace-generic.h b/arch/um/include/asm/ptrace-generic.h index 81c647ef9c6c..adf91ef553ae 100644 --- a/arch/um/include/asm/ptrace-generic.h +++ b/arch/um/include/asm/ptrace-generic.h @@ -36,7 +36,7 @@ extern long subarch_ptrace(struct task_struct *child, long request, extern unsigned long getreg(struct task_struct *child, int regno); extern int putreg(struct task_struct *child, int regno, unsigned long value);
-extern int arch_copy_tls(struct task_struct *new); +extern int arch_set_tls(struct task_struct *new, unsigned long tls); extern void clear_flushed_tls(struct task_struct *task); extern int syscall_trace_enter(struct pt_regs *regs); extern void syscall_trace_leave(struct pt_regs *regs); diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c index 263a8f069133..17045e7211bf 100644 --- a/arch/um/kernel/process.c +++ b/arch/um/kernel/process.c @@ -153,8 +153,8 @@ void fork_handler(void) userspace(¤t->thread.regs.regs, current_thread_info()->aux_fp_regs); }
-int copy_thread(unsigned long clone_flags, unsigned long sp, - unsigned long arg, struct task_struct * p) +int copy_thread_tls(unsigned long clone_flags, unsigned long sp, + unsigned long arg, struct task_struct * p, unsigned long tls) { void (*handler)(void); int kthread = current->flags & PF_KTHREAD; @@ -188,7 +188,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, * Set a new TLS for the child thread? */ if (clone_flags & CLONE_SETTLS) - ret = arch_copy_tls(p); + ret = arch_set_tls(p, tls); }
return ret; diff --git a/arch/x86/um/tls_32.c b/arch/x86/um/tls_32.c index 5bd949da7a4a..ac8eee093f9c 100644 --- a/arch/x86/um/tls_32.c +++ b/arch/x86/um/tls_32.c @@ -215,14 +215,12 @@ static int set_tls_entry(struct task_struct* task, struct user_desc *info, return 0; }
-int arch_copy_tls(struct task_struct *new) +int arch_set_tls(struct task_struct *new, unsigned long tls) { struct user_desc info; int idx, ret = -EFAULT;
- if (copy_from_user(&info, - (void __user *) UPT_SI(&new->thread.regs.regs), - sizeof(info))) + if (copy_from_user(&info, (void __user *) tls, sizeof(info))) goto out;
ret = -EINVAL; diff --git a/arch/x86/um/tls_64.c b/arch/x86/um/tls_64.c index 3a621e0d3925..ebd3855d9b13 100644 --- a/arch/x86/um/tls_64.c +++ b/arch/x86/um/tls_64.c @@ -6,14 +6,13 @@ void clear_flushed_tls(struct task_struct *task) { }
-int arch_copy_tls(struct task_struct *t) +int arch_set_tls(struct task_struct *t, unsigned long tls) { /* * If CLONE_SETTLS is set, we need to save the thread id - * (which is argument 5, child_tid, of clone) so it can be set - * during context switches. + * so it can be set during context switches. */ - t->thread.arch.fs = t->thread.regs.regs.gp[R8 / sizeof(long)]; + t->thread.arch.fs = tls;
return 0; }
On Sat, Jan 04, 2020 at 01:39:30PM +0100, Amanieu d'Antras wrote:
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-um@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x
Thanks. I'm picking this up as part of the copy_thread_tls() series. (Leaving the patch in tact so people can Ack right here if they want to.) If I could get an Ack from one of the maintainers that would be great; see https://lore.kernel.org/lkml/20200102172413.654385-1-amanieu@gmail.com for more context.
Acked-by: Christian Brauner christian.brauner@ubuntu.com
arch/um/Kconfig | 1 + arch/um/include/asm/ptrace-generic.h | 2 +- arch/um/kernel/process.c | 6 +++--- arch/x86/um/tls_32.c | 6 ++---- arch/x86/um/tls_64.c | 7 +++---- 5 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/arch/um/Kconfig b/arch/um/Kconfig index 2a6d04fcb3e9..6f0edd0c0220 100644 --- a/arch/um/Kconfig +++ b/arch/um/Kconfig @@ -14,6 +14,7 @@ config UML select HAVE_FUTEX_CMPXCHG if FUTEX select HAVE_DEBUG_KMEMLEAK select HAVE_DEBUG_BUGVERBOSE
- select HAVE_COPY_THREAD_TLS select GENERIC_IRQ_SHOW select GENERIC_CPU_DEVICES select GENERIC_CLOCKEVENTS
diff --git a/arch/um/include/asm/ptrace-generic.h b/arch/um/include/asm/ptrace-generic.h index 81c647ef9c6c..adf91ef553ae 100644 --- a/arch/um/include/asm/ptrace-generic.h +++ b/arch/um/include/asm/ptrace-generic.h @@ -36,7 +36,7 @@ extern long subarch_ptrace(struct task_struct *child, long request, extern unsigned long getreg(struct task_struct *child, int regno); extern int putreg(struct task_struct *child, int regno, unsigned long value); -extern int arch_copy_tls(struct task_struct *new); +extern int arch_set_tls(struct task_struct *new, unsigned long tls); extern void clear_flushed_tls(struct task_struct *task); extern int syscall_trace_enter(struct pt_regs *regs); extern void syscall_trace_leave(struct pt_regs *regs); diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c index 263a8f069133..17045e7211bf 100644 --- a/arch/um/kernel/process.c +++ b/arch/um/kernel/process.c @@ -153,8 +153,8 @@ void fork_handler(void) userspace(¤t->thread.regs.regs, current_thread_info()->aux_fp_regs); } -int copy_thread(unsigned long clone_flags, unsigned long sp,
unsigned long arg, struct task_struct * p)+int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
unsigned long arg, struct task_struct * p, unsigned long tls){ void (*handler)(void); int kthread = current->flags & PF_KTHREAD; @@ -188,7 +188,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, * Set a new TLS for the child thread? */ if (clone_flags & CLONE_SETTLS)
ret = arch_copy_tls(p);
ret = arch_set_tls(p, tls);return ret; diff --git a/arch/x86/um/tls_32.c b/arch/x86/um/tls_32.c index 5bd949da7a4a..ac8eee093f9c 100644 --- a/arch/x86/um/tls_32.c +++ b/arch/x86/um/tls_32.c @@ -215,14 +215,12 @@ static int set_tls_entry(struct task_struct* task, struct user_desc *info, return 0; } -int arch_copy_tls(struct task_struct *new) +int arch_set_tls(struct task_struct *new, unsigned long tls) { struct user_desc info; int idx, ret = -EFAULT;
- if (copy_from_user(&info,
(void __user *) UPT_SI(&new->thread.regs.regs),sizeof(info)))
- if (copy_from_user(&info, (void __user *) tls, sizeof(info))) goto out;
ret = -EINVAL; diff --git a/arch/x86/um/tls_64.c b/arch/x86/um/tls_64.c index 3a621e0d3925..ebd3855d9b13 100644 --- a/arch/x86/um/tls_64.c +++ b/arch/x86/um/tls_64.c @@ -6,14 +6,13 @@ void clear_flushed_tls(struct task_struct *task) { } -int arch_copy_tls(struct task_struct *t) +int arch_set_tls(struct task_struct *t, unsigned long tls) { /* * If CLONE_SETTLS is set, we need to save the thread id
* (which is argument 5, child_tid, of clone) so it can be set* during context switches.
* so it can be set during context switches.
- t->thread.arch.fs = t->thread.regs.regs.gp[R8 / sizeof(long)];
- t->thread.arch.fs = tls;
return 0; } -- 2.24.1
On Sun, Jan 05, 2020 at 04:19:28PM +0100, Christian Brauner wrote:
On Sat, Jan 04, 2020 at 01:39:30PM +0100, Amanieu d'Antras wrote:
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-um@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x
Thanks. I'm picking this up as part of the copy_thread_tls() series. (Leaving the patch in tact so people can Ack right here if they want to.) If I could get an Ack from one of the maintainers that would be great; see https://lore.kernel.org/lkml/20200102172413.654385-1-amanieu@gmail.com for more context.
Acked-by: Christian Brauner christian.brauner@ubuntu.com
I've this up as part of the series link in above ^^ and moved it into https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=clo...
If I hear no objections I'll merge into into my fixes tree today or tomorrow.
An Ack from one of the maintainers would still be appreciated.
Thanks! Christian
On Tue, Jan 7, 2020 at 1:36 PM Christian Brauner christian.brauner@ubuntu.com wrote:
On Sun, Jan 05, 2020 at 04:19:28PM +0100, Christian Brauner wrote:
On Sat, Jan 04, 2020 at 01:39:30PM +0100, Amanieu d'Antras wrote:
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-um@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x
Thanks. I'm picking this up as part of the copy_thread_tls() series. (Leaving the patch in tact so people can Ack right here if they want to.) If I could get an Ack from one of the maintainers that would be great; see https://lore.kernel.org/lkml/20200102172413.654385-1-amanieu@gmail.com for more context.
Acked-by: Christian Brauner christian.brauner@ubuntu.com
I've this up as part of the series link in above ^^ and moved it into https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=clo...
If I hear no objections I'll merge into into my fixes tree today or tomorrow.
An Ack from one of the maintainers would still be appreciated.
For sure too late, but better than nothing? ;-)
Acked-by: Richard Weinberger richard@nod.at
On Fri, Jan 17, 2020 at 01:12:31AM +0100, Richard Weinberger wrote:
On Tue, Jan 7, 2020 at 1:36 PM Christian Brauner christian.brauner@ubuntu.com wrote:
On Sun, Jan 05, 2020 at 04:19:28PM +0100, Christian Brauner wrote:
On Sat, Jan 04, 2020 at 01:39:30PM +0100, Amanieu d'Antras wrote:
This is required for clone3 which passes the TLS value through a struct rather than a register.
Signed-off-by: Amanieu d'Antras amanieu@gmail.com Cc: linux-um@lists.infradead.org Cc: stable@vger.kernel.org # 5.3.x
Thanks. I'm picking this up as part of the copy_thread_tls() series. (Leaving the patch in tact so people can Ack right here if they want to.) If I could get an Ack from one of the maintainers that would be great; see https://lore.kernel.org/lkml/20200102172413.654385-1-amanieu@gmail.com for more context.
Acked-by: Christian Brauner christian.brauner@ubuntu.com
I've this up as part of the series link in above ^^ and moved it into https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=clo...
If I hear no objections I'll merge into into my fixes tree today or tomorrow.
An Ack from one of the maintainers would still be appreciated.
For sure too late, but better than nothing? ;-)
Acked-by: Richard Weinberger richard@nod.at
Still worth having it. :)
Thanks! Christian
On Thu, Jan 02, 2020 at 06:24:06PM +0100, Amanieu d'Antras wrote:
The clone3 syscall is currently broken when used with CLONE_SETTLS on all architectures that don't have an implementation of copy_thread_tls. The old copy_thread function handles CLONE_SETTLS by reading the new TLS value from pt_regs containing the clone syscall parameters. Since clone3 passes the TLS value in clone_args, this results in the TLS register being initialized to a garbage value.
This patch series implements copy_thread_tls on all architectures that currently define __ARCH_WANT_SYS_CLONE3 and adds a compile-time check to ensure that any architecture that enables clone3 in the future also implements copy_thread_tls.
I have also included a minor fix for the arm64 uapi headers which caused __NR_clone3 to be missing from the exported user headers.
I have only tested this on arm64, but the copy_thread_tls implementations for the various architectures are fairly straightforward.
I've picked up this series and moved it into https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=clo...
If I hear no objections I'll merge into into my fixes tree today or tomorrow.
Thanks! Christian
linux-stable-mirror@lists.linaro.org
-
Amanieu d'Antras -
Arnd Bergmann -
Christian Brauner -
Kees Cook -
Richard Weinberger -
Will Deacon