v12: split test into its own patch
change from padding a u8 to using a u64
casting issues
checkpatch.pl
v11: backout complex compat code, change struct to more generic typing
(padding to ensure struct size is the same in 32 compat)
update selftest
tested selftest on 64 machine and in 32-bit compat mode
v10: move refactor code into patch ahead of change
add compat support
documentation change
v9: tglx feedback
whitespace
documentation of ptrace struct
shorten struct name
helper function for set_syscall_user_dispatch
use task variant of set/clear_syscall_work
use task variant of test_syscall_work in getter
selftest
[truncated version history]
Syscall user dispatch makes it possible to cleanly intercept system
calls from user-land. However, most transparent checkpoint software
presently leverages some combination of ptrace and system call
injection to place software in a ready-to-checkpoint state.
If Syscall User Dispatch is enabled at the time of being quiesced,
injected system calls will subsequently be interposed upon and
dispatched to the task's signal handler.
Patch summary:
- Refactor configuration setting interface to operate on a task
rather than current, so the set and error paths can be consolidated
- Implement a getter interface for Syscall User Dispatch config info.
To resume successfully, the checkpoint/resume software has to
save and restore this information. Presently this configuration
is write-only, with no way for C/R software to save it.
This was done in ptrace because syscall user dispatch is not part of
uapi. The syscall_user_dispatch_config structure was added to the
ptrace exports.
- Selftest for the new feature
Gregory Price (3):
syscall_user_dispatch: helper function to operate on given task
ptrace,syscall_user_dispatch: checkpoint/restore support for SUD
selftest,ptrace: Add selftest for syscall user dispatch config api
.../admin-guide/syscall-user-dispatch.rst | 4 ++
include/linux/syscall_user_dispatch.h | 18 +++++
include/uapi/linux/ptrace.h | 29 ++++++++
kernel/entry/syscall_user_dispatch.c | 65 ++++++++++++++---
kernel/ptrace.c | 9 +++
tools/testing/selftests/ptrace/.gitignore | 1 +
tools/testing/selftests/ptrace/Makefile | 2 +-
tools/testing/selftests/ptrace/get_set_sud.c | 72 +++++++++++++++++++
8 files changed, 191 insertions(+), 9 deletions(-)
create mode 100644 tools/testing/selftests/ptrace/get_set_sud.c
--
2.39.1
Preparatory patch ahead of set/get interfaces which will allow a
ptrace to get/set the syscall user dispatch configuration of a task.
This will simplify the set interface and consolidates error paths.
Signed-off-by: Gregory Price <[email protected]>
---
kernel/entry/syscall_user_dispatch.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/kernel/entry/syscall_user_dispatch.c b/kernel/entry/syscall_user_dispatch.c
index 0b6379adff6b..22396b234854 100644
--- a/kernel/entry/syscall_user_dispatch.c
+++ b/kernel/entry/syscall_user_dispatch.c
@@ -68,8 +68,9 @@ bool syscall_user_dispatch(struct pt_regs *regs)
return true;
}
-int set_syscall_user_dispatch(unsigned long mode, unsigned long offset,
- unsigned long len, char __user *selector)
+static int task_set_syscall_user_dispatch(struct task_struct *task, unsigned long mode,
+ unsigned long offset, unsigned long len,
+ char __user *selector)
{
switch (mode) {
case PR_SYS_DISPATCH_OFF:
@@ -94,15 +95,21 @@ int set_syscall_user_dispatch(unsigned long mode, unsigned long offset,
return -EINVAL;
}
- current->syscall_dispatch.selector = selector;
- current->syscall_dispatch.offset = offset;
- current->syscall_dispatch.len = len;
- current->syscall_dispatch.on_dispatch = false;
+ task->syscall_dispatch.selector = selector;
+ task->syscall_dispatch.offset = offset;
+ task->syscall_dispatch.len = len;
+ task->syscall_dispatch.on_dispatch = false;
if (mode == PR_SYS_DISPATCH_ON)
- set_syscall_work(SYSCALL_USER_DISPATCH);
+ set_task_syscall_work(task, SYSCALL_USER_DISPATCH);
else
- clear_syscall_work(SYSCALL_USER_DISPATCH);
+ clear_task_syscall_work(task, SYSCALL_USER_DISPATCH);
return 0;
}
+
+int set_syscall_user_dispatch(unsigned long mode, unsigned long offset,
+ unsigned long len, char __user *selector)
+{
+ return task_set_syscall_user_dispatch(current, mode, offset, len, selector);
+}
--
2.39.1
Implement ptrace getter/setter interface for syscall user dispatch.
These prctl settings are presently write-only, making it impossible to
implement transparent checkpoint/restore via software like CRIU.
'on_dispatch' field is not exposed because it is a kernel-internal
only field that cannot be 'true' when returning to userland.
Signed-off-by: Gregory Price <[email protected]>
---
.../admin-guide/syscall-user-dispatch.rst | 4 ++
include/linux/syscall_user_dispatch.h | 18 ++++++++
include/uapi/linux/ptrace.h | 29 +++++++++++++
kernel/entry/syscall_user_dispatch.c | 42 +++++++++++++++++++
kernel/ptrace.c | 9 ++++
5 files changed, 102 insertions(+)
diff --git a/Documentation/admin-guide/syscall-user-dispatch.rst b/Documentation/admin-guide/syscall-user-dispatch.rst
index 60314953c728..f7648c08297e 100644
--- a/Documentation/admin-guide/syscall-user-dispatch.rst
+++ b/Documentation/admin-guide/syscall-user-dispatch.rst
@@ -73,6 +73,10 @@ thread-wide, without the need to invoke the kernel directly. selector
can be set to SYSCALL_DISPATCH_FILTER_ALLOW or SYSCALL_DISPATCH_FILTER_BLOCK.
Any other value should terminate the program with a SIGSYS.
+Additionally, a task's syscall user dispatch configuration can be peeked
+and poked via the PTRACE_(GET|SET)_SYSCALL_USER_DISPATCH_CONFIG ptrace
+requests. This is useful for checkpoint/restart software.
+
Security Notes
--------------
diff --git a/include/linux/syscall_user_dispatch.h b/include/linux/syscall_user_dispatch.h
index a0ae443fb7df..641ca8880995 100644
--- a/include/linux/syscall_user_dispatch.h
+++ b/include/linux/syscall_user_dispatch.h
@@ -22,6 +22,12 @@ int set_syscall_user_dispatch(unsigned long mode, unsigned long offset,
#define clear_syscall_work_syscall_user_dispatch(tsk) \
clear_task_syscall_work(tsk, SYSCALL_USER_DISPATCH)
+int syscall_user_dispatch_get_config(struct task_struct *task, unsigned long size,
+ void __user *data);
+
+int syscall_user_dispatch_set_config(struct task_struct *task, unsigned long size,
+ void __user *data);
+
#else
struct syscall_user_dispatch {};
@@ -35,6 +41,18 @@ static inline void clear_syscall_work_syscall_user_dispatch(struct task_struct *
{
}
+static inline int syscall_user_dispatch_get_config(struct task_struct *task,
+ unsigned long size, void __user *data)
+{
+ return -EINVAL;
+}
+
+static inline int syscall_user_dispatch_set_config(struct task_struct *task,
+ unsigned long size, void __user *data)
+{
+ return -EINVAL;
+}
+
#endif /* CONFIG_GENERIC_ENTRY */
#endif /* _SYSCALL_USER_DISPATCH_H */
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index 195ae64a8c87..1e77b02344c3 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -112,6 +112,35 @@ struct ptrace_rseq_configuration {
__u32 pad;
};
+#define PTRACE_SET_SYSCALL_USER_DISPATCH_CONFIG 0x4210
+#define PTRACE_GET_SYSCALL_USER_DISPATCH_CONFIG 0x4211
+
+/*
+ * struct ptrace_sud_config - Per-task configuration for SUD
+ * @mode: One of PR_SYS_DISPATCH_ON or PR_SYS_DISPATCH_OFF
+ * @selector: Tracee's user virtual address of SUD selector
+ * @offset: SUD exclusion area (virtual address)
+ * @len: Length of SUD exclusion area
+ *
+ * Used to get/set the syscall user dispatch configuration for tracee.
+ * process. Selector is optional (may be NULL), and if invalid will produce
+ * a SIGSEGV in the tracee upon first access.
+ *
+ * If mode is PR_SYS_DISPATCH_ON, syscall dispatch will be enabled. If
+ * PR_SYS_DISPATCH_OFF, syscall dispatch will be disabled and all other
+ * parameters must be 0. The value in *selector (if not null), also determines
+ * whether syscall dispatch will occur.
+ *
+ * The SUD Exclusion area described by offset/len is the virtual address space
+ * from which syscalls will not produce a user dispatch.
+ */
+struct ptrace_sud_config {
+ __u64 mode;
+ __u64 selector;
+ __u64 offset;
+ __u64 len;
+};
+
/*
* These values are stored in task->ptrace_message
* by ptrace_stop to describe the current syscall-stop.
diff --git a/kernel/entry/syscall_user_dispatch.c b/kernel/entry/syscall_user_dispatch.c
index 22396b234854..95b7218be71d 100644
--- a/kernel/entry/syscall_user_dispatch.c
+++ b/kernel/entry/syscall_user_dispatch.c
@@ -4,6 +4,7 @@
*/
#include <linux/sched.h>
#include <linux/prctl.h>
+#include <linux/ptrace.h>
#include <linux/syscall_user_dispatch.h>
#include <linux/uaccess.h>
#include <linux/signal.h>
@@ -113,3 +114,44 @@ int set_syscall_user_dispatch(unsigned long mode, unsigned long offset,
{
return task_set_syscall_user_dispatch(current, mode, offset, len, selector);
}
+
+int syscall_user_dispatch_get_config(struct task_struct *task, unsigned long size,
+ void __user *data)
+{
+ struct syscall_user_dispatch *sd = &task->syscall_dispatch;
+ struct ptrace_sud_config config;
+
+ if (size != sizeof(struct ptrace_sud_config))
+ return -EINVAL;
+
+ if (test_task_syscall_work(task, SYSCALL_USER_DISPATCH))
+ config.mode = PR_SYS_DISPATCH_ON;
+ else
+ config.mode = PR_SYS_DISPATCH_OFF;
+
+ config.offset = sd->offset;
+ config.len = sd->len;
+ config.selector = (__u64)(uintptr_t)sd->selector;
+
+ if (copy_to_user(data, &config, sizeof(config)))
+ return -EFAULT;
+
+ return 0;
+}
+
+int syscall_user_dispatch_set_config(struct task_struct *task, unsigned long size,
+ void __user *data)
+{
+ int rc;
+ struct ptrace_sud_config cfg;
+
+ if (size != sizeof(struct ptrace_sud_config))
+ return -EINVAL;
+
+ if (copy_from_user(&cfg, data, sizeof(struct ptrace_sud_config)))
+ return -EFAULT;
+
+ rc = task_set_syscall_user_dispatch(task, cfg.mode, cfg.offset,
+ cfg.len, (char __user *)(uintptr_t)cfg.selector);
+ return rc;
+}
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 0786450074c1..443057bee87c 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -32,6 +32,7 @@
#include <linux/compat.h>
#include <linux/sched/signal.h>
#include <linux/minmax.h>
+#include <linux/syscall_user_dispatch.h>
#include <asm/syscall.h> /* for syscall_get_* */
@@ -1259,6 +1260,14 @@ int ptrace_request(struct task_struct *child, long request,
break;
#endif
+ case PTRACE_SET_SYSCALL_USER_DISPATCH_CONFIG:
+ ret = syscall_user_dispatch_set_config(child, addr, datavp);
+ break;
+
+ case PTRACE_GET_SYSCALL_USER_DISPATCH_CONFIG:
+ ret = syscall_user_dispatch_get_config(child, addr, datavp);
+ break;
+
default:
break;
}
--
2.39.1
Validate that the following new ptrace requests work as expected
* PTRACE_GET_SYSCALL_USER_DISPATCH_CONFIG
- returns the contents of task->syscall_dispatch if enabled
* PTRACE_SET_SYSCALL_USER_DISPATCH_CONFIG
- sets the contents of task->syscall_dispatch
Signed-off-by: Gregory Price <[email protected]>
---
tools/testing/selftests/ptrace/.gitignore | 1 +
tools/testing/selftests/ptrace/Makefile | 2 +-
tools/testing/selftests/ptrace/get_set_sud.c | 72 ++++++++++++++++++++
3 files changed, 74 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/ptrace/get_set_sud.c
diff --git a/tools/testing/selftests/ptrace/.gitignore b/tools/testing/selftests/ptrace/.gitignore
index 792318aaa30c..b7dde152e75a 100644
--- a/tools/testing/selftests/ptrace/.gitignore
+++ b/tools/testing/selftests/ptrace/.gitignore
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
get_syscall_info
+get_set_sud
peeksiginfo
vmaccess
diff --git a/tools/testing/selftests/ptrace/Makefile b/tools/testing/selftests/ptrace/Makefile
index 96ffa94afb91..1c631740a730 100644
--- a/tools/testing/selftests/ptrace/Makefile
+++ b/tools/testing/selftests/ptrace/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
CFLAGS += -std=c99 -pthread -Wall $(KHDR_INCLUDES)
-TEST_GEN_PROGS := get_syscall_info peeksiginfo vmaccess
+TEST_GEN_PROGS := get_syscall_info peeksiginfo vmaccess get_set_sud
include ../lib.mk
diff --git a/tools/testing/selftests/ptrace/get_set_sud.c b/tools/testing/selftests/ptrace/get_set_sud.c
new file mode 100644
index 000000000000..5297b10d25c3
--- /dev/null
+++ b/tools/testing/selftests/ptrace/get_set_sud.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include "../kselftest_harness.h"
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <sys/wait.h>
+#include <sys/syscall.h>
+#include <sys/prctl.h>
+
+#include "linux/ptrace.h"
+
+static int sys_ptrace(int request, pid_t pid, void *addr, void *data)
+{
+ return syscall(SYS_ptrace, request, pid, addr, data);
+}
+
+TEST(get_set_sud)
+{
+ struct ptrace_sud_config config;
+ pid_t child;
+ int ret = 0;
+ int status;
+
+ child = fork();
+ ASSERT_GE(child, 0);
+ if (child == 0) {
+ ASSERT_EQ(0, sys_ptrace(PTRACE_TRACEME, 0, 0, 0)) {
+ TH_LOG("PTRACE_TRACEME: %m");
+ }
+ kill(getpid(), SIGSTOP);
+ _exit(1);
+ }
+
+ waitpid(child, &status, 0);
+
+ memset(&config, 0xff, sizeof(config));
+ config.mode = PR_SYS_DISPATCH_ON;
+
+ ret = sys_ptrace(PTRACE_GET_SYSCALL_USER_DISPATCH_CONFIG, child,
+ (void *)sizeof(config), &config);
+
+ ASSERT_EQ(ret, 0);
+ ASSERT_EQ(config.mode, PR_SYS_DISPATCH_OFF);
+ ASSERT_EQ(config.selector, 0);
+ ASSERT_EQ(config.offset, 0);
+ ASSERT_EQ(config.len, 0);
+
+ config.mode = PR_SYS_DISPATCH_ON;
+ config.selector = 0;
+ config.offset = 0x400000;
+ config.len = 0x1000;
+
+ ret = sys_ptrace(PTRACE_SET_SYSCALL_USER_DISPATCH_CONFIG, child,
+ (void *)sizeof(config), &config);
+
+ ASSERT_EQ(ret, 0);
+
+ memset(&config, 1, sizeof(config));
+ ret = sys_ptrace(PTRACE_GET_SYSCALL_USER_DISPATCH_CONFIG, child,
+ (void *)sizeof(config), &config);
+
+ ASSERT_EQ(ret, 0);
+ ASSERT_EQ(config.mode, PR_SYS_DISPATCH_ON);
+ ASSERT_EQ(config.selector, 0);
+ ASSERT_EQ(config.offset, 0x400000);
+ ASSERT_EQ(config.len, 0x1000);
+
+ kill(child, SIGKILL);
+}
+
+TEST_HARNESS_MAIN
--
2.39.1
Hi Gragory,
On Fri, 24 Feb 2023 at 23:40, Gregory Price <[email protected]> wrote:
>
> Implement ptrace getter/setter interface for syscall user dispatch.
>
> These prctl settings are presently write-only, making it impossible to
> implement transparent checkpoint/restore via software like CRIU.
>
> 'on_dispatch' field is not exposed because it is a kernel-internal
> only field that cannot be 'true' when returning to userland.
>
> Signed-off-by: Gregory Price <[email protected]>
> ---
[..]
> +int syscall_user_dispatch_set_config(struct task_struct *task, unsigned long size,
> + void __user *data)
> +{
> + int rc;
> + struct ptrace_sud_config cfg;
> +
> + if (size != sizeof(struct ptrace_sud_config))
> + return -EINVAL;
> +
> + if (copy_from_user(&cfg, data, sizeof(struct ptrace_sud_config)))
> + return -EFAULT;
It seems that the tool you want here would be copy_struct_from_user(),
which is designed for extendable syscalls.
Thanks,
Dmitry
On Mon, 27 Feb 2023 at 16:02, Dmitry Safonov <[email protected]> wrote:
>
> Hi Gragory,
s/Gragory/Gregory/
Sorry, a typo!
--
Dmitry
On 02/27, Dmitry Safonov wrote:
>
> > +int syscall_user_dispatch_set_config(struct task_struct *task, unsigned long size,
> > + void __user *data)
> > +{
> > + int rc;
> > + struct ptrace_sud_config cfg;
> > +
> > + if (size != sizeof(struct ptrace_sud_config))
> > + return -EINVAL;
> > +
> > + if (copy_from_user(&cfg, data, sizeof(struct ptrace_sud_config)))
> > + return -EFAULT;
>
> It seems that the tool you want here would be copy_struct_from_user(),
> which is designed for extendable syscalls.
Hmm. Why?
In this case ksize == usize, so why do we need copy_struct_from_user ?
Oleg.
On 2/28/23 16:52, Oleg Nesterov wrote:
> On 02/27, Dmitry Safonov wrote:
>>
>>> +int syscall_user_dispatch_set_config(struct task_struct *task, unsigned long size,
>>> + void __user *data)
>>> +{
>>> + int rc;
>>> + struct ptrace_sud_config cfg;
>>> +
>>> + if (size != sizeof(struct ptrace_sud_config))
>>> + return -EINVAL;
>>> +
>>> + if (copy_from_user(&cfg, data, sizeof(struct ptrace_sud_config)))
>>> + return -EFAULT;
>>
>> It seems that the tool you want here would be copy_struct_from_user(),
>> which is designed for extendable syscalls.
>
> Hmm. Why?
>
> In this case ksize == usize, so why do we need copy_struct_from_user ?
In case the structure extends in future, that will let newer userspace
run on an older kernel (as long as it doesn't use [set] any new fields).
With regular sizeof(struct ptrace_sud_config) instead of adding
size-related defines.
It was Christian's idea how-to add/design new syscalls in an
"extensible" manner. Here are his LPC slides:
https://lpc.events/event/7/contributions/657/attachments/639/1159/extensible_syscalls.pdf
[7/18 slide on checks]
And an LWN article:
https://lwn.net/Articles/830666/
Thanks,
Dmitry
Gregory,
I can't resist, I have a couple of cosmetic nits.
On 02/24, Gregory Price wrote:
>
> +int syscall_user_dispatch_get_config(struct task_struct *task, unsigned long size,
> + void __user *data)
> +{
> + struct syscall_user_dispatch *sd = &task->syscall_dispatch;
> + struct ptrace_sud_config config;
> +
> + if (size != sizeof(struct ptrace_sud_config))
> + return -EINVAL;
> +
> + if (test_task_syscall_work(task, SYSCALL_USER_DISPATCH))
> + config.mode = PR_SYS_DISPATCH_ON;
> + else
> + config.mode = PR_SYS_DISPATCH_OFF;
> +
> + config.offset = sd->offset;
> + config.len = sd->len;
> + config.selector = (__u64)(uintptr_t)sd->selector;
> +
> + if (copy_to_user(data, &config, sizeof(config)))
Let me repeat, do not mix sizeof(struct ptrace_sud_config) and sizeof(config).
Perhaps this is just me, but this looks confusing to me. Please use
sizeof(config) both times.
> +int syscall_user_dispatch_set_config(struct task_struct *task, unsigned long size,
> + void __user *data)
> +{
> + int rc;
> + struct ptrace_sud_config cfg;
^^^
Again, this is cosmetic but a bit annoying. Please use either "config" or
"cfg" in both functions to make the naming more consistent.
> + rc = task_set_syscall_user_dispatch(task, cfg.mode, cfg.offset,
> + cfg.len, (char __user *)(uintptr_t)cfg.selector);
rc = task_set_syscall_user_dispatch(task, cfg.mode, cfg.offset, cfg.len,
(char __user *)(uintptr_t)cfg.selector);
looks a bit better to me.
Oleg.
On 02/28, Dmitry Safonov wrote:
>
> On 2/28/23 16:52, Oleg Nesterov wrote:
> > On 02/27, Dmitry Safonov wrote:
> >>
> >>> +int syscall_user_dispatch_set_config(struct task_struct *task, unsigned long size,
> >>> + void __user *data)
> >>> +{
> >>> + int rc;
> >>> + struct ptrace_sud_config cfg;
> >>> +
> >>> + if (size != sizeof(struct ptrace_sud_config))
> >>> + return -EINVAL;
> >>> +
> >>> + if (copy_from_user(&cfg, data, sizeof(struct ptrace_sud_config)))
> >>> + return -EFAULT;
> >>
> >> It seems that the tool you want here would be copy_struct_from_user(),
> >> which is designed for extendable syscalls.
> >
> > Hmm. Why?
> >
> > In this case ksize == usize, so why do we need copy_struct_from_user ?
>
> In case the structure extends in future, that will let newer userspace
> run on an older kernel (as long as it doesn't use [set] any new fields).
Sure, I understand that, but I don't think it's worth the trouble
in this case.
If (unlikely, I think) this structure ever extends we can switch to
copy_struct_from_user() or do something else if check_zeroed_user()
makes no real sense for the new fields.
Right now I think it is more important to ensure that the new users
of this API use the correct size.
Oleg.