* Re: [PATCH v7 0/6] introduce PIDFD_SELF* sentinels
From: Lorenzo Stoakes @ 2025-02-01 16:38 UTC (permalink / raw)
To: Christian Brauner
Cc: Andrew Morton, Pedro Falcato, Christian Brauner, Shuah Khan,
Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
linux-kselftest, linux-mm, linux-fsdevel, linux-api, linux-kernel,
Oliver Sang, John Hubbard, Tejun Heo, Johannes Weiner,
Michal Koutny, Shakeel Butt
In-Reply-To: <20250201-cremig-desorientiert-aac3b09da8e2@brauner>
On Sat, Feb 01, 2025 at 12:12:46PM +0100, Christian Brauner wrote:
> > Intent is for Christian to take this in his tree (if he so wishes) to be
> > clear!
>
> If you send me an updated blurb I can fold it.
Thanks, I therefore propose replacing the cover letter blurb with the below:
----8<----
If you wish to utilise a pidfd interface to refer to the current process or
thread then there is a lot of ceremony involved, looking something like:
pid_t pid = getpid();
int pidfd = pidfd_open(pid, PIDFD_THREAD);
if (pidfd < 0) {
... error handling ...
}
if (process_madvise(pidfd, iovec, 8, MADV_GUARD_INSTALL, 0)) {
... cleanup pidfd ...
... error handling ...
}
...
... cleanup pidfd ...
This adds unnecessary overhead + system calls, complicated error handling
and in addition pidfd_open() is subject to RLIMIT_NOFILE (i.e. the limit of
per-process number of open file descriptors), so the call may fail
spuriously on this basis.
Rather than doing this we can make use of sentinels for this purpose which can
be passed as the pidfd instead. This looks like:
if (process_madvise(PIDFD_SELF, iovec, 8, MADV_GUARD_INSTALL, 0)) {
... error handling ...
}
And avoids all of the aforementioned issues. This series introduces such
sentinels.
It is useful to refer to both the current thread from the userland's
perspective for which we use PIDFD_SELF, and the current process from the
userland's perspective, for which we use PIDFD_SELF_PROCESS.
There is unfortunately some confusion between the kernel and userland as to
what constitutes a process - a thread from the userland perspective is a
process in userland, and a userland process is a thread group (more
specifically the thread group leader from the kernel perspective). We
therefore alias things thusly:
* PIDFD_SELF_THREAD aliased by PIDFD_SELF - use PIDTYPE_PID.
* PIDFD_SELF_THREAD_GROUP alised by PIDFD_SELF_PROCESS - use PIDTYPE_TGID.
In all of the kernel code we refer to PIDFD_SELF_THREAD and
PIDFD_SELF_THREAD_GROUP. However we expect users to use PIDFD_SELF and
PIDFD_SELF_PROCESS.
This matters for cases where, for instance, a user unshare()'s FDs or does
thread-specific signal handling and where the user would be hugely confused
if the FDs referenced or signal processed referred to the thread group
leader rather than the individual thread.
For now we only adjust pidfd_get_task() and the pidfd_send_signal() system
call with specific handling for this, implementing this functionality for
process_madvise(), process_mrelease() (albeit, using it here wouldn't
really make sense) and pidfd_send_signal().
We defer making further changes, as this would require a significant rework
of the pidfd mechanism.
The motivating case here is to support PIDFD_SELF in process_madvise(), so
this suffices for immediate uses. Moving forward, this can be further
expanded to other uses.
^ permalink raw reply
* Re: [PATCH v2] seccomp: passthrough uretprobe systemcall without filtering
From: Jiri Olsa @ 2025-02-02 11:08 UTC (permalink / raw)
To: Eyal Birger
Cc: Kees Cook, Jiri Olsa, luto, wad, oleg, mhiramat, andrii,
alexei.starovoitov, cyphar, songliubraving, yhs, john.fastabend,
peterz, tglx, bp, daniel, ast, andrii.nakryiko, rostedt, rafi,
shmulik.ladkani, bpf, linux-api, linux-trace-kernel, x86,
linux-kernel, stable
In-Reply-To: <Z5v063xNVJfXCnKV@krava>
On Thu, Jan 30, 2025 at 10:53:47PM +0100, Jiri Olsa wrote:
SNIP
> > > > I think this would mean that this test suite would need to run as
> > > > privileged. Is that Ok? or maybe it'd be better to have a new suite?
> > > >
> > > > > With at least these cases combinations below. Check each of:
> > > > >
> > > > > - not using uretprobe passes
> > > > > - using uretprobe passes (and validates that uretprobe did work)
> > > > >
> > > > > in each of the following conditions:
> > > > >
> > > > > - default-allow filter
> > > > > - default-block filter
> > > > > - filter explicitly blocking __NR_uretprobe and nothing else
> > > > > - filter explicitly allowing __NR_uretprobe (and only other
> > > > > required syscalls)
> > > >
> > > > Ok.
> > >
> > > please let me know if I can help in any way with tests
> >
> > Thanks! Is there a way to partition this work? I'd appreciate the help
> > if we can find some way of doing so.
>
> sure, I'll check the seccomp selftests and let you know
hi,
if it's any help, feel free to use the code below that creates uretprobe,
it could be bit simpler if we use libbpf, but I think that's not an option
jirka
---
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 8c3a73461475..1f99d31d05a1 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -47,6 +47,7 @@
#include <linux/kcmp.h>
#include <sys/resource.h>
#include <sys/capability.h>
+#include <linux/perf_event.h>
#include <unistd.h>
#include <sys/syscall.h>
@@ -4888,6 +4889,130 @@ TEST(tsync_vs_dead_thread_leader)
EXPECT_EQ(0, status);
}
+__attribute__((noinline)) int probed(void)
+{
+ return 1;
+}
+
+static int parse_uint_from_file(const char *file, const char *fmt)
+{
+ int err = -1, ret;
+ FILE *f;
+
+ f = fopen(file, "re");
+ if (f) {
+ err = fscanf(f, fmt, &ret);
+ fclose(f);
+ }
+ return err == 1 ? ret : err;
+}
+
+static int determine_uprobe_perf_type(void)
+{
+ const char *file = "/sys/bus/event_source/devices/uprobe/type";
+
+ return parse_uint_from_file(file, "%d\n");
+}
+
+static int determine_uprobe_retprobe_bit(void)
+{
+ const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
+
+ return parse_uint_from_file(file, "config:%d\n");
+}
+
+static ssize_t get_uprobe_offset(const void *addr)
+{
+ size_t start, base, end;
+ bool found = false;
+ char buf[256];
+ FILE *f;
+
+ f = fopen("/proc/self/maps", "r");
+ if (!f)
+ return -1;
+
+ while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
+ if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
+ found = true;
+ break;
+ }
+ }
+ fclose(f);
+ return found ? (uintptr_t)addr - start + base : -1;
+}
+
+static int create_uretprobe(void *addr)
+{
+ const size_t attr_sz = sizeof(struct perf_event_attr);
+ struct perf_event_attr attr;
+ ssize_t offset;
+ int type, bit;
+
+ memset(&attr, 0, attr_sz);
+
+ type = determine_uprobe_perf_type();
+ if (type < 0)
+ return -1;
+ bit = determine_uprobe_retprobe_bit();
+ if (bit < 0)
+ return -1;
+ offset = get_uprobe_offset(probed);
+ if (offset < 0)
+ return -1;
+
+ attr.config |= 1 << bit;
+ attr.size = attr_sz;
+ attr.type = type;
+ attr.config1 = ptr_to_u64("/proc/self/exe");
+ attr.config2 = offset;
+
+ return syscall(__NR_perf_event_open, &attr,
+ getpid() /* pid */, -1 /* cpu */, -1 /* group_fd */,
+ PERF_FLAG_FD_CLOEXEC);
+}
+
+TEST(uretprobe)
+{
+ struct sock_filter filter[] = {
+ BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
+ offsetof(struct seccomp_data, nr)),
+ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_exit_group, 1, 0),
+ BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
+ BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+ };
+ struct sock_fprog prog = {
+ .len = (unsigned short)ARRAY_SIZE(filter),
+ .filter = filter,
+ };
+ long ret;
+ int fd;
+
+ fd = create_uretprobe(probed);
+ ASSERT_GE(fd, 0) {
+ TH_LOG("Failed to create uretprobe!!");
+ }
+
+ ret = prctl(PR_SET_NO_NEW_PRIVS, 1, NULL, 0, 0);
+ ASSERT_EQ(0, ret) {
+ TH_LOG("Kernel does not support PR_SET_NO_NEW_PRIVS!");
+ }
+
+ ret = seccomp(SECCOMP_SET_MODE_FILTER, 0, &prog);
+ ASSERT_NE(ENOSYS, errno) {
+ TH_LOG("Kernel does not support seccomp syscall!");
+ }
+ EXPECT_EQ(0, ret) {
+ TH_LOG("Could not install filter!");
+ }
+
+ /* should not explode */
+ probed();
+
+ /* we could call close(fd), but we'd need extra filter for
+ * that and since we we are calling _exit right away.. */
+}
+
/*
* TODO:
* - expand NNP testing
^ permalink raw reply related
* Re: [PATCH v2] seccomp: passthrough uretprobe systemcall without filtering
From: Eyal Birger @ 2025-02-02 16:28 UTC (permalink / raw)
To: Jiri Olsa
Cc: Kees Cook, luto, wad, oleg, mhiramat, andrii, alexei.starovoitov,
cyphar, songliubraving, yhs, john.fastabend, peterz, tglx, bp,
daniel, ast, andrii.nakryiko, rostedt, rafi, shmulik.ladkani, bpf,
linux-api, linux-trace-kernel, x86, linux-kernel, stable
In-Reply-To: <Z59SHdsme3qlx8UZ@krava>
On Sun, Feb 2, 2025 at 3:08 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Jan 30, 2025 at 10:53:47PM +0100, Jiri Olsa wrote:
>
> SNIP
>
> > > > > I think this would mean that this test suite would need to run as
> > > > > privileged. Is that Ok? or maybe it'd be better to have a new suite?
> > > > >
> > > > > > With at least these cases combinations below. Check each of:
> > > > > >
> > > > > > - not using uretprobe passes
> > > > > > - using uretprobe passes (and validates that uretprobe did work)
> > > > > >
> > > > > > in each of the following conditions:
> > > > > >
> > > > > > - default-allow filter
> > > > > > - default-block filter
> > > > > > - filter explicitly blocking __NR_uretprobe and nothing else
> > > > > > - filter explicitly allowing __NR_uretprobe (and only other
> > > > > > required syscalls)
> > > > >
> > > > > Ok.
> > > >
> > > > please let me know if I can help in any way with tests
> > >
> > > Thanks! Is there a way to partition this work? I'd appreciate the help
> > > if we can find some way of doing so.
> >
> > sure, I'll check the seccomp selftests and let you know
>
> hi,
> if it's any help, feel free to use the code below that creates uretprobe,
> it could be bit simpler if we use libbpf, but I think that's not an option
Thank you very much!
Eyal.
^ permalink raw reply
* [PATCH v3 0/2] seccomp: pass uretprobe system call through seccomp
From: Eyal Birger @ 2025-02-02 16:29 UTC (permalink / raw)
To: kees, luto, wad, oleg, mhiramat, andrii, jolsa
Cc: alexei.starovoitov, olsajiri, cyphar, songliubraving, yhs,
john.fastabend, peterz, tglx, bp, daniel, ast, andrii.nakryiko,
rostedt, rafi, shmulik.ladkani, bpf, linux-api,
linux-trace-kernel, x86, linux-kernel, Eyal Birger
uretprobe(2) is an performance enhancement system call added to improve
uretprobes on x86_64.
Confinement environments such as Docker are not aware of this new system
call and kill confined processes when uretprobes are attached to them.
Since uretprobe is a "kernel implementation detail" system call which is
not used by userspace application code directly, pass this system call
through seccomp without forcing existing userspace confinement environments
to be changed.
To: Kees Cook <kees@kernel.org>
To: Andy Lutomirski <luto@amacapital.net>
To: Will Drewry <wad@chromium.org>
To: Oleg Nesterov <oleg@redhat.com>
To: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Jiri Olsa <jolsa@kernel.org>
To: Andrii Nakryiko <andrii@kernel.org>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
Eyal Birger (2):
seccomp: passthrough uretprobe systemcall without filtering
selftests/seccomp: validate uretprobe syscall passes through seccomp
kernel/seccomp.c | 24 ++-
tools/testing/selftests/seccomp/seccomp_bpf.c | 195 ++++++++++++++++++
2 files changed, 216 insertions(+), 3 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v3 1/2] seccomp: passthrough uretprobe systemcall without filtering
From: Eyal Birger @ 2025-02-02 16:29 UTC (permalink / raw)
To: kees, luto, wad, oleg, mhiramat, andrii, jolsa
Cc: alexei.starovoitov, olsajiri, cyphar, songliubraving, yhs,
john.fastabend, peterz, tglx, bp, daniel, ast, andrii.nakryiko,
rostedt, rafi, shmulik.ladkani, bpf, linux-api,
linux-trace-kernel, x86, linux-kernel, Eyal Birger, stable
In-Reply-To: <20250202162921.335813-1-eyal.birger@gmail.com>
When attaching uretprobes to processes running inside docker, the attached
process is segfaulted when encountering the retprobe.
The reason is that now that uretprobe is a system call the default seccomp
filters in docker block it as they only allow a specific set of known
syscalls. This is true for other userspace applications which use seccomp
to control their syscall surface.
Since uretprobe is a "kernel implementation detail" system call which is
not used by userspace application code directly, it is impractical and
there's very little point in forcing all userspace applications to
explicitly allow it in order to avoid crashing tracked processes.
Pass this systemcall through seccomp without depending on configuration.
Note: uretprobe isn't supported in i386 and __NR_ia32_rt_tgsigqueueinfo
uses the same number as __NR_uretprobe so the syscall isn't forced in the
compat bitmap.
Fixes: ff474a78cef5 ("uprobe: Add uretprobe syscall to speed up return probe")
Reported-by: Rafael Buchbinder <rafi@rbk.io>
Link: https://lore.kernel.org/lkml/CAHsH6Gs3Eh8DFU0wq58c_LF8A4_+o6z456J7BidmcVY2AqOnHQ@mail.gmail.com/
Link: https://lore.kernel.org/lkml/20250121182939.33d05470@gandalf.local.home/T/#me2676c378eff2d6a33f3054fed4a5f3afa64e65b
Link: https://lore.kernel.org/lkml/20250128145806.1849977-1-eyal.birger@gmail.com/
Cc: stable@vger.kernel.org
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
---
v3: no change - deferring 32bit compat handling as there aren't plans to
support this syscall in compat mode.
v2: use action_cache bitmap and mode1 array to check the syscall
---
kernel/seccomp.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index f59381c4a2ff..09b6f8e6db51 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -734,13 +734,13 @@ seccomp_prepare_user_filter(const char __user *user_filter)
#ifdef SECCOMP_ARCH_NATIVE
/**
- * seccomp_is_const_allow - check if filter is constant allow with given data
+ * seccomp_is_filter_const_allow - check if filter is constant allow with given data
* @fprog: The BPF programs
* @sd: The seccomp data to check against, only syscall number and arch
* number are considered constant.
*/
-static bool seccomp_is_const_allow(struct sock_fprog_kern *fprog,
- struct seccomp_data *sd)
+static bool seccomp_is_filter_const_allow(struct sock_fprog_kern *fprog,
+ struct seccomp_data *sd)
{
unsigned int reg_value = 0;
unsigned int pc;
@@ -812,6 +812,21 @@ static bool seccomp_is_const_allow(struct sock_fprog_kern *fprog,
return false;
}
+static bool seccomp_is_const_allow(struct sock_fprog_kern *fprog,
+ struct seccomp_data *sd)
+{
+#ifdef __NR_uretprobe
+ if (sd->nr == __NR_uretprobe
+#ifdef SECCOMP_ARCH_COMPAT
+ && sd->arch != SECCOMP_ARCH_COMPAT
+#endif
+ )
+ return true;
+#endif
+
+ return seccomp_is_filter_const_allow(fprog, sd);
+}
+
static void seccomp_cache_prepare_bitmap(struct seccomp_filter *sfilter,
void *bitmap, const void *bitmap_prev,
size_t bitmap_size, int arch)
@@ -1023,6 +1038,9 @@ static inline void seccomp_log(unsigned long syscall, long signr, u32 action,
*/
static const int mode1_syscalls[] = {
__NR_seccomp_read, __NR_seccomp_write, __NR_seccomp_exit, __NR_seccomp_sigreturn,
+#ifdef __NR_uretprobe
+ __NR_uretprobe,
+#endif
-1, /* negative terminated */
};
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/2] selftests/seccomp: validate uretprobe syscall passes through seccomp
From: Eyal Birger @ 2025-02-02 16:29 UTC (permalink / raw)
To: kees, luto, wad, oleg, mhiramat, andrii, jolsa
Cc: alexei.starovoitov, olsajiri, cyphar, songliubraving, yhs,
john.fastabend, peterz, tglx, bp, daniel, ast, andrii.nakryiko,
rostedt, rafi, shmulik.ladkani, bpf, linux-api,
linux-trace-kernel, x86, linux-kernel, Eyal Birger
In-Reply-To: <20250202162921.335813-1-eyal.birger@gmail.com>
The uretprobe syscall is implemented as a performance enhancement on
x86_64 by having the kernel inject a call to it on function exit; User
programs cannot call this system call explicitly.
As such, this syscall is considered a kernel implementation detail and
should not be filtered by seccomp.
Enhance the seccomp bpf test suite to check that uretprobes can be
attached to processes without the killing the process regardless of
seccomp policy.
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
---
tools/testing/selftests/seccomp/seccomp_bpf.c | 195 ++++++++++++++++++
1 file changed, 195 insertions(+)
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 8c3a73461475..bee4f424c5c3 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -47,6 +47,7 @@
#include <linux/kcmp.h>
#include <sys/resource.h>
#include <sys/capability.h>
+#include <linux/perf_event.h>
#include <unistd.h>
#include <sys/syscall.h>
@@ -68,6 +69,10 @@
# define PR_SET_PTRACER 0x59616d61
#endif
+#ifndef noinline
+#define noinline __attribute__((noinline))
+#endif
+
#ifndef PR_SET_NO_NEW_PRIVS
#define PR_SET_NO_NEW_PRIVS 38
#define PR_GET_NO_NEW_PRIVS 39
@@ -4888,6 +4893,196 @@ TEST(tsync_vs_dead_thread_leader)
EXPECT_EQ(0, status);
}
+noinline int probed(void)
+{
+ return 1;
+}
+
+static int parse_uint_from_file(const char *file, const char *fmt)
+{
+ int err = -1, ret;
+ FILE *f;
+
+ f = fopen(file, "re");
+ if (f) {
+ err = fscanf(f, fmt, &ret);
+ fclose(f);
+ }
+ return err == 1 ? ret : err;
+}
+
+static int determine_uprobe_perf_type(void)
+{
+ const char *file = "/sys/bus/event_source/devices/uprobe/type";
+
+ return parse_uint_from_file(file, "%d\n");
+}
+
+static int determine_uprobe_retprobe_bit(void)
+{
+ const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
+
+ return parse_uint_from_file(file, "config:%d\n");
+}
+
+static ssize_t get_uprobe_offset(const void *addr)
+{
+ size_t start, base, end;
+ bool found = false;
+ char buf[256];
+ FILE *f;
+
+ f = fopen("/proc/self/maps", "r");
+ if (!f)
+ return -1;
+
+ while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
+ if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
+ found = true;
+ break;
+ }
+ }
+ fclose(f);
+ return found ? (uintptr_t)addr - start + base : -1;
+}
+
+FIXTURE(URETPROBE) {
+ int fd;
+};
+
+FIXTURE_VARIANT(URETPROBE) {
+ /*
+ * All of the URETPROBE behaviors can be tested with either
+ * uretprobe attached or not
+ */
+ bool attach;
+};
+
+FIXTURE_VARIANT_ADD(URETPROBE, attached) {
+ .attach = true,
+};
+
+FIXTURE_VARIANT_ADD(URETPROBE, not_attached) {
+ .attach = false,
+};
+
+FIXTURE_SETUP(URETPROBE)
+{
+ const size_t attr_sz = sizeof(struct perf_event_attr);
+ struct perf_event_attr attr;
+ ssize_t offset;
+ int type, bit;
+
+ if (!variant->attach)
+ return;
+
+ memset(&attr, 0, attr_sz);
+
+ type = determine_uprobe_perf_type();
+ ASSERT_GE(type, 0);
+ bit = determine_uprobe_retprobe_bit();
+ ASSERT_GE(bit, 0);
+ offset = get_uprobe_offset(probed);
+ ASSERT_GE(offset, 0);
+
+ attr.config |= 1 << bit;
+ attr.size = attr_sz;
+ attr.type = type;
+ attr.config1 = ptr_to_u64("/proc/self/exe");
+ attr.config2 = offset;
+
+ self->fd = syscall(__NR_perf_event_open, &attr,
+ getpid() /* pid */, -1 /* cpu */, -1 /* group_fd */,
+ PERF_FLAG_FD_CLOEXEC);
+}
+
+FIXTURE_TEARDOWN(URETPROBE)
+{
+ /* we could call close(self->fd), but we'd need extra filter for
+ * that and since we are calling _exit right away..
+ */
+}
+
+static int run_probed_with_filter(struct sock_fprog *prog)
+{
+ if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) ||
+ seccomp(SECCOMP_SET_MODE_FILTER, 0, prog)) {
+ return -1;
+ }
+
+ probed();
+ return 0;
+}
+
+TEST_F(URETPROBE, uretprobe_default_allow)
+{
+ struct sock_filter filter[] = {
+ BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+ };
+ struct sock_fprog prog = {
+ .len = (unsigned short)ARRAY_SIZE(filter),
+ .filter = filter,
+ };
+
+ ASSERT_EQ(0, run_probed_with_filter(&prog));
+}
+
+TEST_F(URETPROBE, uretprobe_default_block)
+{
+ struct sock_filter filter[] = {
+ BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
+ offsetof(struct seccomp_data, nr)),
+ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_exit_group, 1, 0),
+ BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
+ BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+ };
+ struct sock_fprog prog = {
+ .len = (unsigned short)ARRAY_SIZE(filter),
+ .filter = filter,
+ };
+
+ ASSERT_EQ(0, run_probed_with_filter(&prog));
+}
+
+TEST_F(URETPROBE, uretprobe_block_uretprobe_syscall)
+{
+ struct sock_filter filter[] = {
+ BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
+ offsetof(struct seccomp_data, nr)),
+#ifdef __NR_uretprobe
+ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_uretprobe, 0, 1),
+#endif
+ BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
+ BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+ };
+ struct sock_fprog prog = {
+ .len = (unsigned short)ARRAY_SIZE(filter),
+ .filter = filter,
+ };
+
+ ASSERT_EQ(0, run_probed_with_filter(&prog));
+}
+
+TEST_F(URETPROBE, uretprobe_default_block_with_uretprobe_syscall)
+{
+ struct sock_filter filter[] = {
+ BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
+ offsetof(struct seccomp_data, nr)),
+#ifdef __NR_uretprobe
+ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_uretprobe, 2, 0),
+#endif
+ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_exit_group, 1, 0),
+ BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
+ BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+ };
+ struct sock_fprog prog = {
+ .len = (unsigned short)ARRAY_SIZE(filter),
+ .filter = filter,
+ };
+
+ ASSERT_EQ(0, run_probed_with_filter(&prog));
+}
+
/*
* TODO:
* - expand NNP testing
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3 2/2] selftests/seccomp: validate uretprobe syscall passes through seccomp
From: Jiri Olsa @ 2025-02-02 20:51 UTC (permalink / raw)
To: Eyal Birger
Cc: kees, luto, wad, oleg, mhiramat, andrii, alexei.starovoitov,
olsajiri, cyphar, songliubraving, yhs, john.fastabend, peterz,
tglx, bp, daniel, ast, andrii.nakryiko, rostedt, rafi,
shmulik.ladkani, bpf, linux-api, linux-trace-kernel, x86,
linux-kernel
In-Reply-To: <20250202162921.335813-3-eyal.birger@gmail.com>
On Sun, Feb 02, 2025 at 08:29:21AM -0800, Eyal Birger wrote:
SNIP
> +TEST_F(URETPROBE, uretprobe_default_block)
> +{
> + struct sock_filter filter[] = {
> + BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> + offsetof(struct seccomp_data, nr)),
> + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_exit_group, 1, 0),
> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> + };
> + struct sock_fprog prog = {
> + .len = (unsigned short)ARRAY_SIZE(filter),
> + .filter = filter,
> + };
> +
> + ASSERT_EQ(0, run_probed_with_filter(&prog));
> +}
> +
> +TEST_F(URETPROBE, uretprobe_block_uretprobe_syscall)
> +{
> + struct sock_filter filter[] = {
> + BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> + offsetof(struct seccomp_data, nr)),
> +#ifdef __NR_uretprobe
> + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_uretprobe, 0, 1),
> +#endif
does it make sense to run these tests on archs without __NR_uretprobe ?
jirka
> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> + };
> + struct sock_fprog prog = {
> + .len = (unsigned short)ARRAY_SIZE(filter),
> + .filter = filter,
> + };
> +
> + ASSERT_EQ(0, run_probed_with_filter(&prog));
> +}
> +
> +TEST_F(URETPROBE, uretprobe_default_block_with_uretprobe_syscall)
> +{
> + struct sock_filter filter[] = {
> + BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> + offsetof(struct seccomp_data, nr)),
> +#ifdef __NR_uretprobe
> + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_uretprobe, 2, 0),
> +#endif
> + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_exit_group, 1, 0),
> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> + };
> + struct sock_fprog prog = {
> + .len = (unsigned short)ARRAY_SIZE(filter),
> + .filter = filter,
> + };
> +
> + ASSERT_EQ(0, run_probed_with_filter(&prog));
> +}
> +
> /*
> * TODO:
> * - expand NNP testing
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v3 2/2] selftests/seccomp: validate uretprobe syscall passes through seccomp
From: Eyal Birger @ 2025-02-02 21:13 UTC (permalink / raw)
To: Jiri Olsa
Cc: kees, luto, wad, oleg, mhiramat, andrii, alexei.starovoitov,
cyphar, songliubraving, yhs, john.fastabend, peterz, tglx, bp,
daniel, ast, andrii.nakryiko, rostedt, rafi, shmulik.ladkani, bpf,
linux-api, linux-trace-kernel, x86, linux-kernel
In-Reply-To: <Z5_a33NQwrVC9n3r@krava>
On Sun, Feb 2, 2025 at 12:51 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Sun, Feb 02, 2025 at 08:29:21AM -0800, Eyal Birger wrote:
>
> SNIP
>
> > +TEST_F(URETPROBE, uretprobe_default_block)
> > +{
> > + struct sock_filter filter[] = {
> > + BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> > + offsetof(struct seccomp_data, nr)),
> > + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_exit_group, 1, 0),
> > + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
> > + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> > + };
> > + struct sock_fprog prog = {
> > + .len = (unsigned short)ARRAY_SIZE(filter),
> > + .filter = filter,
> > + };
> > +
> > + ASSERT_EQ(0, run_probed_with_filter(&prog));
> > +}
> > +
> > +TEST_F(URETPROBE, uretprobe_block_uretprobe_syscall)
> > +{
> > + struct sock_filter filter[] = {
> > + BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> > + offsetof(struct seccomp_data, nr)),
> > +#ifdef __NR_uretprobe
> > + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_uretprobe, 0, 1),
> > +#endif
>
> does it make sense to run these tests on archs without __NR_uretprobe ?
I considered ifdefing them out, but then thought that given it's not
a lot of code it'd be better for the tests to be compiling and
ready in case support is added on a new platform than to have to
worry about that at that point.
Eyal.
^ permalink raw reply
* [PATCH v4 6/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO request
From: Dmitry V. Levin @ 2025-02-03 7:00 UTC (permalink / raw)
To: Andrew Morton
Cc: Oleg Nesterov, Alexey Gladkov, Charlie Jenkins,
Eugene Syromyatnikov, Mike Frysinger, Renzo Davoli,
Davide Berardi, strace-devel, linux-kernel, linux-api
In-Reply-To: <20250203065849.GA14120@strace.io>
PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements
PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of
system calls the tracee is blocked in.
This API allows ptracers to obtain and modify system call details
in a straightforward and architecture-agnostic way.
Current implementation supports changing only those bits of system call
information that are used by strace, namely, syscall number, syscall
arguments, and syscall return value.
Support of changing additional details returned by PTRACE_GET_SYSCALL_INFO,
such as instruction pointer and stack pointer, could be added later if
needed, by using struct ptrace_syscall_info.flags to specify the additional
details that should be set. Currently, "flags" and "reserved" fields of
struct ptrace_syscall_info must be initialized with zeroes; "arch",
"instruction_pointer", and "stack_pointer" fields are ignored.
PTRACE_SET_SYSCALL_INFO currently supports only PTRACE_SYSCALL_INFO_ENTRY,
PTRACE_SYSCALL_INFO_EXIT, and PTRACE_SYSCALL_INFO_SECCOMP operations.
Other operations could be added later if needed.
Ideally, PTRACE_SET_SYSCALL_INFO should have been introduced along with
PTRACE_GET_SYSCALL_INFO, but it didn't happen. The last straw that
convinced me to implement PTRACE_SET_SYSCALL_INFO was apparent failure
to provide an API of changing the first system call argument on riscv
architecture.
ptrace(2) man page:
long ptrace(enum __ptrace_request request, pid_t pid, void *addr, void *data);
...
PTRACE_SET_SYSCALL_INFO
Modify information about the system call that caused the stop.
The "data" argument is a pointer to struct ptrace_syscall_info
that specifies the system call information to be set.
The "addr" argument should be set to sizeof(struct ptrace_syscall_info)).
Link: https://lore.kernel.org/all/59505464-c84a-403d-972f-d4b2055eeaac@gmail.com/
Signed-off-by: Dmitry V. Levin <ldv@strace.io>
Reviewed-by: Alexey Gladkov <legion@kernel.org>
Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
Tested-by: Charlie Jenkins <charlie@rivosinc.com>
Reviewed-by: Eugene Syromiatnikov <esyr@redhat.com>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
---
include/uapi/linux/ptrace.h | 7 ++-
kernel/ptrace.c | 121 +++++++++++++++++++++++++++++++++++-
2 files changed, 126 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index 72c038fc71d0..5f8ef6156752 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -74,6 +74,7 @@ struct seccomp_metadata {
};
#define PTRACE_GET_SYSCALL_INFO 0x420e
+#define PTRACE_SET_SYSCALL_INFO 0x4212
#define PTRACE_SYSCALL_INFO_NONE 0
#define PTRACE_SYSCALL_INFO_ENTRY 1
#define PTRACE_SYSCALL_INFO_EXIT 2
@@ -81,7 +82,8 @@ struct seccomp_metadata {
struct ptrace_syscall_info {
__u8 op; /* PTRACE_SYSCALL_INFO_* */
- __u8 pad[3];
+ __u8 reserved;
+ __u16 flags;
__u32 arch;
__u64 instruction_pointer;
__u64 stack_pointer;
@@ -98,6 +100,7 @@ struct ptrace_syscall_info {
__u64 nr;
__u64 args[6];
__u32 ret_data;
+ __u32 reserved2;
} seccomp;
};
};
@@ -142,6 +145,8 @@ struct ptrace_sud_config {
__u64 len;
};
+/* 0x4212 is PTRACE_SET_SYSCALL_INFO */
+
/*
* These values are stored in task->ptrace_message
* by ptrace_stop to describe the current syscall-stop.
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 22e7d74cf4cd..75a84efad40f 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -944,7 +944,10 @@ ptrace_get_syscall_info_seccomp(struct task_struct *child, struct pt_regs *regs,
ptrace_get_syscall_info_entry(child, regs, info);
info->seccomp.ret_data = child->ptrace_message;
- /* ret_data is the last field in struct ptrace_syscall_info.seccomp */
+ /*
+ * ret_data is the last non-reserved field
+ * in struct ptrace_syscall_info.seccomp
+ */
return offsetofend(struct ptrace_syscall_info, seccomp.ret_data);
}
@@ -1016,6 +1019,118 @@ ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
write_size = min(actual_size, user_size);
return copy_to_user(datavp, &info, write_size) ? -EFAULT : actual_size;
}
+
+static int
+ptrace_set_syscall_info_entry(struct task_struct *child, struct pt_regs *regs,
+ struct ptrace_syscall_info *info)
+{
+ unsigned long args[ARRAY_SIZE(info->entry.args)];
+ int nr = info->entry.nr;
+ int i;
+
+ /*
+ * Check that the syscall number specified in info->entry.nr
+ * is either a value of type "int" or a sign-extended value
+ * of type "int".
+ */
+ if (nr != info->entry.nr)
+ return -ERANGE;
+
+ for (i = 0; i < ARRAY_SIZE(args); i++) {
+ args[i] = info->entry.args[i];
+ /*
+ * Check that the syscall argument specified in
+ * info->entry.args[i] is either a value of type
+ * "unsigned long" or a sign-extended value of type "long".
+ */
+ if (args[i] != info->entry.args[i])
+ return -ERANGE;
+ }
+
+ syscall_set_nr(child, regs, nr);
+ /*
+ * If the syscall number is set to -1, setting syscall arguments is not
+ * just pointless, it would also clobber the syscall return value on
+ * those architectures that share the same register both for the first
+ * argument of syscall and its return value.
+ */
+ if (nr != -1)
+ syscall_set_arguments(child, regs, args);
+
+ return 0;
+}
+
+static int
+ptrace_set_syscall_info_seccomp(struct task_struct *child, struct pt_regs *regs,
+ struct ptrace_syscall_info *info)
+{
+ /*
+ * info->entry is currently a subset of info->seccomp,
+ * info->seccomp.ret_data is currently ignored.
+ */
+ return ptrace_set_syscall_info_entry(child, regs, info);
+}
+
+static int
+ptrace_set_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
+ struct ptrace_syscall_info *info)
+{
+ long rval = info->exit.rval;
+
+ /*
+ * Check that the return value specified in info->exit.rval
+ * is either a value of type "long" or a sign-extended value
+ * of type "long".
+ */
+ if (rval != info->exit.rval)
+ return -ERANGE;
+
+ if (info->exit.is_error)
+ syscall_set_return_value(child, regs, rval, 0);
+ else
+ syscall_set_return_value(child, regs, 0, rval);
+
+ return 0;
+}
+
+static int
+ptrace_set_syscall_info(struct task_struct *child, unsigned long user_size,
+ const void __user *datavp)
+{
+ struct pt_regs *regs = task_pt_regs(child);
+ struct ptrace_syscall_info info;
+
+ if (user_size < sizeof(info))
+ return -EINVAL;
+
+ /*
+ * The compatibility is tracked by info.op and info.flags: if user-space
+ * does not instruct us to use unknown extra bits from future versions
+ * of ptrace_syscall_info, we are not going to read them either.
+ */
+ if (copy_from_user(&info, datavp, sizeof(info)))
+ return -EFAULT;
+
+ /* Reserved for future use. */
+ if (info.flags || info.reserved)
+ return -EINVAL;
+
+ /* Changing the type of the system call stop is not supported yet. */
+ if (ptrace_get_syscall_info_op(child) != info.op)
+ return -EINVAL;
+
+ switch (info.op) {
+ case PTRACE_SYSCALL_INFO_ENTRY:
+ return ptrace_set_syscall_info_entry(child, regs, &info);
+ case PTRACE_SYSCALL_INFO_EXIT:
+ return ptrace_set_syscall_info_exit(child, regs, &info);
+ case PTRACE_SYSCALL_INFO_SECCOMP:
+ return ptrace_set_syscall_info_seccomp(child, regs, &info);
+ default:
+ /* Other types of system call stops are not supported yet. */
+ return -EINVAL;
+ }
+}
#endif /* CONFIG_HAVE_ARCH_TRACEHOOK */
int ptrace_request(struct task_struct *child, long request,
@@ -1234,6 +1349,10 @@ int ptrace_request(struct task_struct *child, long request,
case PTRACE_GET_SYSCALL_INFO:
ret = ptrace_get_syscall_info(child, addr, datavp);
break;
+
+ case PTRACE_SET_SYSCALL_INFO:
+ ret = ptrace_set_syscall_info(child, addr, datavp);
+ break;
#endif
case PTRACE_SECCOMP_GET_FILTER:
--
ldv
^ permalink raw reply related
* [PATCH v4 0/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API
From: Dmitry V. Levin @ 2025-02-03 6:58 UTC (permalink / raw)
To: Andrew Morton
Cc: Oleg Nesterov, Alexey Gladkov, Charlie Jenkins,
Eugene Syromyatnikov, Mike Frysinger, Renzo Davoli,
Davide Berardi, Vineet Gupta, Russell King, Will Deacon, Guo Ren,
Brian Cain, Huacai Chen, WANG Xuerui, Geert Uytterhoeven,
Michal Simek, Thomas Bogendoerfer, Dinh Nguyen, Jonas Bonn,
Stefan Kristiansson, Stafford Horne, James E.J. Bottomley,
Helge Deller, Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N Rao, Madhavan Srinivasan, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Richard Weinberger, Anton Ivanov, Johannes Berg, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Chris Zankel, Max Filippov, Arnd Bergmann, Shuah Khan,
strace-devel, linux-snps-arc, linux-kernel, linux-arm-kernel,
linux-csky, linux-hexagon, loongarch, linux-m68k, linux-mips,
linux-openrisc, linux-parisc, linuxppc-dev, linux-riscv,
linux-s390, linux-sh, sparclinux, linux-um, linux-arch,
linux-kselftest, linux-api
PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements
PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of
system calls the tracee is blocked in.
This API allows ptracers to obtain and modify system call details
in a straightforward and architecture-agnostic way.
Current implementation supports changing only those bits of system call
information that are used by strace, namely, syscall number, syscall
arguments, and syscall return value.
Support of changing additional details returned by PTRACE_GET_SYSCALL_INFO,
such as instruction pointer and stack pointer, could be added later if
needed, by using struct ptrace_syscall_info.flags to specify the additional
details that should be set. Currently, "flags" and "reserved" fields of
struct ptrace_syscall_info must be initialized with zeroes; "arch",
"instruction_pointer", and "stack_pointer" fields are ignored.
PTRACE_SET_SYSCALL_INFO currently supports only PTRACE_SYSCALL_INFO_ENTRY,
PTRACE_SYSCALL_INFO_EXIT, and PTRACE_SYSCALL_INFO_SECCOMP operations.
Other operations could be added later if needed.
Ideally, PTRACE_SET_SYSCALL_INFO should have been introduced along with
PTRACE_GET_SYSCALL_INFO, but it didn't happen. The last straw that
convinced me to implement PTRACE_SET_SYSCALL_INFO was apparent failure
to provide an API of changing the first system call argument on riscv
architecture [1].
ptrace(2) man page:
long ptrace(enum __ptrace_request request, pid_t pid, void *addr, void *data);
...
PTRACE_SET_SYSCALL_INFO
Modify information about the system call that caused the stop.
The "data" argument is a pointer to struct ptrace_syscall_info
that specifies the system call information to be set.
The "addr" argument should be set to sizeof(struct ptrace_syscall_info)).
[1] https://lore.kernel.org/all/59505464-c84a-403d-972f-d4b2055eeaac@gmail.com/
Notes:
v4:
* Split out syscall_set_return_value() for hexagon into a separate patch
* s390: Change the style of syscall_set_arguments() implementation as
requested
* Add more Reviewed-by
* v3: https://lore.kernel.org/all/20250128091445.GA8257@strace.io/
v3:
* powerpc: Submit syscall_set_return_value() fix for "sc" case separately
* mips: Do not introduce erroneous argument truncation on mips n32,
add a detailed description to the commit message of the
mips_get_syscall_arg() change
* ptrace: Add explicit padding to the end of struct ptrace_syscall_info,
simplify obtaining of user ptrace_syscall_info,
do not introduce PTRACE_SYSCALL_INFO_SIZE_VER0
* ptrace: Change the return type of ptrace_set_syscall_info_* functions
from "unsigned long" to "int"
* ptrace: Add -ERANGE check to ptrace_set_syscall_info_exit(),
add comments to -ERANGE checks
* ptrace: Update comments about supported syscall stops
* selftests: Extend set_syscall_info test, fix for mips n32
* Add Tested-by and Reviewed-by
v2:
* Add patch to fix syscall_set_return_value() on powerpc
* Add patch to fix mips_get_syscall_arg() on mips
* Add syscall_set_return_value() implementation on hexagon
* Add syscall_set_return_value() invocation to syscall_set_nr()
on arm and arm64.
* Fix syscall_set_nr() and mips_set_syscall_arg() on mips
* Add a comment to syscall_set_nr() on arc, powerpc, s390, sh,
and sparc
* Remove redundant ptrace_syscall_info.op assignments in
ptrace_get_syscall_info_*
* Minor style tweaks in ptrace_get_syscall_info_op()
* Remove syscall_set_return_value() invocation from
ptrace_set_syscall_info_entry()
* Skip syscall_set_arguments() invocation in case of syscall number -1
in ptrace_set_syscall_info_entry()
* Split ptrace_syscall_info.reserved into ptrace_syscall_info.reserved
and ptrace_syscall_info.flags
* Use __kernel_ulong_t instead of unsigned long in set_syscall_info test
v1:
Dmitry V. Levin (7):
mips: fix mips_get_syscall_arg() for o32
hexagon: add syscall_set_return_value()
syscall.h: add syscall_set_arguments()
syscall.h: introduce syscall_set_nr()
ptrace_get_syscall_info: factor out ptrace_get_syscall_info_op
ptrace: introduce PTRACE_SET_SYSCALL_INFO request
selftests/ptrace: add a test case for PTRACE_SET_SYSCALL_INFO
arch/arc/include/asm/syscall.h | 25 +
arch/arm/include/asm/syscall.h | 37 ++
arch/arm64/include/asm/syscall.h | 29 +
arch/csky/include/asm/syscall.h | 13 +
arch/hexagon/include/asm/syscall.h | 21 +
arch/loongarch/include/asm/syscall.h | 15 +
arch/m68k/include/asm/syscall.h | 7 +
arch/microblaze/include/asm/syscall.h | 7 +
arch/mips/include/asm/syscall.h | 70 ++-
arch/nios2/include/asm/syscall.h | 16 +
arch/openrisc/include/asm/syscall.h | 13 +
arch/parisc/include/asm/syscall.h | 19 +
arch/powerpc/include/asm/syscall.h | 20 +
arch/riscv/include/asm/syscall.h | 16 +
arch/s390/include/asm/syscall.h | 21 +
arch/sh/include/asm/syscall_32.h | 24 +
arch/sparc/include/asm/syscall.h | 22 +
arch/um/include/asm/syscall-generic.h | 19 +
arch/x86/include/asm/syscall.h | 43 ++
arch/xtensa/include/asm/syscall.h | 18 +
include/asm-generic/syscall.h | 30 +
include/uapi/linux/ptrace.h | 7 +-
kernel/ptrace.c | 179 +++++-
tools/testing/selftests/ptrace/Makefile | 2 +-
.../selftests/ptrace/set_syscall_info.c | 514 ++++++++++++++++++
25 files changed, 1140 insertions(+), 47 deletions(-)
create mode 100644 tools/testing/selftests/ptrace/set_syscall_info.c
--
ldv
^ permalink raw reply
* Re: [PATCH v4 0/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API
From: Alexander Gordeev @ 2025-02-03 9:29 UTC (permalink / raw)
To: Dmitry V. Levin
Cc: Andrew Morton, Oleg Nesterov, Alexey Gladkov, Charlie Jenkins,
Eugene Syromyatnikov, Mike Frysinger, Renzo Davoli,
Davide Berardi, Vineet Gupta, Russell King, Will Deacon, Guo Ren,
Brian Cain, Huacai Chen, WANG Xuerui, Geert Uytterhoeven,
Michal Simek, Thomas Bogendoerfer, Dinh Nguyen, Jonas Bonn,
Stefan Kristiansson, Stafford Horne, James E.J. Bottomley,
Helge Deller, Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N Rao, Madhavan Srinivasan, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Richard Weinberger, Anton Ivanov, Johannes Berg, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Chris Zankel, Max Filippov, Arnd Bergmann, Shuah Khan,
strace-devel, linux-snps-arc, linux-kernel, linux-arm-kernel,
linux-csky, linux-hexagon, loongarch, linux-m68k, linux-mips,
linux-openrisc, linux-parisc, linuxppc-dev, linux-riscv,
linux-s390, linux-sh, sparclinux, linux-um, linux-arch,
linux-kselftest, linux-api
In-Reply-To: <20250203065849.GA14120@strace.io>
On Mon, Feb 03, 2025 at 08:58:49AM +0200, Dmitry V. Levin wrote:
Hi Dmitry,
> PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements
> PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of
> system calls the tracee is blocked in.
...
FWIW, I am getting these on s390:
# ./tools/testing/selftests/ptrace/set_syscall_info
TAP version 13
1..1
# Starting 1 tests from 1 test cases.
# RUN global.set_syscall_info ...
# set_syscall_info.c:87:set_syscall_info:Expected exp_entry->nr (-1) == info->entry.nr (65535)
# set_syscall_info.c:88:set_syscall_info:wait #3: PTRACE_GET_SYSCALL_INFO #2: syscall nr mismatch
# set_syscall_info: Test terminated by assertion
# FAIL global.set_syscall_info
not ok 1 global.set_syscall_info
# FAILED: 0 / 1 tests passed.
# Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0
I remember one of the earlier versions (v1 or v2) was working for me.
Thanks!
^ permalink raw reply
* Re: [PATCH v4 0/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API
From: Dmitry V. Levin @ 2025-02-03 10:35 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-snps-arc, Rich Felker, Thomas Gleixner, Andreas Larsson,
John Paul Adrian Glaubitz, x86, linux-kernel,
James E.J. Bottomley, Guo Ren, linux-csky, linux-kselftest,
H. Peter Anvin, sparclinux, linux-hexagon, WANG Xuerui, linux-api,
Will Deacon, Eugene Syromyatnikov, Anton Ivanov, Jonas Bonn,
linux-s390, Madhavan Srinivasan, Vasily Gorbik, Yoshinori Sato,
linux-sh, Michael Ellerman, Helge Deller, Huacai Chen,
Russell King, Christophe Leroy, Dave Hansen, Ingo Molnar,
Geert Uytterhoeven, Vineet Gupta, Christian Borntraeger,
Arnd Bergmann, linux-arch, Shuah Khan, Albert Ou, Mike Frysinger,
Davide Berardi, Renzo Davoli, linux-um, Heiko Carstens,
strace-devel, Charlie Jenkins, Naveen N Rao, Nicholas Piggin,
Stefan Kristiansson, linux-m68k, Borislav Petkov, loongarch,
Paul Walmsley, Stafford Horne, Johannes Berg, linux-arm-kernel,
Brian Cain, Michal Simek, Thomas Bogendoerfer, linux-parisc,
linux-openrisc, linux-mips, linuxppc-dev, Oleg Nesterov,
Dinh Nguyen, linux-riscv, Palmer Dabbelt, Sven Schnelle,
Richard Weinberger, Andrew Morton, Alexey Gladkov,
David S. Miller
In-Reply-To: <Z6CMgVm8QKEMRf8L@li-008a6a4c-3549-11b2-a85c-c5cc2836eea2.ibm.com>
On Mon, Feb 03, 2025 at 10:29:37AM +0100, Alexander Gordeev wrote:
> On Mon, Feb 03, 2025 at 08:58:49AM +0200, Dmitry V. Levin wrote:
>
> Hi Dmitry,
>
> > PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements
> > PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of
> > system calls the tracee is blocked in.
> ...
>
> FWIW, I am getting these on s390:
>
> # ./tools/testing/selftests/ptrace/set_syscall_info
> TAP version 13
> 1..1
> # Starting 1 tests from 1 test cases.
> # RUN global.set_syscall_info ...
> # set_syscall_info.c:87:set_syscall_info:Expected exp_entry->nr (-1) == info->entry.nr (65535)
> # set_syscall_info.c:88:set_syscall_info:wait #3: PTRACE_GET_SYSCALL_INFO #2: syscall nr mismatch
> # set_syscall_info: Test terminated by assertion
> # FAIL global.set_syscall_info
> not ok 1 global.set_syscall_info
> # FAILED: 0 / 1 tests passed.
> # Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0
>
> I remember one of the earlier versions (v1 or v2) was working for me.
>
> Thanks!
In v3, this test was extended to check whether PTRACE_GET_SYSCALL_INFO
called immediately after PTRACE_SET_SYSCALL_INFO returns the same syscall
number, and on s390 it apparently doesn't, thanks to its implementation
of syscall_get_nr() that returns 0xffff in this case.
To workaround this, we could either change syscall_get_nr() to return -1
in this case, or add an #ifdef __s390x__ exception to the test.
What would you prefer?
--
ldv
^ permalink raw reply
* Re: [PATCH 0/2 v8] add ioctl/sysfs to donate file-backed pages
From: Christoph Hellwig @ 2025-02-04 5:49 UTC (permalink / raw)
To: Jaegeuk Kim
Cc: linux-kernel, linux-f2fs-devel, linux-mm, linux-api,
linux-fsdevel
In-Reply-To: <20250131222914.1634961-1-jaegeuk@kernel.org>
On Fri, Jan 31, 2025 at 10:27:55PM +0000, Jaegeuk Kim wrote:
> Note, let me keep improving this patch set, while trying to get some feedbacks
> from MM and API folks from [1].
Please actually drive it instead of only interacting once after
I told you to. The feedback is clearly that it is a MM thing, so please
drive it forward instead of going back to the hacky file system version.
>
> If users clearly know which file-backed pages to reclaim in system view, they
> can use this ioctl() to register in advance and reclaim all at once later.
>
> I'd like to propose this API in F2FS only, since
> 1) the use-case is quite limited in Android at the moment. Once it's generall
> accepted with more use-cases, happy to propose a generic API such as fadvise.
> Please chime in, if there's any needs.
>
> 2) it's file-backed pages which requires to maintain the list of inode objects.
> I'm not sure this fits in MM tho, also happy to listen to any feedback.
>
> [1] https://lore.kernel.org/lkml/Z4qmF2n2pzuHqad_@google.com/
>
> Change log from v7:
> - change the sysfs entry to reclaim pages in all f2fs mounts
>
> Change log from v6:
> - change sysfs entry name to reclaim_caches_kb
>
> Jaegeuk Kim (2):
> f2fs: register inodes which is able to donate pages
> f2fs: add a sysfs entry to request donate file-backed pages
>
> Jaegeuk Kim (2):
> f2fs: register inodes which is able to donate pages
> f2fs: add a sysfs entry to request donate file-backed pages
>
> Documentation/ABI/testing/sysfs-fs-f2fs | 7 ++
> fs/f2fs/debug.c | 3 +
> fs/f2fs/f2fs.h | 14 +++-
> fs/f2fs/file.c | 60 +++++++++++++++++
> fs/f2fs/inode.c | 14 ++++
> fs/f2fs/shrinker.c | 90 +++++++++++++++++++++++++
> fs/f2fs/super.c | 1 +
> fs/f2fs/sysfs.c | 63 +++++++++++++++++
> include/uapi/linux/f2fs.h | 7 ++
> 9 files changed, 258 insertions(+), 1 deletion(-)
>
> --
> 2.48.1.362.g079036d154-goog
>
>
---end quoted text---
^ permalink raw reply
* Re: [PATCH v7 0/6] introduce PIDFD_SELF* sentinels
From: Christian Brauner @ 2025-02-04 9:46 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Christian Brauner, Shuah Khan, Liam R . Howlett,
Suren Baghdasaryan, Vlastimil Babka, pedro.falcato,
linux-kselftest, linux-mm, linux-fsdevel, linux-api, linux-kernel,
Oliver Sang, John Hubbard, Tejun Heo, Johannes Weiner,
Michal Koutny, Andrew Morton, Shakeel Butt
In-Reply-To: <cover.1738268370.git.lorenzo.stoakes@oracle.com>
On Thu, 30 Jan 2025 20:40:25 +0000, Lorenzo Stoakes wrote:
> If you wish to utilise a pidfd interface to refer to the current process or
> thread it is rather cumbersome, requiring something like:
>
> int pidfd = pidfd_open(getpid(), 0 or PIDFD_THREAD);
>
> ...
>
> [...]
Updated merge message. I've slightly rearranged pidfd_send_signal() so
we don't have to call CLASS(fd, f)(pidfd) unconditionally anymore.
---
Applied to the vfs-6.15.pidfs branch of the vfs/vfs.git tree.
Patches in the vfs-6.15.pidfs branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-6.15.pidfs
[1/6] pidfd: add PIDFD_SELF* sentinels to refer to own thread/process
https://git.kernel.org/vfs/vfs/c/e6e4ed42f8d8
[2/6] selftests/pidfd: add missing system header imcludes to pidfd tests
https://git.kernel.org/vfs/vfs/c/c9f04f4a251d
[3/6] tools: testing: separate out wait_for_pid() into helper header
https://git.kernel.org/vfs/vfs/c/fb67fe44116e
[4/6] selftests: pidfd: add pidfd.h UAPI wrapper
https://git.kernel.org/vfs/vfs/c/ac331e56724d
[5/6] selftests: pidfd: add tests for PIDFD_SELF_*
https://git.kernel.org/vfs/vfs/c/881a3515c191
[6/6] selftests/mm: use PIDFD_SELF in guard pages test
https://git.kernel.org/vfs/vfs/c/b4703f056f42
^ permalink raw reply
* Re: [PATCH v7 0/6] introduce PIDFD_SELF* sentinels
From: Lorenzo Stoakes @ 2025-02-04 10:01 UTC (permalink / raw)
To: Christian Brauner
Cc: Shuah Khan, Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, linux-kernel, Oliver Sang, John Hubbard, Tejun Heo,
Johannes Weiner, Michal Koutny, Andrew Morton, Shakeel Butt
In-Reply-To: <20250204-joggen-buddeln-29e5ca75abb7@brauner>
On Tue, Feb 04, 2025 at 10:46:35AM +0100, Christian Brauner wrote:
> On Thu, 30 Jan 2025 20:40:25 +0000, Lorenzo Stoakes wrote:
> > If you wish to utilise a pidfd interface to refer to the current process or
> > thread it is rather cumbersome, requiring something like:
> >
> > int pidfd = pidfd_open(getpid(), 0 or PIDFD_THREAD);
> >
> > ...
> >
> > [...]
>
> Updated merge message. I've slightly rearranged pidfd_send_signal() so
> we don't have to call CLASS(fd, f)(pidfd) unconditionally anymore.
Sounds good and thank you! Glad to get this in :)
>
> ---
>
> Applied to the vfs-6.15.pidfs branch of the vfs/vfs.git tree.
> Patches in the vfs-6.15.pidfs branch should appear in linux-next soon.
>
> Please report any outstanding bugs that were missed during review in a
> new review to the original patch series allowing us to drop it.
>
> It's encouraged to provide Acked-bys and Reviewed-bys even though the
> patch has now been applied. If possible patch trailers will be updated.
>
> Note that commit hashes shown below are subject to change due to rebase,
> trailer updates or similar. If in doubt, please check the listed branch.
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> branch: vfs-6.15.pidfs
>
> [1/6] pidfd: add PIDFD_SELF* sentinels to refer to own thread/process
> https://git.kernel.org/vfs/vfs/c/e6e4ed42f8d8
> [2/6] selftests/pidfd: add missing system header imcludes to pidfd tests
> https://git.kernel.org/vfs/vfs/c/c9f04f4a251d
> [3/6] tools: testing: separate out wait_for_pid() into helper header
> https://git.kernel.org/vfs/vfs/c/fb67fe44116e
> [4/6] selftests: pidfd: add pidfd.h UAPI wrapper
> https://git.kernel.org/vfs/vfs/c/ac331e56724d
> [5/6] selftests: pidfd: add tests for PIDFD_SELF_*
> https://git.kernel.org/vfs/vfs/c/881a3515c191
> [6/6] selftests/mm: use PIDFD_SELF in guard pages test
> https://git.kernel.org/vfs/vfs/c/b4703f056f42
^ permalink raw reply
* Re: [PATCH 0/2 v8] add ioctl/sysfs to donate file-backed pages
From: Jaegeuk Kim @ 2025-02-04 16:26 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-kernel, linux-f2fs-devel, linux-mm, linux-api,
linux-fsdevel
In-Reply-To: <Z6GqbJxJAsRPQ4uQ@infradead.org>
On 02/03, Christoph Hellwig wrote:
> On Fri, Jan 31, 2025 at 10:27:55PM +0000, Jaegeuk Kim wrote:
> > Note, let me keep improving this patch set, while trying to get some feedbacks
> > from MM and API folks from [1].
>
> Please actually drive it instead of only interacting once after
> I told you to. The feedback is clearly that it is a MM thing, so please
> drive it forward instead of going back to the hacky file system version.
I keep saying working in parallel for production. And, no worries, I won't
merge this to -next until I get the feedback from the MM folks. I was
waiting for a couple of weeks before bothering them, so will ping there.
>
> >
> > If users clearly know which file-backed pages to reclaim in system view, they
> > can use this ioctl() to register in advance and reclaim all at once later.
> >
> > I'd like to propose this API in F2FS only, since
> > 1) the use-case is quite limited in Android at the moment. Once it's generall
> > accepted with more use-cases, happy to propose a generic API such as fadvise.
> > Please chime in, if there's any needs.
> >
> > 2) it's file-backed pages which requires to maintain the list of inode objects.
> > I'm not sure this fits in MM tho, also happy to listen to any feedback.
> >
> > [1] https://lore.kernel.org/lkml/Z4qmF2n2pzuHqad_@google.com/
> >
> > Change log from v7:
> > - change the sysfs entry to reclaim pages in all f2fs mounts
> >
> > Change log from v6:
> > - change sysfs entry name to reclaim_caches_kb
> >
> > Jaegeuk Kim (2):
> > f2fs: register inodes which is able to donate pages
> > f2fs: add a sysfs entry to request donate file-backed pages
> >
> > Jaegeuk Kim (2):
> > f2fs: register inodes which is able to donate pages
> > f2fs: add a sysfs entry to request donate file-backed pages
> >
> > Documentation/ABI/testing/sysfs-fs-f2fs | 7 ++
> > fs/f2fs/debug.c | 3 +
> > fs/f2fs/f2fs.h | 14 +++-
> > fs/f2fs/file.c | 60 +++++++++++++++++
> > fs/f2fs/inode.c | 14 ++++
> > fs/f2fs/shrinker.c | 90 +++++++++++++++++++++++++
> > fs/f2fs/super.c | 1 +
> > fs/f2fs/sysfs.c | 63 +++++++++++++++++
> > include/uapi/linux/f2fs.h | 7 ++
> > 9 files changed, 258 insertions(+), 1 deletion(-)
> >
> > --
> > 2.48.1.362.g079036d154-goog
> >
> >
> ---end quoted text---
^ permalink raw reply
* Re: [PATCH v7 1/6] pidfd: add PIDFD_SELF* sentinels to refer to own thread/process
From: Shakeel Butt @ 2025-02-04 16:51 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Christian Brauner, Shuah Khan, Liam R . Howlett,
Suren Baghdasaryan, Vlastimil Babka, pedro.falcato,
linux-kselftest, linux-mm, linux-fsdevel, linux-api, linux-kernel,
Oliver Sang, John Hubbard, Tejun Heo, Johannes Weiner,
Michal Koutny, Andrew Morton
In-Reply-To: <24315a16a3d01a548dd45c7515f7d51c767e954e.1738268370.git.lorenzo.stoakes@oracle.com>
On Thu, Jan 30, 2025 at 08:40:26PM +0000, Lorenzo Stoakes wrote:
> It is useful to be able to utilise the pidfd mechanism to reference the
> current thread or process (from a userland point of view - thread group
> leader from the kernel's point of view).
>
> Therefore introduce PIDFD_SELF_THREAD to refer to the current thread, and
> PIDFD_SELF_THREAD_GROUP to refer to the current thread group leader.
>
> For convenience and to avoid confusion from userland's perspective we alias
> these:
>
> * PIDFD_SELF is an alias for PIDFD_SELF_THREAD - This is nearly always what
> the user will want to use, as they would find it surprising if for
> instance fd's were unshared()'d and they wanted to invoke pidfd_getfd()
> and that failed.
>
> * PIDFD_SELF_PROCESS is an alias for PIDFD_SELF_THREAD_GROUP - Most users
> have no concept of thread groups or what a thread group leader is, and
> from userland's perspective and nomenclature this is what userland
> considers to be a process.
>
> We adjust pidfd_get_task() and the pidfd_send_signal() system call with
> specific handling for this, implementing this functionality for
> process_madvise(), process_mrelease() (albeit, using it here wouldn't
> really make sense) and pidfd_send_signal().
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
^ permalink raw reply
* Re: [PATCH v7 0/6] introduce PIDFD_SELF* sentinels
From: Suren Baghdasaryan @ 2025-02-04 17:43 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Christian Brauner, Shuah Khan, Liam R . Howlett, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, linux-kernel, Oliver Sang, John Hubbard, Tejun Heo,
Johannes Weiner, Michal Koutny, Andrew Morton, Shakeel Butt,
Elliott Hughes
In-Reply-To: <7a8a1719-466f-4e10-b1eb-9e9e1ef8ad52@lucifer.local>
On Tue, Feb 4, 2025 at 2:01 AM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> On Tue, Feb 04, 2025 at 10:46:35AM +0100, Christian Brauner wrote:
> > On Thu, 30 Jan 2025 20:40:25 +0000, Lorenzo Stoakes wrote:
> > > If you wish to utilise a pidfd interface to refer to the current process or
> > > thread it is rather cumbersome, requiring something like:
> > >
> > > int pidfd = pidfd_open(getpid(), 0 or PIDFD_THREAD);
> > >
> > > ...
> > >
> > > [...]
> >
> > Updated merge message. I've slightly rearranged pidfd_send_signal() so
> > we don't have to call CLASS(fd, f)(pidfd) unconditionally anymore.
>
> Sounds good and thank you! Glad to get this in :)
Sorry, a bit late to the party...
We were discussing MADV_GUARD_INSTALL use with Android Bionic team and
the possibility of caching pidfd_open() result for reuse when
installing multiple guards, however doing that in libraries would pose
issues as we can't predict the user behavior, which can fork() in
between such calls. That would be an additional reason why having
these sentinels is beneficial.
>
> >
> > ---
> >
> > Applied to the vfs-6.15.pidfs branch of the vfs/vfs.git tree.
> > Patches in the vfs-6.15.pidfs branch should appear in linux-next soon.
> >
> > Please report any outstanding bugs that were missed during review in a
> > new review to the original patch series allowing us to drop it.
> >
> > It's encouraged to provide Acked-bys and Reviewed-bys even though the
> > patch has now been applied. If possible patch trailers will be updated.
> >
> > Note that commit hashes shown below are subject to change due to rebase,
> > trailer updates or similar. If in doubt, please check the listed branch.
> >
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> > branch: vfs-6.15.pidfs
> >
> > [1/6] pidfd: add PIDFD_SELF* sentinels to refer to own thread/process
> > https://git.kernel.org/vfs/vfs/c/e6e4ed42f8d8
> > [2/6] selftests/pidfd: add missing system header imcludes to pidfd tests
> > https://git.kernel.org/vfs/vfs/c/c9f04f4a251d
> > [3/6] tools: testing: separate out wait_for_pid() into helper header
> > https://git.kernel.org/vfs/vfs/c/fb67fe44116e
> > [4/6] selftests: pidfd: add pidfd.h UAPI wrapper
> > https://git.kernel.org/vfs/vfs/c/ac331e56724d
> > [5/6] selftests: pidfd: add tests for PIDFD_SELF_*
> > https://git.kernel.org/vfs/vfs/c/881a3515c191
> > [6/6] selftests/mm: use PIDFD_SELF in guard pages test
> > https://git.kernel.org/vfs/vfs/c/b4703f056f42
^ permalink raw reply
* Re: [PATCH v7 2/6] selftests/pidfd: add missing system header imcludes to pidfd tests
From: Shakeel Butt @ 2025-02-05 5:13 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Christian Brauner, Shuah Khan, Liam R . Howlett,
Suren Baghdasaryan, Vlastimil Babka, pedro.falcato,
linux-kselftest, linux-mm, linux-fsdevel, linux-api, linux-kernel,
Oliver Sang, John Hubbard, Tejun Heo, Johannes Weiner,
Michal Koutny, Andrew Morton
In-Reply-To: <fab8843ea8664b5089f95ccfdcfd5bd7a5a6bb0b.1738268370.git.lorenzo.stoakes@oracle.com>
On Thu, Jan 30, 2025 at 08:40:27PM +0000, Lorenzo Stoakes wrote:
> The pidfd_fdinfo_test.c and pidfd_setns_test.c tests appear to be missing
> fundamental system header imports required to execute correctly. Add these.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
^ permalink raw reply
* Re: [PATCH v7 3/6] tools: testing: separate out wait_for_pid() into helper header
From: Shakeel Butt @ 2025-02-05 5:15 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Christian Brauner, Shuah Khan, Liam R . Howlett,
Suren Baghdasaryan, Vlastimil Babka, pedro.falcato,
linux-kselftest, linux-mm, linux-fsdevel, linux-api, linux-kernel,
Oliver Sang, John Hubbard, Tejun Heo, Johannes Weiner,
Michal Koutny, Andrew Morton
In-Reply-To: <c611be6f7df6aa85f72c6d4d329b30bbe4a0fbed.1738268370.git.lorenzo.stoakes@oracle.com>
On Thu, Jan 30, 2025 at 08:40:28PM +0000, Lorenzo Stoakes wrote:
> It seems tests other than the pidfd tests use the wait_for_pid() function
> declared in pidfd.h.
>
> Since we will shortly be modifying pidfd.h in a way that might clash with
> other tests, separate this out and update tests accordingly.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
^ permalink raw reply
* Re: [PATCH v7 5/6] selftests: pidfd: add tests for PIDFD_SELF_*
From: Shakeel Butt @ 2025-02-05 5:27 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Christian Brauner, Shuah Khan, Liam R . Howlett,
Suren Baghdasaryan, Vlastimil Babka, pedro.falcato,
linux-kselftest, linux-mm, linux-fsdevel, linux-api, linux-kernel,
Oliver Sang, John Hubbard, Tejun Heo, Johannes Weiner,
Michal Koutny, Andrew Morton
In-Reply-To: <7ab0e48b26ba53abf7b703df2dd11a2e99b8efb2.1738268370.git.lorenzo.stoakes@oracle.com>
On Thu, Jan 30, 2025 at 08:40:30PM +0000, Lorenzo Stoakes wrote:
> Add tests to assert that PIDFD_SELF* correctly refers to the current
> thread and process.
>
> We explicitly test pidfd_send_signal(), however We defer testing of
> mm-specific functionality which uses pidfd, namely process_madvise() and
> process_mrelease() to mm testing (though note the latter can not be
> sensibly tested as it would require the testing process to be dying).
>
> We also correct the pidfd_open_test.c fields which refer to .request_mask
> whereas the UAPI header refers to .mask, which otherwise break the import
> of the UAPI header.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
^ permalink raw reply
* Re: [PATCH v7 6/6] selftests/mm: use PIDFD_SELF in guard pages test
From: Shakeel Butt @ 2025-02-05 5:28 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Christian Brauner, Shuah Khan, Liam R . Howlett,
Suren Baghdasaryan, Vlastimil Babka, pedro.falcato,
linux-kselftest, linux-mm, linux-fsdevel, linux-api, linux-kernel,
Oliver Sang, John Hubbard, Tejun Heo, Johannes Weiner,
Michal Koutny, Andrew Morton
In-Reply-To: <69fbbe088d3424de9983e145228459cb05a8f13d.1738268370.git.lorenzo.stoakes@oracle.com>
On Thu, Jan 30, 2025 at 08:40:31PM +0000, Lorenzo Stoakes wrote:
> Now we have PIDFD_SELF available for process_madvise(), make use of it in
> the guard pages test.
>
> This is both more convenient and asserts that PIDFD_SELF works as expected.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
^ permalink raw reply
* Re: [PATCH v7 0/6] introduce PIDFD_SELF* sentinels
From: Christian Brauner @ 2025-02-05 9:29 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Lorenzo Stoakes, Shuah Khan, Liam R . Howlett, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, linux-kernel, Oliver Sang, John Hubbard, Tejun Heo,
Johannes Weiner, Michal Koutny, Andrew Morton, Shakeel Butt,
Elliott Hughes
In-Reply-To: <CAJuCfpEUusRt_ss7RtxRPP9q_LRwi+Lw+SOq32EUA58s3JOx1A@mail.gmail.com>
On Tue, Feb 04, 2025 at 09:43:31AM -0800, Suren Baghdasaryan wrote:
> On Tue, Feb 4, 2025 at 2:01 AM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > On Tue, Feb 04, 2025 at 10:46:35AM +0100, Christian Brauner wrote:
> > > On Thu, 30 Jan 2025 20:40:25 +0000, Lorenzo Stoakes wrote:
> > > > If you wish to utilise a pidfd interface to refer to the current process or
> > > > thread it is rather cumbersome, requiring something like:
> > > >
> > > > int pidfd = pidfd_open(getpid(), 0 or PIDFD_THREAD);
> > > >
> > > > ...
> > > >
> > > > [...]
> > >
> > > Updated merge message. I've slightly rearranged pidfd_send_signal() so
> > > we don't have to call CLASS(fd, f)(pidfd) unconditionally anymore.
> >
> > Sounds good and thank you! Glad to get this in :)
>
> Sorry, a bit late to the party...
>
> We were discussing MADV_GUARD_INSTALL use with Android Bionic team and
> the possibility of caching pidfd_open() result for reuse when
> installing multiple guards, however doing that in libraries would pose
> issues as we can't predict the user behavior, which can fork() in
> between such calls. That would be an additional reason why having
> these sentinels is beneficial.
Ok, added this to the cover letter as well.
Note that starting with v6.14 pidfs supports file handles.
This works because pidfs provides each pidfd with a unique 64bit inode
number that is exposed in statx(). On 64-bit the ->st_ino simply is the
inode number. On 32-bit the unique identifier can be reconstructed using
->st_ino and the inode generation number which can be retrieved via the
FS_IOC_GETVERSION ioctl. So the 64-bit identifier on 32-bit is
reconstructed by using ->st_ino as the lower 32-bits and the 32-bit
generation number as the upper 32-bits.
Also note that since the introduction of pidfs each struct pid will
refer to a different inode but the same struct pid will refer to the
same inode if it's opened multiple times. In contrast to pre-pidfs
pidfds where each struct pid refered to the same inode.
IOW, with pidfs statx() is sufficient to compare to pidfds whether they
refer to the same process. On 64-bit it's sufficient to do the usual
st1->st_dev == st2->st_dev && st1->st_ino == st2->st_ino and on 32-bit
you will want to also compare the generation number:
TEST_F(pidfd_bind_mount, reopen)
{
int pidfd;
char proc_path[PATH_MAX];
sprintf(proc_path, "/proc/self/fd/%d", self->pidfd);
pidfd = open(proc_path, O_RDONLY | O_NOCTTY | O_CLOEXEC);
ASSERT_GE(pidfd, 0);
ASSERT_GE(fstat(self->pidfd, &self->st2), 0);
ASSERT_EQ(ioctl(self->pidfd, FS_IOC_GETVERSION, &self->gen2), 0);
ASSERT_TRUE(self->st1.st_dev == self->st2.st_dev && self->st1.st_ino == self->st2.st_ino);
ASSERT_TRUE(self->gen1 == self->gen2);
ASSERT_EQ(close(pidfd), 0);
}
Plus, you can bind-mount them now.
In any case, this allows us to create file handles that are unique for
the lifetime of the system. Please see
tools/testing/selftests/pidfd/pidfd_file_handle_test.c
for how that works. The gist is that decoding and encoding for pidfs is
unprivileged and the only requirement we have is that the process the
file handle resolves to must be valid in the caller's pid namespace
hierarchy:
TEST_F(file_handle, file_handle_child_pidns)
{
int mnt_id;
struct file_handle *fh;
int pidfd = -EBADF;
struct stat st1, st2;
fh = malloc(sizeof(struct file_handle) + MAX_HANDLE_SZ);
ASSERT_NE(fh, NULL);
memset(fh, 0, sizeof(struct file_handle) + MAX_HANDLE_SZ);
fh->handle_bytes = MAX_HANDLE_SZ;
ASSERT_EQ(name_to_handle_at(self->child_pidfd2, "", fh, &mnt_id, AT_EMPTY_PATH), 0);
ASSERT_EQ(fstat(self->child_pidfd2, &st1), 0);
pidfd = open_by_handle_at(self->pidfd, fh, 0);
ASSERT_GE(pidfd, 0);
ASSERT_EQ(fstat(pidfd, &st2), 0);
ASSERT_TRUE(st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino);
ASSERT_EQ(close(pidfd), 0);
pidfd = open_by_handle_at(self->pidfd, fh, O_CLOEXEC);
ASSERT_GE(pidfd, 0);
ASSERT_EQ(fstat(pidfd, &st2), 0);
ASSERT_TRUE(st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino);
ASSERT_EQ(close(pidfd), 0);
pidfd = open_by_handle_at(self->pidfd, fh, O_NONBLOCK);
ASSERT_GE(pidfd, 0);
ASSERT_EQ(fstat(pidfd, &st2), 0);
ASSERT_TRUE(st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino);
ASSERT_EQ(close(pidfd), 0);
free(fh);
}
So you don't need to keep the fd open.
>
>
> >
> > >
> > > ---
> > >
> > > Applied to the vfs-6.15.pidfs branch of the vfs/vfs.git tree.
> > > Patches in the vfs-6.15.pidfs branch should appear in linux-next soon.
> > >
> > > Please report any outstanding bugs that were missed during review in a
> > > new review to the original patch series allowing us to drop it.
> > >
> > > It's encouraged to provide Acked-bys and Reviewed-bys even though the
> > > patch has now been applied. If possible patch trailers will be updated.
> > >
> > > Note that commit hashes shown below are subject to change due to rebase,
> > > trailer updates or similar. If in doubt, please check the listed branch.
> > >
> > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> > > branch: vfs-6.15.pidfs
> > >
> > > [1/6] pidfd: add PIDFD_SELF* sentinels to refer to own thread/process
> > > https://git.kernel.org/vfs/vfs/c/e6e4ed42f8d8
> > > [2/6] selftests/pidfd: add missing system header imcludes to pidfd tests
> > > https://git.kernel.org/vfs/vfs/c/c9f04f4a251d
> > > [3/6] tools: testing: separate out wait_for_pid() into helper header
> > > https://git.kernel.org/vfs/vfs/c/fb67fe44116e
> > > [4/6] selftests: pidfd: add pidfd.h UAPI wrapper
> > > https://git.kernel.org/vfs/vfs/c/ac331e56724d
> > > [5/6] selftests: pidfd: add tests for PIDFD_SELF_*
> > > https://git.kernel.org/vfs/vfs/c/881a3515c191
> > > [6/6] selftests/mm: use PIDFD_SELF in guard pages test
> > > https://git.kernel.org/vfs/vfs/c/b4703f056f42
^ permalink raw reply
* Re: [PATCH v7 2/6] selftests/pidfd: add missing system header imcludes to pidfd tests
From: Peter Seiderer @ 2025-02-05 12:06 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Christian Brauner, Shuah Khan, Liam R . Howlett,
Suren Baghdasaryan, Vlastimil Babka, pedro.falcato,
linux-kselftest, linux-mm, linux-fsdevel, linux-api, linux-kernel,
Oliver Sang, John Hubbard, Tejun Heo, Johannes Weiner,
Michal Koutny, Andrew Morton, Shakeel Butt
In-Reply-To: <fab8843ea8664b5089f95ccfdcfd5bd7a5a6bb0b.1738268370.git.lorenzo.stoakes@oracle.com>
Hello *,
On Thu, 30 Jan 2025 20:40:27 +0000, Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> The pidfd_fdinfo_test.c and pidfd_setns_test.c tests appear to be missing
> fundamental system header imports required to execute correctly. Add these.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
> tools/testing/selftests/pidfd/pidfd_fdinfo_test.c | 1 +
> tools/testing/selftests/pidfd/pidfd_setns_test.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
> index f062a986e382..f718aac75068 100644
> --- a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
> +++ b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
> @@ -13,6 +13,7 @@
> #include <syscall.h>
> #include <sys/wait.h>
> #include <sys/mman.h>
> +#include <sys/mount.h>
>
> #include "pidfd.h"
> #include "../kselftest.h"
Predated patch already available, see
https://lore.kernel.org/linux-kselftest/20250115105211.390370-1-ps.report@gmx.net/
> diff --git a/tools/testing/selftests/pidfd/pidfd_setns_test.c b/tools/testing/selftests/pidfd/pidfd_setns_test.c
> index 222f8131283b..a55f6641e0b6 100644
> --- a/tools/testing/selftests/pidfd/pidfd_setns_test.c
> +++ b/tools/testing/selftests/pidfd/pidfd_setns_test.c
> @@ -14,6 +14,7 @@
> #include <sys/prctl.h>
> #include <sys/wait.h>
> #include <unistd.h>
> +#include <sys/ioctl.h>
> #include <sys/socket.h>
> #include <sys/stat.h>
> #include <linux/ioctl.h>
and predated patch available, see
https://lore.kernel.org/linux-kselftest/20250115105211.390370-2-ps.report@gmx.net/
Regards,
Peter
^ permalink raw reply
* [PATCH RFT v14 0/8] fork: Support shadow stacks in clone3()
From: Mark Brown @ 2025-02-06 11:38 UTC (permalink / raw)
To: 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, Valentin Schneider, Christian Brauner, Shuah Khan
Cc: linux-kernel, Catalin Marinas, Will Deacon, jannh, bsegall,
Yury Khrustalev, Wilco Dijkstra, linux-kselftest, linux-api,
Mark Brown, Kees Cook, Kees Cook, Shuah Khan
The kernel has recently added support for shadow stacks, currently
x86 only using their CET feature but both arm64 and RISC-V have
equivalent features (GCS and Zicfiss respectively), I am actively
working on GCS[1]. With shadow stacks the hardware maintains an
additional stack containing only the return addresses for branch
instructions which is not generally writeable by userspace and ensures
that any returns are to the recorded addresses. This provides some
protection against ROP attacks and making it easier to collect call
stacks. These shadow stacks are allocated in the address space of the
userspace process.
Our API for shadow stacks does not currently offer userspace any
flexiblity for managing the allocation of shadow stacks for newly
created threads, instead the kernel allocates a new shadow stack with
the same size as the normal stack whenever a thread is created with the
feature enabled. The stacks allocated in this way are freed by the
kernel when the thread exits or shadow stacks are disabled for the
thread. This lack of flexibility and control isn't ideal, in the vast
majority of cases the shadow stack will be over allocated and the
implicit allocation and deallocation is not consistent with other
interfaces. As far as I can tell the interface is done in this manner
mainly because the shadow stack patches were in development since before
clone3() was implemented.
Since clone3() is readily extensible let's add support for specifying a
shadow stack when creating a new thread or process, keeping the current
implicit allocation behaviour if one is not specified either with
clone3() or through the use of clone(). The user must provide a shadow
stack pointer, this must point to memory mapped for use as a shadow
stackby map_shadow_stack() with an architecture specified shadow stack
token at the top of the stack.
Please note that the x86 portions of this code are build tested only, I
don't appear to have a system that can run CET available to me.
[1] https://lore.kernel.org/linux-arm-kernel/20241001-arm64-gcs-v13-0-222b78d87eee@kernel.org/T/#mc58f97f27461749ccf400ebabf6f9f937116a86b
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Changes in v14:
- Rebase onto v6.14-rc1.
- Link to v13: https://lore.kernel.org/r/20241203-clone3-shadow-stack-v13-0-93b89a81a5ed@kernel.org
Changes in v13:
- Rebase onto v6.13-rc1.
- Link to v12: https://lore.kernel.org/r/20241031-clone3-shadow-stack-v12-0-7183eb8bee17@kernel.org
Changes in v12:
- Add the regular prctl() to the userspace API document since arm64
support is queued in -next.
- Link to v11: https://lore.kernel.org/r/20241005-clone3-shadow-stack-v11-0-2a6a2bd6d651@kernel.org
Changes in v11:
- Rebase onto arm64 for-next/gcs, which is based on v6.12-rc1, and
integrate arm64 support.
- Rework the interface to specify a shadow stack pointer rather than a
base and size like we do for the regular stack.
- Link to v10: https://lore.kernel.org/r/20240821-clone3-shadow-stack-v10-0-06e8797b9445@kernel.org
Changes in v10:
- Integrate fixes & improvements for the x86 implementation from Rick
Edgecombe.
- Require that the shadow stack be VM_WRITE.
- Require that the shadow stack base and size be sizeof(void *) aligned.
- Clean up trailing newline.
- Link to v9: https://lore.kernel.org/r/20240819-clone3-shadow-stack-v9-0-962d74f99464@kernel.org
Changes in v9:
- Pull token validation earlier and report problems with an error return
to parent rather than signal delivery to the child.
- Verify that the top of the supplied shadow stack is VM_SHADOW_STACK.
- Rework token validation to only do the page mapping once.
- Drop no longer needed support for testing for signals in selftest.
- Fix typo in comments.
- Link to v8: https://lore.kernel.org/r/20240808-clone3-shadow-stack-v8-0-0acf37caf14c@kernel.org
Changes in v8:
- Fix token verification with user specified shadow stack.
- Don't track user managed shadow stacks for child processes.
- Link to v7: https://lore.kernel.org/r/20240731-clone3-shadow-stack-v7-0-a9532eebfb1d@kernel.org
Changes in v7:
- Rebase onto v6.11-rc1.
- Typo fixes.
- Link to v6: https://lore.kernel.org/r/20240623-clone3-shadow-stack-v6-0-9ee7783b1fb9@kernel.org
Changes in v6:
- Rebase onto v6.10-rc3.
- Ensure we don't try to free the parent shadow stack in error paths of
x86 arch code.
- Spelling fixes in userspace API document.
- Additional cleanups and improvements to the clone3() tests to support
the shadow stack tests.
- Link to v5: https://lore.kernel.org/r/20240203-clone3-shadow-stack-v5-0-322c69598e4b@kernel.org
Changes in v5:
- Rebase onto v6.8-rc2.
- Rework ABI to have the user allocate the shadow stack memory with
map_shadow_stack() and a token.
- Force inlining of the x86 shadow stack enablement.
- Move shadow stack enablement out into a shared header for reuse by
other tests.
- Link to v4: https://lore.kernel.org/r/20231128-clone3-shadow-stack-v4-0-8b28ffe4f676@kernel.org
Changes in v4:
- Formatting changes.
- Use a define for minimum shadow stack size and move some basic
validation to fork.c.
- Link to v3: https://lore.kernel.org/r/20231120-clone3-shadow-stack-v3-0-a7b8ed3e2acc@kernel.org
Changes in v3:
- Rebase onto v6.7-rc2.
- Remove stale shadow_stack in internal kargs.
- If a shadow stack is specified unconditionally use it regardless of
CLONE_ parameters.
- Force enable shadow stacks in the selftest.
- Update changelogs for RISC-V feature rename.
- Link to v2: https://lore.kernel.org/r/20231114-clone3-shadow-stack-v2-0-b613f8681155@kernel.org
Changes in v2:
- Rebase onto v6.7-rc1.
- Remove ability to provide preallocated shadow stack, just specify the
desired size.
- Link to v1: https://lore.kernel.org/r/20231023-clone3-shadow-stack-v1-0-d867d0b5d4d0@kernel.org
---
Mark Brown (8):
arm64/gcs: Return a success value from gcs_alloc_thread_stack()
Documentation: userspace-api: Add shadow stack API documentation
selftests: Provide helper header for shadow stack testing
fork: Add shadow stack support to clone3()
selftests/clone3: Remove redundant flushes of output streams
selftests/clone3: Factor more of main loop into test_clone3()
selftests/clone3: Allow tests to flag if -E2BIG is a valid error code
selftests/clone3: Test shadow stack support
Documentation/userspace-api/index.rst | 1 +
Documentation/userspace-api/shadow_stack.rst | 44 +++++
arch/arm64/include/asm/gcs.h | 8 +-
arch/arm64/kernel/process.c | 8 +-
arch/arm64/mm/gcs.c | 62 +++++-
arch/x86/include/asm/shstk.h | 11 +-
arch/x86/kernel/process.c | 2 +-
arch/x86/kernel/shstk.c | 57 +++++-
include/asm-generic/cacheflush.h | 11 ++
include/linux/sched/task.h | 17 ++
include/uapi/linux/sched.h | 10 +-
kernel/fork.c | 96 +++++++--
tools/testing/selftests/clone3/clone3.c | 226 ++++++++++++++++++----
tools/testing/selftests/clone3/clone3_selftests.h | 65 ++++++-
tools/testing/selftests/ksft_shstk.h | 98 ++++++++++
15 files changed, 635 insertions(+), 81 deletions(-)
---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20231019-clone3-shadow-stack-15d40d2bf536
Best regards,
--
Mark Brown <broonie@kernel.org>
^ 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