From: Kees Cook <keescook@chromium.org>
To: Russell King <linux@armlinux.org.uk>
Cc: Kees Cook <keescook@chromium.org>,
Arnd Bergmann <arnd@kernel.org>,
Lecopzer Chen <lecopzer.chen@mediatek.com>,
Oleg Nesterov <oleg@redhat.com>,
linux-arm-kernel@lists.infradead.org,
Linus Walleij <linus.walleij@linaro.org>,
"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>,
linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
linux-hardening@vger.kernel.org
Subject: [PATCH] ARM: ptrace: Restore syscall skipping and restart while tracing
Date: Fri, 4 Aug 2023 00:10:53 -0700 [thread overview]
Message-ID: <20230804071045.never.134-kees@kernel.org> (raw)
Since commit 4e57a4ddf6b0 ("ARM: 9107/1: syscall: always store
thread_info->abi_syscall"), the seccomp selftests "syscall_errno",
"syscall_faked", and "syscall_restart" have been broken. This was
related to two issues:
- seccomp and PTRACE depend on using the special value of "-1" for
skipping syscalls. This value wasn't working because it was getting
masked by __NR_SYSCALL_MASK in both PTRACE_SET_SYSCALL and
get_syscall_nr().
- the syscall entry label "local_restart" is used for resuming syscalls
interrupted by signals, but the updated syscall number (in scno) was
not being stored in current_thread_info()->abi_syscall, causing traced
syscall restarting to fail.
Explicitly test for -1 in PTRACE_SET_SYSCALL and get_syscall_nr(),
leaving it exposed when present, allowing tracers to skip syscalls
again.
Move the AEABI-only assignment of current_thread_info()->abi_syscall
after the "local_restart" label to allow tracers to survive syscall
restarting.
Cc: Russell King <linux@armlinux.org.uk>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: Lecopzer Chen <lecopzer.chen@mediatek.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: linux-arm-kernel@lists.infradead.org
Fixes: 4e57a4ddf6b0 ("ARM: 9107/1: syscall: always store thread_info->abi_syscall")
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Note that I haven't tested OABI at all, and AEABI+OABI_COMPAT doesn't
work with seccomp. I booted an AEABI system under AEABI+OABI_COMPAT,
but I wasn't able to test tracing...
---
arch/arm/include/asm/syscall.h | 3 +++
arch/arm/kernel/entry-common.S | 5 +++--
arch/arm/kernel/ptrace.c | 5 +++--
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/syscall.h b/arch/arm/include/asm/syscall.h
index dfeed440254a..fe4326d938c1 100644
--- a/arch/arm/include/asm/syscall.h
+++ b/arch/arm/include/asm/syscall.h
@@ -25,6 +25,9 @@ static inline int syscall_get_nr(struct task_struct *task,
if (IS_ENABLED(CONFIG_AEABI) && !IS_ENABLED(CONFIG_OABI_COMPAT))
return task_thread_info(task)->abi_syscall;
+ if (task_thread_info(task)->abi_syscall == -1)
+ return -1;
+
return task_thread_info(task)->abi_syscall & __NR_SYSCALL_MASK;
}
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index bcc4c9ec3aa4..08bd624e4c6f 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -246,8 +246,6 @@ ENTRY(vector_swi)
bic scno, scno, #0xff000000 @ mask off SWI op-code
str scno, [tsk, #TI_ABI_SYSCALL]
eor scno, scno, #__NR_SYSCALL_BASE @ check OS number
-#else
- str scno, [tsk, #TI_ABI_SYSCALL]
#endif
/*
* Reload the registers that may have been corrupted on entry to
@@ -256,6 +254,9 @@ ENTRY(vector_swi)
TRACE( ldmia sp, {r0 - r3} )
local_restart:
+#if defined(CONFIG_AEABI) && !defined(CONFIG_OABI_COMPAT)
+ str scno, [tsk, #TI_ABI_SYSCALL] @ store scno for syscall restart
+#endif
ldr r10, [tsk, #TI_FLAGS] @ check for syscall tracing
stmdb sp!, {r4, r5} @ push fifth and sixth args
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 2d8e2516906b..fef32d73f912 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -783,8 +783,9 @@ long arch_ptrace(struct task_struct *child, long request,
break;
case PTRACE_SET_SYSCALL:
- task_thread_info(child)->abi_syscall = data &
- __NR_SYSCALL_MASK;
+ if (data != -1)
+ data &= __NR_SYSCALL_MASK;
+ task_thread_info(child)->abi_syscall = data;
ret = 0;
break;
--
2.34.1
WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org>
To: Russell King <linux@armlinux.org.uk>
Cc: Kees Cook <keescook@chromium.org>,
Arnd Bergmann <arnd@kernel.org>,
Lecopzer Chen <lecopzer.chen@mediatek.com>,
Oleg Nesterov <oleg@redhat.com>,
linux-arm-kernel@lists.infradead.org,
Linus Walleij <linus.walleij@linaro.org>,
"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>,
linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
linux-hardening@vger.kernel.org
Subject: [PATCH] ARM: ptrace: Restore syscall skipping and restart while tracing
Date: Fri, 4 Aug 2023 00:10:53 -0700 [thread overview]
Message-ID: <20230804071045.never.134-kees@kernel.org> (raw)
Since commit 4e57a4ddf6b0 ("ARM: 9107/1: syscall: always store
thread_info->abi_syscall"), the seccomp selftests "syscall_errno",
"syscall_faked", and "syscall_restart" have been broken. This was
related to two issues:
- seccomp and PTRACE depend on using the special value of "-1" for
skipping syscalls. This value wasn't working because it was getting
masked by __NR_SYSCALL_MASK in both PTRACE_SET_SYSCALL and
get_syscall_nr().
- the syscall entry label "local_restart" is used for resuming syscalls
interrupted by signals, but the updated syscall number (in scno) was
not being stored in current_thread_info()->abi_syscall, causing traced
syscall restarting to fail.
Explicitly test for -1 in PTRACE_SET_SYSCALL and get_syscall_nr(),
leaving it exposed when present, allowing tracers to skip syscalls
again.
Move the AEABI-only assignment of current_thread_info()->abi_syscall
after the "local_restart" label to allow tracers to survive syscall
restarting.
Cc: Russell King <linux@armlinux.org.uk>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: Lecopzer Chen <lecopzer.chen@mediatek.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: linux-arm-kernel@lists.infradead.org
Fixes: 4e57a4ddf6b0 ("ARM: 9107/1: syscall: always store thread_info->abi_syscall")
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Note that I haven't tested OABI at all, and AEABI+OABI_COMPAT doesn't
work with seccomp. I booted an AEABI system under AEABI+OABI_COMPAT,
but I wasn't able to test tracing...
---
arch/arm/include/asm/syscall.h | 3 +++
arch/arm/kernel/entry-common.S | 5 +++--
arch/arm/kernel/ptrace.c | 5 +++--
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/syscall.h b/arch/arm/include/asm/syscall.h
index dfeed440254a..fe4326d938c1 100644
--- a/arch/arm/include/asm/syscall.h
+++ b/arch/arm/include/asm/syscall.h
@@ -25,6 +25,9 @@ static inline int syscall_get_nr(struct task_struct *task,
if (IS_ENABLED(CONFIG_AEABI) && !IS_ENABLED(CONFIG_OABI_COMPAT))
return task_thread_info(task)->abi_syscall;
+ if (task_thread_info(task)->abi_syscall == -1)
+ return -1;
+
return task_thread_info(task)->abi_syscall & __NR_SYSCALL_MASK;
}
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index bcc4c9ec3aa4..08bd624e4c6f 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -246,8 +246,6 @@ ENTRY(vector_swi)
bic scno, scno, #0xff000000 @ mask off SWI op-code
str scno, [tsk, #TI_ABI_SYSCALL]
eor scno, scno, #__NR_SYSCALL_BASE @ check OS number
-#else
- str scno, [tsk, #TI_ABI_SYSCALL]
#endif
/*
* Reload the registers that may have been corrupted on entry to
@@ -256,6 +254,9 @@ ENTRY(vector_swi)
TRACE( ldmia sp, {r0 - r3} )
local_restart:
+#if defined(CONFIG_AEABI) && !defined(CONFIG_OABI_COMPAT)
+ str scno, [tsk, #TI_ABI_SYSCALL] @ store scno for syscall restart
+#endif
ldr r10, [tsk, #TI_FLAGS] @ check for syscall tracing
stmdb sp!, {r4, r5} @ push fifth and sixth args
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 2d8e2516906b..fef32d73f912 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -783,8 +783,9 @@ long arch_ptrace(struct task_struct *child, long request,
break;
case PTRACE_SET_SYSCALL:
- task_thread_info(child)->abi_syscall = data &
- __NR_SYSCALL_MASK;
+ if (data != -1)
+ data &= __NR_SYSCALL_MASK;
+ task_thread_info(child)->abi_syscall = data;
ret = 0;
break;
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next reply other threads:[~2023-08-04 7:11 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-04 7:10 Kees Cook [this message]
2023-08-04 7:10 ` [PATCH] ARM: ptrace: Restore syscall skipping and restart while tracing Kees Cook
2023-08-09 19:47 ` Arnd Bergmann
2023-08-09 19:47 ` Arnd Bergmann
2023-08-10 19:32 ` Kees Cook
2023-08-10 19:32 ` Kees Cook
2023-08-10 20:10 ` Arnd Bergmann
2023-08-10 20:10 ` Arnd Bergmann
2023-08-10 20:15 ` Kees Cook
2023-08-10 20:15 ` Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230804071045.never.134-kees@kernel.org \
--to=keescook@chromium.org \
--cc=arnd@kernel.org \
--cc=lecopzer.chen@mediatek.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux@armlinux.org.uk \
--cc=oleg@redhat.com \
--cc=rmk+kernel@armlinux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.