* Re: [PATCH -next 1/2] mm/memfd: make F_SEAL_FUTURE_WRITE seal more robust
From: Andrew Morton @ 2018-11-25 0:42 UTC (permalink / raw)
To: Joel Fernandes
Cc: Andy Lutomirski, Stephen Rothwell, LKML, Hugh Dickins, Jann Horn,
Khalid Aziz, Linux API, open list:KERNEL SELFTEST FRAMEWORK,
Linux-MM, marcandre.lureau, Matthew Wilcox, Mike Kravetz,
Shuah Khan
In-Reply-To: <20181122230906.GA198127@google.com>
On Thu, 22 Nov 2018 15:09:06 -0800 Joel Fernandes <joel@joelfernandes.org> wrote:
> Android uses ashmem for sharing memory regions. We are looking forward to
> migrating all usecases of ashmem to memfd so that we can possibly remove
> the ashmem driver in the future from staging while also benefiting from
> using memfd and contributing to it. Note staging drivers are also not ABI
> and generally can be removed at anytime.
>
> One of the main usecases Android has is the ability to create a region and
> mmap it as writeable, then add protection against making any "future"
> writes while keeping the existing already mmap'ed writeable-region active.
> This allows us to implement a usecase where receivers of the shared
> memory buffer can get a read-only view, while the sender continues to
> write to the buffer. See CursorWindow documentation in Android for more
> details:
> https://developer.android.com/reference/android/database/CursorWindow
>
> This usecase cannot be implemented with the existing F_SEAL_WRITE seal.
> To support the usecase, this patch adds a new F_SEAL_FUTURE_WRITE seal
> which prevents any future mmap and write syscalls from succeeding while
> keeping the existing mmap active.
>
> A better way to do F_SEAL_FUTURE_WRITE seal was discussed [1] last week
> where we don't need to modify core VFS structures to get the same
> behavior of the seal. This solves several side-effects pointed by Andy.
> self-tests are provided in later patch to verify the expected semantics.
>
> [1] https://lore.kernel.org/lkml/20181111173650.GA256781@google.com/
This changelog doesn't have the nifty test case code which was in
earlier versions?
^ permalink raw reply
* Re: [PATCH -next 1/2] mm/memfd: make F_SEAL_FUTURE_WRITE seal more robust
From: Matthew Wilcox @ 2018-11-25 0:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Joel Fernandes, Andy Lutomirski, Stephen Rothwell, LKML,
Hugh Dickins, Jann Horn, Khalid Aziz, Linux API,
open list:KERNEL SELFTEST FRAMEWORK, Linux-MM, marcandre.lureau,
Mike Kravetz, Shuah Khan
In-Reply-To: <20181124164229.89c670b6e7a3530ef7b0a40c@linux-foundation.org>
On Sat, Nov 24, 2018 at 04:42:29PM -0800, Andrew Morton wrote:
> This changelog doesn't have the nifty test case code which was in
> earlier versions?
Why do we put regression tests in the changelogs anyway? We have
tools/testing/selftests/vm/ already, perhaps they should go there?
^ permalink raw reply
* [PATCH RESEND v3 0/3] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Elvira Khabirova @ 2018-11-25 1:21 UTC (permalink / raw)
To: oleg-H+wXaHxf7aLQT0dZR+AlfA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mingo-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, esyr-H+wXaHxf7aLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, luto-DgEjT+Ai2ygdnm+yROfE0A,
ldv-u2l5PoMzF/Vg9hUCZPvPmw, strace-devel-3+4lAyCyj6AWlMsSdNXQLw
Resent with linux-api@ Cc'ed.
PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in. The request succeeds when the tracee is in a
syscall-enter-stop, syscall-exit-stop or PTRACE_EVENT_SECCOMP stop,
and fails with -EINVAL otherwise.
There are two reasons for a special syscall-related ptrace request.
Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls. Some examples include:
* The notorious int-0x80-from-64-bit-task issue. See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up. But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared. On such architectures as ia64 this results in all syscall
arguments being unavailable.
Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee. For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.
PTRACE_GET_SYSCALL_INFO returns the following structure:
struct ptrace_syscall_info {
__u8 op; /* PTRACE_SYSCALL_INFO_* */
__u8 __pad0[3];
__u32 arch;
union {
struct {
__u64 nr;
__u64 instruction_pointer;
__u64 stack_pointer;
__u64 frame_pointer;
__u64 args[6];
} entry;
struct {
__s64 rval;
__u8 is_error;
__u8 __pad1[7];
} exit;
};
};
The structure was chosen according to [2], except for the following
changes:
* arch is returned unconditionally to aid with tracing system calls such as
execve();
* the type of nr field was changed from int to __u64 because syscall
numbers are, as a practical matter, 64 bits;
* stack_pointer and frame_pointer fields were added along with
instruction_pointer field since they are readily available and can save
the tracer from extra PTRACE_GETREGSET calls;
* a boolean is_error field was added along with rval field, this way
the tracer can more reliably distinguish a return value
from an error value.
This changeset should be applied on top of [3] and [4].
[1] https://lore.kernel.org/lkml/CA+55aFzcSVmdDj9Lh_gdbz1OzHyEm6ZrGPBDAJnywm2LF_eVyg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org/
[2] https://lore.kernel.org/lkml/CAObL_7GM0n80N7J_DFw_eQyfLyzq+sf4y2AvsCCV88Tb3AwEHA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org/
[3] https://lore.kernel.org/lkml/20181119210139.GA8360-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org/
[4] https://lore.kernel.org/lkml/20181120001128.GA11300-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org/
v3: Split into three changes.
Change struct ptrace_syscall_info.
Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
Add proper defines for ptrace_syscall_info.op values.
Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT and move
them to uapi.
Elvira Khabirova (3):
ptrace: pass type of a syscall-stop in ptrace_message
ptrace: add PTRACE_GET_SYSCALL_INFO request
ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO
include/linux/ptrace.h | 1 +
include/linux/sched.h | 1 +
include/linux/tracehook.h | 10 ++++---
include/uapi/linux/ptrace.h | 34 ++++++++++++++++++++++++
kernel/ptrace.c | 53 +++++++++++++++++++++++++++++++++++++
5 files changed, 96 insertions(+), 3 deletions(-)
--
2.19.1
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply
* [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message
From: Elvira Khabirova @ 2018-11-25 1:22 UTC (permalink / raw)
To: oleg-H+wXaHxf7aLQT0dZR+AlfA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mingo-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, esyr-H+wXaHxf7aLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, luto-DgEjT+Ai2ygdnm+yROfE0A,
ldv-u2l5PoMzF/Vg9hUCZPvPmw, strace-devel-3+4lAyCyj6AWlMsSdNXQLw
In-Reply-To: <20181125022150.46258a20@akathisia>
Define two constants, PTRACE_EVENTMSG_SYSCALL_ENTRY and
PTRACE_EVENTMSG_SYSCALL_EXIT, and place them in ptrace_message
for the duration of syscall-stops.
This way ptracers can distinguish syscall-enter-stops
from syscall-exit-stops using PTRACE_GETEVENTMSG request.
Signed-off-by: Elvira Khabirova <lineprinter-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
Signed-off-by: Dmitry V. Levin <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
---
include/linux/tracehook.h | 9 ++++++---
include/uapi/linux/ptrace.h | 10 ++++++++++
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 40b0b4c1bf7b..633a83fe7051 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -57,13 +57,15 @@ struct linux_binprm;
/*
* ptrace report for syscall entry and exit looks identical.
*/
-static inline int ptrace_report_syscall(struct pt_regs *regs)
+static inline int ptrace_report_syscall(struct pt_regs *regs,
+ unsigned long message)
{
int ptrace = current->ptrace;
if (!(ptrace & PT_PTRACED))
return 0;
+ current->ptrace_message = message;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
/*
@@ -76,6 +78,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
current->exit_code = 0;
}
+ current->ptrace_message = 0;
return fatal_signal_pending(current);
}
@@ -101,7 +104,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
static inline __must_check int tracehook_report_syscall_entry(
struct pt_regs *regs)
{
- return ptrace_report_syscall(regs);
+ return ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_ENTRY);
}
/**
@@ -126,7 +129,7 @@ static inline void tracehook_report_syscall_exit(struct pt_regs *regs, int step)
if (step)
user_single_step_report(regs);
else
- ptrace_report_syscall(regs);
+ ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_EXIT);
}
/**
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index d5a1b8a492b9..cb138902d042 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -104,6 +104,16 @@ struct seccomp_metadata {
#define PTRACE_O_MASK (\
0x000000ff | PTRACE_O_EXITKILL | PTRACE_O_SUSPEND_SECCOMP)
+/*
+ * These values are stored in task->ptrace_message by tracehook_report_syscall_*
+ * to describe current syscall-stop.
+ *
+ * Values for these constants are chosen so that they do not appear
+ * in task->ptrace_message by other means.
+ */
+#define PTRACE_EVENTMSG_SYSCALL_ENTRY 0x80000000U
+#define PTRACE_EVENTMSG_SYSCALL_EXIT 0x90000000U
+
#include <asm/ptrace.h>
--
2.19.1
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply related
* [PATCH RESEND v3 2/3] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Elvira Khabirova @ 2018-11-25 1:23 UTC (permalink / raw)
To: oleg-H+wXaHxf7aLQT0dZR+AlfA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mingo-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, esyr-H+wXaHxf7aLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, luto-DgEjT+Ai2ygdnm+yROfE0A,
ldv-u2l5PoMzF/Vg9hUCZPvPmw, strace-devel-3+4lAyCyj6AWlMsSdNXQLw
In-Reply-To: <20181125022150.46258a20@akathisia>
PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in. The request succeeds when the tracee is in a
syscall-enter-stop or syscall-exit-stop, and fails with -EINVAL otherwise.
A subsequent change may extend PTRACE_GET_SYSCALL_INFO for the case
of PTRACE_EVENT_SECCOMP stop.
There are two reasons for a special syscall-related ptrace request.
Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls. Some examples include:
* The notorious int-0x80-from-64-bit-task issue. See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up. But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared. On such architectures as ia64 this results in all syscall
arguments being unavailable.
Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee. For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.
ptrace(2) man page:
long ptrace(enum __ptrace_request request, pid_t pid,
void *addr, void *data);
...
PTRACE_GET_SYSCALL_INFO
Retrieve information about the syscall that caused the stop.
The information is placed into the buffer pointed by "data"
argument, which should be a pointer to a buffer of type
"struct ptrace_syscall_info".
The "addr" argument contains the size of the buffer pointed to
by "data" (i.e., sizeof(struct ptrace_syscall_info)).
The return value contains the number of bytes available
to be written by the kernel.
If the size of data to be written by the kernel exceeds the size
specified by "addr" argument, the output is truncated.
This operation fails with EINVAL if the tracee is not
in a syscall-enter-stop, a syscall-exit-stop, or
a PTRACE_EVENT_SECCOMP stop.
Signed-off-by: Elvira Khabirova <lineprinter-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
Signed-off-by: Dmitry V. Levin <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
---
include/uapi/linux/ptrace.h | 24 ++++++++++++++++++
kernel/ptrace.c | 50 +++++++++++++++++++++++++++++++++++++
2 files changed, 74 insertions(+)
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index cb138902d042..49b0b1b41943 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -73,6 +73,30 @@ struct seccomp_metadata {
__u64 flags; /* Output: filter's flags */
};
+#define PTRACE_GET_SYSCALL_INFO 0x420f
+#define PTRACE_SYSCALL_INFO_ENTRY 0
+#define PTRACE_SYSCALL_INFO_EXIT 1
+
+struct ptrace_syscall_info {
+ __u8 op; /* PTRACE_SYSCALL_INFO_* */
+ __u8 __pad0[3];
+ __u32 arch;
+ union {
+ struct {
+ __u64 nr;
+ __u64 instruction_pointer;
+ __u64 stack_pointer;
+ __u64 frame_pointer;
+ __u64 args[6];
+ } entry;
+ struct {
+ __s64 rval;
+ __u8 is_error;
+ __u8 __pad1[7];
+ } exit;
+ };
+};
+
/* Read signals from a shared (process wide) queue */
#define PTRACE_PEEKSIGINFO_SHARED (1 << 0)
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 80b34dffdfb9..92c47cd5ad84 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -30,6 +30,10 @@
#include <linux/cn_proc.h>
#include <linux/compat.h>
+#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
+#include <asm/syscall.h> /* For syscall_get_* */
+#endif
+
/*
* Access another process' address space via ptrace.
* Source/target buffer must be kernel space,
@@ -890,6 +894,46 @@ static int ptrace_regset(struct task_struct *task, int req, unsigned int type,
EXPORT_SYMBOL_GPL(task_user_regset_view);
#endif
+#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
+static int ptrace_get_syscall(struct task_struct *child,
+ unsigned long user_size, void __user *datavp)
+{
+ struct ptrace_syscall_info info;
+ struct pt_regs *regs = task_pt_regs(child);
+ unsigned long args[ARRAY_SIZE(info.entry.args)];
+ unsigned long actual_size;
+ unsigned long write_size;
+
+ if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) {
+ int i;
+
+ info.op = PTRACE_SYSCALL_INFO_ENTRY;
+ info.arch = syscall_get_arch(child);
+ info.entry.nr = syscall_get_nr(child, regs);
+ info.entry.instruction_pointer = instruction_pointer(regs);
+ info.entry.stack_pointer = user_stack_pointer(regs);
+ info.entry.frame_pointer = frame_pointer(regs);
+ syscall_get_arguments(child, regs, 0, ARRAY_SIZE(args), args);
+ for (i = 0; i < ARRAY_SIZE(args); i++)
+ info.entry.args[i] = args[i];
+ actual_size = offsetofend(struct ptrace_syscall_info, entry);
+ } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
+ info.op = PTRACE_SYSCALL_INFO_EXIT;
+ info.arch = syscall_get_arch(child);
+ info.exit.rval = syscall_get_error(child, regs);
+ info.exit.is_error = !!info.exit.rval;
+ if (!info.exit.is_error)
+ info.exit.rval = syscall_get_return_value(child, regs);
+ actual_size = offsetofend(struct ptrace_syscall_info, exit);
+ } else {
+ return -EINVAL;
+ }
+
+ write_size = min(actual_size, user_size);
+ return copy_to_user(datavp, &info, write_size) ? -EFAULT : actual_size;
+}
+#endif
+
int ptrace_request(struct task_struct *child, long request,
unsigned long addr, unsigned long data)
{
@@ -1105,6 +1149,12 @@ int ptrace_request(struct task_struct *child, long request,
ret = seccomp_get_metadata(child, addr, datavp);
break;
+#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
+ case PTRACE_GET_SYSCALL_INFO:
+ ret = ptrace_get_syscall(child, addr, datavp);
+ break;
+#endif
+
default:
break;
}
--
2.19.1
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply related
* [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO
From: Elvira Khabirova @ 2018-11-25 1:23 UTC (permalink / raw)
To: oleg-H+wXaHxf7aLQT0dZR+AlfA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mingo-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, esyr-H+wXaHxf7aLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, luto-DgEjT+Ai2ygdnm+yROfE0A,
ldv-u2l5PoMzF/Vg9hUCZPvPmw, strace-devel-3+4lAyCyj6AWlMsSdNXQLw
In-Reply-To: <20181125022150.46258a20@akathisia>
Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops.
The information returned is the same as for syscall-enter-stops.
Signed-off-by: Elvira Khabirova <lineprinter-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
Signed-off-by: Dmitry V. Levin <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
---
include/linux/ptrace.h | 1 +
include/linux/sched.h | 1 +
include/linux/tracehook.h | 1 +
kernel/ptrace.c | 7 +++++--
4 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 6c2ffed907f5..a993d0fde865 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -166,6 +166,7 @@ static inline void ptrace_event(int event, unsigned long message)
{
if (unlikely(ptrace_event_enabled(current, event))) {
current->ptrace_message = message;
+ current->ptrace_event = event;
ptrace_notify((event << 8) | SIGTRAP);
} else if (event == PTRACE_EVENT_EXEC) {
/* legacy EXEC report via SIGTRAP */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a51c13c2b1a0..86215fb654d6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -964,6 +964,7 @@ struct task_struct {
/* Ptrace state: */
unsigned long ptrace_message;
+ int ptrace_event;
kernel_siginfo_t *last_siginfo;
struct task_io_accounting ioac;
diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 633a83fe7051..5d2e5aa07a5c 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -66,6 +66,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs,
return 0;
current->ptrace_message = message;
+ current->ptrace_event = 0;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
/*
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 92c47cd5ad84..74a37e74c7f1 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -904,7 +904,9 @@ static int ptrace_get_syscall(struct task_struct *child,
unsigned long actual_size;
unsigned long write_size;
- if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) {
+ if ((child->ptrace_event == 0 &&
+ child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) ||
+ child->ptrace_event == PTRACE_EVENT_SECCOMP) {
int i;
info.op = PTRACE_SYSCALL_INFO_ENTRY;
@@ -917,7 +919,8 @@ static int ptrace_get_syscall(struct task_struct *child,
for (i = 0; i < ARRAY_SIZE(args); i++)
info.entry.args[i] = args[i];
actual_size = offsetofend(struct ptrace_syscall_info, entry);
- } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
+ } else if (child->ptrace_event == 0 &&
+ child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
info.op = PTRACE_SYSCALL_INFO_EXIT;
info.arch = syscall_get_arch(child);
info.exit.rval = syscall_get_error(child, regs);
--
2.19.1
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply related
* Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message
From: Joey Pabalinas @ 2018-11-25 1:54 UTC (permalink / raw)
To: Elvira Khabirova
Cc: oleg, rostedt, mingo, linux-api, esyr, linux-kernel, luto, ldv,
strace-devel, Joey Pabalinas
In-Reply-To: <20181125022227.55e8caf7@akathisia>
[-- Attachment #1: Type: text/plain, Size: 516 bytes --]
On Sun, Nov 25, 2018 at 02:22:27AM +0100, Elvira Khabirova wrote:
> Define two constants, PTRACE_EVENTMSG_SYSCALL_ENTRY and
> PTRACE_EVENTMSG_SYSCALL_EXIT, and place them in ptrace_message
> for the duration of syscall-stops.
> This way ptracers can distinguish syscall-enter-stops
> from syscall-exit-stops using PTRACE_GETEVENTMSG request.
Is there an advantage to using two constants instead of a single
sys_exit bit (set/unset for syscall-exit-stop/syscall-enter-stop)?
--
Cheers,
Joey Pabalinas
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message
From: Dmitry V. Levin @ 2018-11-25 2:10 UTC (permalink / raw)
To: Joey Pabalinas
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, Oleg Nesterov, Steven Rostedt,
Eugene Syromyatnikov, strace-devel-3+4lAyCyj6AWlMsSdNXQLw,
Andy Lutomirski, Ingo Molnar, linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20181125015402.glcaw3kghcu4pr22-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1064 bytes --]
On Sat, Nov 24, 2018 at 03:54:02PM -1000, Joey Pabalinas wrote:
> On Sun, Nov 25, 2018 at 02:22:27AM +0100, Elvira Khabirova wrote:
> > Define two constants, PTRACE_EVENTMSG_SYSCALL_ENTRY and
> > PTRACE_EVENTMSG_SYSCALL_EXIT, and place them in ptrace_message
> > for the duration of syscall-stops.
> > This way ptracers can distinguish syscall-enter-stops
> > from syscall-exit-stops using PTRACE_GETEVENTMSG request.
>
> Is there an advantage to using two constants instead of a single
> sys_exit bit (set/unset for syscall-exit-stop/syscall-enter-stop)?
Given that without this patch the value returned by PTRACE_GETEVENTMSG
during syscall stop is undefined, we need two different ptrace_message
values that cannot be set by other ptrace events to enable reliable
identification of syscall-enter-stop and syscall-exit-stop in userspace:
if we make PTRACE_GETEVENTMSG return 0 or any other value routinely set by
other ptrace events, it would be hard for userspace to find out whether
the kernel implements new semantics or not.
--
ldv
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 137 bytes --]
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply
* Re: [RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Dmitry V. Levin @ 2018-11-25 4:10 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Eugene Syromiatnikov, Kees Cook, Jann Horn, Linux API,
Oleg Nesterov, Steven Rostedt, LKML, Ingo Molnar,
strace-devel-3+4lAyCyj6AWlMsSdNXQLw
In-Reply-To: <20181123040139.GB2572-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 3266 bytes --]
On Fri, Nov 23, 2018 at 07:01:39AM +0300, Dmitry V. Levin wrote:
> On Thu, Nov 22, 2018 at 04:19:10PM -0800, Andy Lutomirski wrote:
> > On Thu, Nov 22, 2018 at 11:15 AM Dmitry V. Levin wrote:
> > > On Thu, Nov 22, 2018 at 06:55:29AM -0800, Andy Lutomirski wrote:
> > > > On Wed, Nov 21, 2018 at 3:56 PM Dmitry V. Levin wrote:
> > > > > On Wed, Nov 21, 2018 at 02:56:57PM -0800, Andy Lutomirski wrote:
> > > > > > Please cc linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org for future versions.
> > > > > >
> > > > > > On Wed, Nov 21, 2018 at 7:58 AM Elvira Khabirova wrote:
> > > > > > >
> > > > > > > struct ptrace_syscall_info {
> > > > > > > __u8 op; /* 0 for entry, 1 for exit */
> > > > > >
> > > > > > Can you add proper defines, like:
> > > > > >
> > > > > > #define PTRACE_SYSCALL_ENTRY 0
> > > > > > #define PTRACE_SYSCALL_EXIT 1
> > > > > > #define PTRACE_SYSCALL_SECCOMP 2
> > > > > >
> > > > > > and make seccomp work from the start? I'd rather we don't merge an
> > > > > > implementation that doesn't work for seccomp and then have to rework
> > > > > > it later.
> > > > >
> > > > > What's the difference between PTRACE_EVENT_SECCOMP and syscall-entry-stop
> > > > > with regards to PTRACE_GET_SYSCALL_INFO request? At least they have the
> > > > > same entry_info to return.
> > > >
> > > > I'm not sure there's any material difference.
> > >
> > > In that case we don't really need PTRACE_SYSCALL_SECCOMP: op field
> > > describes the structure inside the union to use, not the ptrace stop.
> >
> > Unless we think the structures might diverge in the future.
>
> If these structures ever diverge, then a seccomp structure will be added
> to the union, and a portable userspace code will likely look this way:
>
> #include <linux/ptrace.h>
> ...
> struct ptrace_syscall_info info;
> long rc = ptrace(PTRACE_GET_SYSCALL_INFO, pid, (void *) sizeof(info), &info);
> ...
> switch (info.op) {
> case PTRACE_SYSCALL_INFO_ENTRY:
> /* handle info.entry */
> case PTRACE_SYSCALL_INFO_EXIT:
> /* handle info.exit */
> #ifdef PTRACE_SYSCALL_INFO_SECCOMP
> case PTRACE_SYSCALL_INFO_SECCOMP:
> /* handle info.seccomp */
> #endif
> default:
> /* handle unknown info.op */
> }
>
> In other words, it would be better if PTRACE_SYSCALL_INFO_* selector
> constants were introduced along with corresponding structures in the
> union.
However, the approach I suggested doesn't provide forward compatibility:
if userspace is compiled with kernel headers that don't define
PTRACE_SYSCALL_INFO_SECCOMP, it will break when the kernel
starts to use PTRACE_SYSCALL_INFO_SECCOMP instead of
PTRACE_SYSCALL_INFO_ENTRY for PTRACE_EVENT_SECCOMP support
in PTRACE_GET_SYSCALL_INFO.
The solution is to introduce PTRACE_SYSCALL_INFO_SECCOMP and struct
ptrace_syscall_info.seccomp along with PTRACE_EVENT_SECCOMP support
in PTRACE_GET_SYSCALL_INFO. The initial revision of the seccomp
structure could be made the same as the entry structure, or it can
diverge from the beginning, e.g., by adding ret_data field containing
SECCOMP_RET_DATA return value stored in ptrace_message, this would save
ptracers an extra PTRACE_GETEVENTMSG call currently required to obtain it.
--
ldv
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 137 bytes --]
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply
* Re: Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message
From: Joey Pabalinas @ 2018-11-25 6:17 UTC (permalink / raw)
To: Dmitry V. Levin
Cc: Joey Pabalinas, linux-api-u79uwXL29TY76Z2rM5mHXA, Oleg Nesterov,
Steven Rostedt, Eugene Syromyatnikov,
strace-devel-3+4lAyCyj6AWlMsSdNXQLw, Andy Lutomirski, Ingo Molnar,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20181125021059.GA1190-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 644 bytes --]
On Sun, Nov 25, 2018 at 05:10:59AM +0300, Dmitry V. Levin wrote:
> Given that without this patch the value returned by PTRACE_GETEVENTMSG
> during syscall stop is undefined, we need two different ptrace_message
> values that cannot be set by other ptrace events to enable reliable
> identification of syscall-enter-stop and syscall-exit-stop in userspace:
> if we make PTRACE_GETEVENTMSG return 0 or any other value routinely set by
> other ptrace events, it would be hard for userspace to find out whether
> the kernel implements new semantics or not.
Ah, I see, I agree that definitely makes sense.
--
Cheers,
Joey Pabalinas
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 137 bytes --]
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply
* Re: [PATCH] Add /proc/pid_generation
From: Pavel Machek @ 2018-11-25 22:55 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Daniel Colascione, linux-kernel, Linux API, Tim Murray,
Primiano Tucci, Joel Fernandes, Jonathan Corbet, Andrew Morton,
Mike Rapoport, Roman Gushchin, Vlastimil Babka,
Dennis Zhou (Facebook), Prashant Dhamdhere, Eric W. Biederman,
rostedt, tglx, mingo, linux, pasha.tatashin, jpoimboe,
ard.biesheuvel, Michal Hocko, David Howells <dhowells>
In-Reply-To: <20181122020633.GN3065@bombadil.infradead.org>
[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]
On Wed 2018-11-21 18:06:33, Matthew Wilcox wrote:
> On Wed, Nov 21, 2018 at 12:38:20PM -0800, Daniel Colascione wrote:
> > On Wed, Nov 21, 2018 at 12:31 PM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > On Wed, Nov 21, 2018 at 12:14:44PM -0800, Daniel Colascione wrote:
> > > > This change adds a per-pid-namespace 64-bit generation number,
> > > > incremented on PID rollover, and exposes it via a new proc file
> > > > /proc/pid_generation. By examining this file before and after /proc
> > > > enumeration, user code can detect the potential reuse of a PID and
> > > > restart the task enumeration process, repeating until it gets a
> > > > coherent snapshot.
> > > >
> > > > PID rollover ought to be rare, so in practice, scan repetitions will
> > > > be rare.
> > >
> > > Then why does it need to be 64-bit?
> >
> > [Resending because of accidental HTML. I really need to switch to a
> > better email client.]
> >
> > Because 64 bits is enough for anyone. :-) A u64 is big enough that
> > we'll never observe an overflow on a running system, and PID
> > namespaces are rare enough that we won't miss the four extra bytes we
> > use by upgrading from a u32. And after reading about some security
> > problems caused by too-clever handling of 32-bit rollover, I'd rather
> > the code be obviously correct than save a trivial amount of space.
>
> I don't think you understand how big 4 billion is. If it happens once a
> second, it will take 136 years for a 2^32 count to roll over. How often
> does a PID roll over happen?
Well, the cost of 64-bit vs. 32-bit is really small here... I'd go
with 64bits. If you have 1000 CPUs, rollovers may be faster..
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH] Add /proc/pid_generation
From: Pavel Machek @ 2018-11-25 23:00 UTC (permalink / raw)
To: David Laight
Cc: 'Kevin Easton', Daniel Colascione,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
timmurray@google.com, primiano@google.com, joelaf@google.com,
Jonathan Corbet, Andrew Morton, Mike Rapoport, Roman Gushchin,
Vlastimil Babka, Dennis Zhou (Facebook), Prashant Dhamdhere,
Eric W. Biederman, Steven Rostedt (VMware), Thomas Gleixner,
Ingo Molnar
In-Reply-To: <f00df51af0a4491fa7d86f9dc0c07bf6@AcuMS.aculab.com>
[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]
On Fri 2018-11-23 11:14:17, David Laight wrote:
> From: Kevin Easton
> > Sent: 22 November 2018 11:20
> >
> > On Wed, Nov 21, 2018 at 12:14:44PM -0800, Daniel Colascione wrote:
> > > This change adds a per-pid-namespace 64-bit generation number,
> > > incremented on PID rollover, and exposes it via a new proc file
> > > /proc/pid_generation. By examining this file before and after /proc
> > > enumeration, user code can detect the potential reuse of a PID and
> > > restart the task enumeration process, repeating until it gets a
> > > coherent snapshot.
> >
> > I see downthread this patch has been withdrawn, but nonetheless I'm
> > still curious - does this actually solve the problem?
> >
> > It seems to me that a PID could be reused within a scan even if the
> > generation number remains the same at the beginning and end of a scan:
>
> Why not allocate a 48bit generation number to each 16bit pid?
> Then you have a 64bit 'extended-pid' that can be assumed to never be reused.
> Provided enough interfaces are enhanced to support 'extended-pid' values
> you'll never get reused values.
For the record, I really like this proposal.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v4 1/5] glibc: Perform rseq(2) registration at nptl init and thread creation
From: Florian Weimer @ 2018-11-26 8:28 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Rich Felker, carlos, Joseph Myers, Szabolcs Nagy, libc-alpha,
Thomas Gleixner, Ben Maurer, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Will Deacon, Dave Watson, Paul Turner, linux-kernel,
linux-api
In-Reply-To: <1758017676.12041.1543007347347.JavaMail.zimbra@efficios.com>
* Mathieu Desnoyers:
> Using a "weak" symbol in early adopter libraries is important, so they
> can be loaded together into the same process without causing loader
> errors due to many definitions of the same strong symbol.
This is not how ELF dynamic linking works. If the symbol name is the
same, one definition interposes the others.
You need to ensure that the symbol has the same size everywhere, though.
There are some tricky interactions with symbol versions, too. (The
interposing libraries must not use symbol versioning.)
Thanks,
Florian
^ permalink raw reply
* Re: [RFC PATCH v4 1/5] glibc: Perform rseq(2) registration at nptl init and thread creation
From: Szabolcs Nagy @ 2018-11-26 11:56 UTC (permalink / raw)
To: Mathieu Desnoyers, Rich Felker
Cc: nd, Florian Weimer, carlos, Joseph Myers, libc-alpha,
Thomas Gleixner, Ben Maurer, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Will Deacon, Dave Watson, Paul Turner, linux-kernel,
linux-api
In-Reply-To: <1758017676.12041.1543007347347.JavaMail.zimbra@efficios.com>
On 23/11/18 21:09, Mathieu Desnoyers wrote:
> ----- On Nov 23, 2018, at 1:35 PM, Rich Felker dalias@libc.org wrote:
>
>> On Fri, Nov 23, 2018 at 12:52:21PM -0500, Mathieu Desnoyers wrote:
>>> ----- On Nov 23, 2018, at 12:30 PM, Rich Felker dalias@libc.org wrote:
>>>
>>>> On Fri, Nov 23, 2018 at 12:05:20PM -0500, Mathieu Desnoyers wrote:
>>>>> ----- On Nov 23, 2018, at 9:28 AM, Rich Felker dalias@libc.org wrote:
>>>>> [...]
>>>>>>
>>>>>> Absolutely. As long as it's in libc, implicit destruction will happen.
>>>>>> Actually I think the glibc code shound unconditionally unregister the
>>>>>> rseq address at exit (after blocking signals, so no application code
>>>>>> can run) in case a third-party rseq library was linked and failed to
>>>>>> do so before thread exit (e.g. due to mismatched ref counts) rather
>>>>>> than respecting the reference count, since it knows it's the last
>>>>>> user. This would make potentially-buggy code safer.
>>>>>
>>>>> OK, let me go ahead with a few ideas/questions along that path.
>>>> ^^^^^^^^^^^^^^^
>>>>>
>>>>> Let's say our stated goal is to let the "exit" system call from the
>>>>> glibc thread exit path perform rseq unregistration (without explicit
>>>>> unregistration beforehand). Let's look at what we need.
>>>>
>>>> This is not "along that path". The above-quoted text is not about
>>>> assuming it's safe to make SYS_exit without unregistering the rseq
>>>> object, but rather about glibc being able to perform the
>>>> rseq-unregister syscall without caring about reference counts, since
>>>> it knows no other code that might depend on rseq can run after it.
>>>
>>> When saying "along that path", what I mean is: if we go in that direction,
>>> then we should look into going all the way there, and rely on thread
>>> exit to implicitly unregister the TLS area.
>>>
>>> Do you see any reason for doing an explicit unregistration at thread
>>> exit rather than simply rely on the exit system call ?
>>
>> Whether this is needed is an implementation detail of glibc that
>> should be permitted to vary between versions. Unless glibc wants to
>> promise that it would become a public guarantee, it's not part of the
>> discussion around the API/ABI. Only part of the discussion around
>> implementation internals of the glibc rseq stuff.
>>
>> Of course I may be biased thinking application code should not assume
>> this since it's not true on musl -- for detached threads, the thread
>> frees its own stack before exiting (and thus has to unregister
>> set_tid_address and set_robustlist before exiting).
>
> OK, so on glibc, the implementation could rely on exit side-effect to
> implicitly unregister rseq. On musl, based on the scenario you describe,
> the library should unregister rseq explicitly before stack reclaim.
>
> Am I understanding the situation correctly ?
i think the point is that you don't need to know these
details in order to come up with a design that allows
both implementations. (then the libc can change later)
so
- is there a need for public unregister api (does the
user do it or the rseq library implicitly unregisters)?
- is there a need for ref counting (or the rseq lib
unconditionally unregisters at the end of a thread,
the libc can certainly do this)?
>>> OK, AFAIU so you argue for leaving the __rseq_abi symbol "weak". Just making
>>> sure I correctly understand your position.
>>
>> I don't think it matters, and I don't think making it weak is
>> meaningful or useful (weak in a shared library is largely meaningless)
>> but maybe I'm missing something here.
>
> Using a "weak" symbol in early adopter libraries is important, so they
> can be loaded together into the same process without causing loader
> errors due to many definitions of the same strong symbol.
>
> Using "weak" in a C library is something I'm not sure is a characteristic
> we want or need, because I doubt we would ever want to load two libc at the
> same time in a given process.
>
> The only reason I see for using "weak" for the __rseq_abi symbol in the
> libc is if we want to allow early adopter applications to define
> __rseq_abi as a strong symbol, which would make some sense.
weak really does not matter in dynamic linking
(unless you set the LD_DYNAMIC_WEAK env var for
backward compat with very old glibc, or if it's
an undefined weak reference)
>> Just blocking at start/exit won't solve the problem because
>> global-dynamic TLS in glibc involves dynamic allocation, which is hard
>> to make AS-safe and of course can fail, leaving no way to make forward
>> progress.
>
> How hard would it be to create a async-signal-safe memory pool, which would
> be always accessed with signals blocked, so we could fix those corner-cases
> for good ?
that is hard.
in musl tls access is as-safe, but it uses a different
approach: it does all allocations at thread creation or
dlopen time.
glibc has further issues because it supports dlclose
with module unloading and then dynamic tls related
internal structures are hard to free (it is valid to
implement dlclose as a noop, which is what musl does.
tls access needs to synchronize with dlopen and dlclose
when accessing internal structures, but you need a
lock-free mechanism if the access has to be as-safe,
and dlclose is harder to do that way than dlopen)
^ permalink raw reply
* Re: [PATCH v3 3/4] libbpf: add bpf_prog_test_run_xattr
From: Lorenz Bauer @ 2018-11-26 12:45 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, linux-api, Y Song
In-Reply-To: <20181124222012.lcc6hn6vr6xbw7zr@ast-mbp.dhcp.thefacebook.com>
On Sat, 24 Nov 2018 at 22:20, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Nov 23, 2018 at 11:25:11PM +0100, Daniel Borkmann wrote:
> > On 11/22/2018 03:09 PM, Lorenz Bauer wrote:
> > > Add a new function, which encourages safe usage of the test interface.
> > > bpf_prog_test_run continues to work as before, but should be considered
> > > unsafe.
> > >
> > > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> >
> > Set looks good to me, thanks! Three small things below:
> >
> > > ---
> > > tools/lib/bpf/bpf.c | 27 +++++++++++++++++++++++++++
> > > tools/lib/bpf/bpf.h | 13 +++++++++++++
> > > 2 files changed, 40 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > > index 961e1b9fc592..f8518bef6886 100644
> > > --- a/tools/lib/bpf/bpf.c
> > > +++ b/tools/lib/bpf/bpf.c
> > > @@ -424,6 +424,33 @@ int bpf_prog_test_run(int prog_fd, int repeat, void *data, __u32 size,
> > > return ret;
> > > }
> > >
> > > +int bpf_prog_test_run_xattr(const struct bpf_prog_test_run_attr *test_attr,
> > > + __u32 *size_out, __u32 *retval, __u32 *duration)
> > > +{
> > > + union bpf_attr attr;
> > > + int ret;
> > > +
> > > + if (!test_attr->data_out && test_attr->size_out > 0)
> > > + return -EINVAL;
> > > +
> > > + bzero(&attr, sizeof(attr));
> > > + attr.test.prog_fd = test_attr->prog_fd;
> > > + attr.test.data_in = ptr_to_u64(test_attr->data);
> > > + attr.test.data_out = ptr_to_u64(test_attr->data_out);
> > > + attr.test.data_size_in = test_attr->size;
> > > + attr.test.data_size_out = test_attr->size_out;
> > > + attr.test.repeat = test_attr->repeat;
> > > +
> > > + ret = sys_bpf(BPF_PROG_TEST_RUN, &attr, sizeof(attr));
> > > + if (size_out)
> > > + *size_out = attr.test.data_size_out;
> > > + if (retval)
> > > + *retval = attr.test.retval;
> > > + if (duration)
> > > + *duration = attr.test.duration;
> > > + return ret;
> > > +}
> > > +
> > > int bpf_prog_get_next_id(__u32 start_id, __u32 *next_id)
> > > {
> > > union bpf_attr attr;
> > > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > > index 26a51538213c..570f19f77f42 100644
> > > --- a/tools/lib/bpf/bpf.h
> > > +++ b/tools/lib/bpf/bpf.h
> > > @@ -110,6 +110,19 @@ LIBBPF_API int bpf_prog_attach(int prog_fd, int attachable_fd,
> > > LIBBPF_API int bpf_prog_detach(int attachable_fd, enum bpf_attach_type type);
> > > LIBBPF_API int bpf_prog_detach2(int prog_fd, int attachable_fd,
> > > enum bpf_attach_type type);
> > > +
> > > +struct bpf_prog_test_run_attr {
> > > + int prog_fd;
> > > + int repeat;
> > > + const void *data;
> > > + __u32 size;
> > > + void *data_out; /* optional */
> > > + __u32 size_out;
> >
> > Small nit: could we name these data_{in,out} and data_size_{in,out} as
> > well, so it's analog to the ones from the bpf_attr?
> >
> > > +};
> > > +
> > > +LIBBPF_API int bpf_prog_test_run_xattr(const struct bpf_prog_test_run_attr *test_attr,
> > > + __u32 *size_out, __u32 *retval,
> > > + __u32 *duration);
>
> can we keep return values in struct bpf_prog_test_run_attr ?
> I think the interface will be easier to use and faster. Less pointers
> to pass around.
That's what I had initially, but that makes re-using test_attr really
awkward. Either
you need to reset data_out_size before every call because it is used
to return the
buffer size, or we need another member in test_attr specifically for this. Then
naming becomes really awkward (data_size_out_out anyone?). It also means
we can't take a const struct attr, which is contrary to the other
xattr functions which
already exist.
I think actually inspecting the required size of the output buffer
will be a rare
occurrence, so making the user jump through the hoop of a pointer doesn't seem
too onerous.
>
> > > LIBBPF_API int bpf_prog_test_run(int prog_fd, int repeat, void *data,
> > > __u32 size, void *data_out, __u32 *size_out,
> > > __u32 *retval, __u32 *duration);
> >
> > Could we add a comment into the header here stating that we discourage
> > bpf_prog_test_run()'s use?
> >
> > It would probably also make sense since we go that route that we would
> > convert the 10 bpf_prog_test_run() instances under test_progs.c at the
> > same time so that people extending or looking at BPF kselftests don't
> > copy discouraged bpf_prog_test_run() api as examples from this point
> > onwards anymore.
>
> I would keep bpf_prog_test_run() and test_progs.c as-is.
> I think the prog_test_run in the current form is actually fine.
> I don't find it 'unsafe'.
> When it's called on a given bpf prog the user knows what prog
> suppose to do. If prog is increasing the packet size the test writer
> knows that and should size the output buffer accordingly.
I guess this is a matter of perspective. I prefer an interface that
gives me back
an error message, rather than corrupt my stack / heap, when the assumptions
change. It's also nicer to use from "managed" languages like Go where users
aren't as accustomed to issues like these.
Do you object to me adding the disclaimer to the header as Daniel suggested?
> Also there are plenty of tests where progs don't change the packet size
> and any test with 'repeat > 1' should have the packet size
> return to initial value. Like if the test is doing ipip encap
> at the end of the run the bpf prog should remove that encap.
> Otherwise 'repeat > 1' will produce wrong results.
>
Yup. Another thorny part of the test interface, which I'd like to improve! :)
Don't know how to do it yet though.
--
Lorenz Bauer | Systems Engineer
25 Lavington St., London SE1 0NZ
www.cloudflare.com
^ permalink raw reply
* Re: [PATCH -next 1/2] mm/memfd: make F_SEAL_FUTURE_WRITE seal more robust
From: Joel Fernandes @ 2018-11-26 13:35 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Andrew Morton, Andy Lutomirski, Stephen Rothwell, LKML,
Hugh Dickins, Jann Horn, Khalid Aziz, Linux API,
open list:KERNEL SELFTEST FRAMEWORK, Linux-MM, marcandre.lureau,
Mike Kravetz, Shuah Khan
In-Reply-To: <20181125004736.GA3065@bombadil.infradead.org>
On Sat, Nov 24, 2018 at 04:47:36PM -0800, Matthew Wilcox wrote:
> On Sat, Nov 24, 2018 at 04:42:29PM -0800, Andrew Morton wrote:
> > This changelog doesn't have the nifty test case code which was in
> > earlier versions?
>
> Why do we put regression tests in the changelogs anyway? We have
> tools/testing/selftests/vm/ already, perhaps they should go there?
The reason is I didn't add it was that test case went out of date and the
updated version of the test case went into the selftests in patch 2/2. I
thought that would suffice which covers all the cases. That's why I dropped
it. Would that be Ok?
The changelog of the previous series had it because the selftest was added
only later.
Let me know, thanks,
- Joel
^ permalink raw reply
* Re: [PATCH v3 3/4] libbpf: add bpf_prog_test_run_xattr
From: Daniel Borkmann @ 2018-11-26 13:39 UTC (permalink / raw)
To: Lorenz Bauer, Alexei Starovoitov
Cc: Alexei Starovoitov, netdev, linux-api, Y Song
In-Reply-To: <CACAyw98oSS6qbeNCaqukM81V6vWXdrU2HCp2-b++7E=Xnx7e=w@mail.gmail.com>
On 11/26/2018 01:45 PM, Lorenz Bauer wrote:
> On Sat, 24 Nov 2018 at 22:20, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>> On Fri, Nov 23, 2018 at 11:25:11PM +0100, Daniel Borkmann wrote:
>>> On 11/22/2018 03:09 PM, Lorenz Bauer wrote:
[...]
>>>> LIBBPF_API int bpf_prog_test_run(int prog_fd, int repeat, void *data,
>>>> __u32 size, void *data_out, __u32 *size_out,
>>>> __u32 *retval, __u32 *duration);
>>>
>>> Could we add a comment into the header here stating that we discourage
>>> bpf_prog_test_run()'s use?
>>>
>>> It would probably also make sense since we go that route that we would
>>> convert the 10 bpf_prog_test_run() instances under test_progs.c at the
>>> same time so that people extending or looking at BPF kselftests don't
>>> copy discouraged bpf_prog_test_run() api as examples from this point
>>> onwards anymore.
>>
>> I would keep bpf_prog_test_run() and test_progs.c as-is.
>> I think the prog_test_run in the current form is actually fine.
>> I don't find it 'unsafe'.
>> When it's called on a given bpf prog the user knows what prog
>> suppose to do. If prog is increasing the packet size the test writer
>> knows that and should size the output buffer accordingly.
>
> I guess this is a matter of perspective. I prefer an interface that
> gives me back
> an error message, rather than corrupt my stack / heap, when the assumptions
> change. It's also nicer to use from "managed" languages like Go where users
> aren't as accustomed to issues like these.
>
> Do you object to me adding the disclaimer to the header as Daniel suggested?
Agree that prog_test_run() in the current form is actually fine, I was mostly
thinking that it may be non-obvious to users extending the tests or writing
their own test framework and checking how BPF kselftests does it (since it's
kind of a prime example), so they might end up using the wrong API and run
into mentioned issues w/o realizing. At min some comment with more context
would be needed.
>> Also there are plenty of tests where progs don't change the packet size
>> and any test with 'repeat > 1' should have the packet size
>> return to initial value. Like if the test is doing ipip encap
>> at the end of the run the bpf prog should remove that encap.
>> Otherwise 'repeat > 1' will produce wrong results.
>
> Yup. Another thorny part of the test interface, which I'd like to improve! :)
> Don't know how to do it yet though.
Yep, somehow here we would need to restore original input packet layout/data
so that the test program always runs on the user defined one.
^ permalink raw reply
* Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO
From: Oleg Nesterov @ 2018-11-26 14:35 UTC (permalink / raw)
To: Elvira Khabirova
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, esyr-H+wXaHxf7aLQT0dZR+AlfA,
rostedt-nx8X9YLhiw1AfugRpC6u6w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, mingo-H+wXaHxf7aLQT0dZR+AlfA,
luto-DgEjT+Ai2ygdnm+yROfE0A, ldv-u2l5PoMzF/Vg9hUCZPvPmw,
strace-devel-3+4lAyCyj6AWlMsSdNXQLw
In-Reply-To: <20181125022340.5703400f@akathisia>
On 11/25, Elvira Khabirova wrote:
>
> Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops.
> The information returned is the same as for syscall-enter-stops.
Oh, this is not nice ;) there must be a better option, I hope... Plus
Can't ptrace_get_syscall() check
child->exit_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP;
to detect the PTRACE_EVENT_SECCOMP case?
Oleg.
> Signed-off-by: Elvira Khabirova <lineprinter-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Dmitry V. Levin <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
> ---
> include/linux/ptrace.h | 1 +
> include/linux/sched.h | 1 +
> include/linux/tracehook.h | 1 +
> kernel/ptrace.c | 7 +++++--
> 4 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
> index 6c2ffed907f5..a993d0fde865 100644
> --- a/include/linux/ptrace.h
> +++ b/include/linux/ptrace.h
> @@ -166,6 +166,7 @@ static inline void ptrace_event(int event, unsigned long message)
> {
> if (unlikely(ptrace_event_enabled(current, event))) {
> current->ptrace_message = message;
> + current->ptrace_event = event;
> ptrace_notify((event << 8) | SIGTRAP);
> } else if (event == PTRACE_EVENT_EXEC) {
> /* legacy EXEC report via SIGTRAP */
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index a51c13c2b1a0..86215fb654d6 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -964,6 +964,7 @@ struct task_struct {
>
> /* Ptrace state: */
> unsigned long ptrace_message;
> + int ptrace_event;
> kernel_siginfo_t *last_siginfo;
>
> struct task_io_accounting ioac;
> diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
> index 633a83fe7051..5d2e5aa07a5c 100644
> --- a/include/linux/tracehook.h
> +++ b/include/linux/tracehook.h
> @@ -66,6 +66,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs,
> return 0;
>
> current->ptrace_message = message;
> + current->ptrace_event = 0;
> ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
>
> /*
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 92c47cd5ad84..74a37e74c7f1 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -904,7 +904,9 @@ static int ptrace_get_syscall(struct task_struct *child,
> unsigned long actual_size;
> unsigned long write_size;
>
> - if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) {
> + if ((child->ptrace_event == 0 &&
> + child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) ||
> + child->ptrace_event == PTRACE_EVENT_SECCOMP) {
> int i;
>
> info.op = PTRACE_SYSCALL_INFO_ENTRY;
> @@ -917,7 +919,8 @@ static int ptrace_get_syscall(struct task_struct *child,
> for (i = 0; i < ARRAY_SIZE(args); i++)
> info.entry.args[i] = args[i];
> actual_size = offsetofend(struct ptrace_syscall_info, entry);
> - } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
> + } else if (child->ptrace_event == 0 &&
> + child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
> info.op = PTRACE_SYSCALL_INFO_EXIT;
> info.arch = syscall_get_arch(child);
> info.exit.rval = syscall_get_error(child, regs);
> --
> 2.19.1
>
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Sean Christopherson @ 2018-11-26 14:35 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Jethro Beekman, Andy Lutomirski, Dave Hansen, Florian Weimer,
Linux API, Jann Horn, Linus Torvalds, X86 ML, linux-arch, LKML,
Peter Zijlstra, Rich Felker, nhorman@redhat.com,
npmccallum@redhat.com, Ayoun, Serge, shay.katz-zamir@intel.com,
linux-sgx@vger.kernel.org, Andy Shevchenko, Thomas
In-Reply-To: <20181121151734.GA16564@linux.intel.com>
On Wed, Nov 21, 2018 at 05:17:34PM +0200, Jarkko Sakkinen wrote:
> On Wed, Nov 21, 2018 at 05:17:32AM +0000, Jethro Beekman wrote:
> > Jarkko, can you please explain you solution in detail? The CPU receives an
> > exception. This will be handled by the kernel exception handler. What
> > information does the kernel exception handler use to determine whether to
> > deliver the exception as a regular signal to the process, or whether to set
> > the special registers values for userspace and just continue executing the
> > process manually?
>
> Now we throw SIGSEGV when PF_SGX set, right? In my solution that would
> be turned just doing iret to AEP with the extra that three registers get
> exception data (type, reason, addr). No decoding or RIP adjusting
> involved.
>
> That would mean that you would actually have to implement AEP handler
> than just have enclu there.
>
> I've also proposed that perhaps for SGX also #UD should be propagated
> this way because for some instructions you need outside help to emulate
> "non-enclave" environment.
And how would you determine the #UD is related to SGX? Hardware doesn't
provide any indication that a #UD (or any other fault) is related to SGX
or occurred in an enclave. The only fault that is special-cased in a
non-virtualized environment is #PF signaled by the EPCM, which gets the
PF_SGX bit set in the error code.
> That is all I have drafted together so far. I'll try to finish v18 this
> week with other stuff and refine further next week (unless someone gives
> obvious reason why this doesn't work, which might well be because I
> haven't went too deep with my analysis yet because of lack of time).
>
> /Jarkko
^ permalink raw reply
* Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message
From: Oleg Nesterov @ 2018-11-26 14:56 UTC (permalink / raw)
To: Elvira Khabirova
Cc: rostedt, mingo, linux-kernel, ldv, esyr, luto, strace-devel,
linux-api
In-Reply-To: <20181125022227.55e8caf7@akathisia>
On 11/25, Elvira Khabirova wrote:
>
> + * These values are stored in task->ptrace_message by tracehook_report_syscall_*
> + * to describe current syscall-stop.
> + *
> + * Values for these constants are chosen so that they do not appear
> + * in task->ptrace_message by other means.
> + */
> +#define PTRACE_EVENTMSG_SYSCALL_ENTRY 0x80000000U
> +#define PTRACE_EVENTMSG_SYSCALL_EXIT 0x90000000U
Stupid question, why not
#define PTRACE_EVENT_SYSCALL_ENTRY 8
#define PTRACE_EVENT_SYSCALL_EXIT 9
right after other PTRACE_EVENT_* constants?
The next 3/3 tries to detect PTRACE_EVENT_SECCOMP case anyway, so I am not sure
I understand what "do not appear in task->ptrace_message by other means" actually
means...
Oleg.
^ permalink raw reply
* Re: extending wait4(2) or waitid(2) linux syscall
From: Florian Weimer @ 2018-11-26 15:18 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Dmitry V. Levin, Albert ARIBAUD, H. Peter Anvin, GNU C Library,
Linux API
In-Reply-To: <CAK8P3a38zcMikOiaHXrKbc3URL6-6kETk=1F-Bx7LywPyRh=yg@mail.gmail.com>
* Arnd Bergmann:
> On Thu, Nov 15, 2018 at 7:30 AM Dmitry V. Levin <ldv@altlinux.org> wrote:
>> On Thu, Nov 15, 2018 at 06:39:03AM -0800, Arnd Bergmann wrote:
>> > On Thu, Nov 15, 2018 at 6:05 AM Dmitry V. Levin wrote:
>> > > On Thu, Apr 20, 2017 at 03:20:51PM +0200, Albert ARIBAUD wrote:
>>
>> 1. strace needs a race-free invocation of wait4(2) or waitid(2)
>> with a different signal mask, this cannot be achieved without
>> an extended version of syscall, similar to pselect6(2) extension
>> over select(2) and ppoll(2) extension over poll(2).
>>
>> Signal mask specification in linux requires two parameters:
>> "const sigset_t *sigmask" and "size_t sigsetsize".
>> Creating pwait6(2) as an extension of wait4(2) with two arguments
>> is straightforward.
>> Creating pwaitid(2) as an extension of waitid(2) that already has 5
>> arguments would require an indirection similar to pselect6(2).
>
> Getting back to this point: you could also do the same thing with
> the CLONE_FD approach from Josh Triplett[1] or Casey Dahlin's
> older waitfd() syscall, correct?
A descriptor-based solution would not be useful to glibc because
applications assume that glibc does not (persistently) open any file
descriptors behind t heir back.
Thanks,
Florian
^ permalink raw reply
* Re: [RFC PATCH v4 1/5] glibc: Perform rseq(2) registration at nptl init and thread creation
From: Mathieu Desnoyers @ 2018-11-26 15:51 UTC (permalink / raw)
To: Florian Weimer
Cc: Rich Felker, carlos, Joseph Myers, Szabolcs Nagy, libc-alpha,
Thomas Gleixner, Ben Maurer, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Will Deacon, Dave Watson, Paul Turner, linux-kernel,
linux-api
In-Reply-To: <87bm6cqm31.fsf@oldenburg.str.redhat.com>
----- On Nov 26, 2018, at 3:28 AM, Florian Weimer fweimer@redhat.com wrote:
> * Mathieu Desnoyers:
>
>> Using a "weak" symbol in early adopter libraries is important, so they
>> can be loaded together into the same process without causing loader
>> errors due to many definitions of the same strong symbol.
>
> This is not how ELF dynamic linking works. If the symbol name is the
> same, one definition interposes the others.
>
> You need to ensure that the symbol has the same size everywhere, though.
> There are some tricky interactions with symbol versions, too. (The
> interposing libraries must not use symbol versioning.)
I was under the impression that loading the same strong symbol into an
application multiple times would cause some kind of warning if non-weak. I did
some testing to figure out which case I remembered would cause this.
When compiling with "-fno-common", dynamic and static linking work fine, but
trying to add multiple instances of a given symbol into a single object fails
with:
/tmp/ccSakXZV.o:(.bss+0x0): multiple definition of `a'
/tmp/ccQBJBOo.o:(.bss+0x0): first defined here
Even if the symbol has the same size.
So considering that we don't care about compiling into a single object here,
and only care about static and dynamic linking of libraries, indeed the "weak"
symbol is not useful.
So let's make __rseq_abi and __rseq_refcount strong symbols then ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH v4 1/5] glibc: Perform rseq(2) registration at nptl init and thread creation
From: Florian Weimer @ 2018-11-26 16:03 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Rich Felker, carlos, Joseph Myers, Szabolcs Nagy, libc-alpha,
Thomas Gleixner, Ben Maurer, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Will Deacon, Dave Watson, Paul Turner, linux-kernel,
linux-api
In-Reply-To: <688718071.12798.1543247469553.JavaMail.zimbra@efficios.com>
* Mathieu Desnoyers:
> So let's make __rseq_abi and __rseq_refcount strong symbols then ?
Yes, please. (But I'm still not sure we need the reference counter.)
Thanks,
Florian
^ permalink raw reply
* Re: [RFC PATCH v4 1/5] glibc: Perform rseq(2) registration at nptl init and thread creation
From: Mathieu Desnoyers @ 2018-11-26 16:30 UTC (permalink / raw)
To: Florian Weimer
Cc: Rich Felker, carlos, Joseph Myers, Szabolcs Nagy, libc-alpha,
Thomas Gleixner, Ben Maurer, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Will Deacon, Dave Watson, Paul Turner, linux-kernel,
linux-api
In-Reply-To: <688718071.12798.1543247469553.JavaMail.zimbra@efficios.com>
----- On Nov 26, 2018, at 10:51 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
> ----- On Nov 26, 2018, at 3:28 AM, Florian Weimer fweimer@redhat.com wrote:
>
>> * Mathieu Desnoyers:
>>
>>> Using a "weak" symbol in early adopter libraries is important, so they
>>> can be loaded together into the same process without causing loader
>>> errors due to many definitions of the same strong symbol.
>>
>> This is not how ELF dynamic linking works. If the symbol name is the
>> same, one definition interposes the others.
>>
>> You need to ensure that the symbol has the same size everywhere, though.
>> There are some tricky interactions with symbol versions, too. (The
>> interposing libraries must not use symbol versioning.)
>
> I was under the impression that loading the same strong symbol into an
> application multiple times would cause some kind of warning if non-weak. I did
> some testing to figure out which case I remembered would cause this.
>
> When compiling with "-fno-common", dynamic and static linking work fine, but
> trying to add multiple instances of a given symbol into a single object fails
> with:
>
> /tmp/ccSakXZV.o:(.bss+0x0): multiple definition of `a'
> /tmp/ccQBJBOo.o:(.bss+0x0): first defined here
>
> Even if the symbol has the same size.
>
> So considering that we don't care about compiling into a single object here,
> and only care about static and dynamic linking of libraries, indeed the "weak"
> symbol is not useful.
>
> So let's make __rseq_abi and __rseq_refcount strong symbols then ?
Actually, looking into ld(1) --warn-common, it looks like "weak" would be cleaner
after all, especially for __rseq_abi which we needs to be initialized to a specific
value, which is therefore not a common symbol.
" --warn-common
Warn when a common symbol is combined with another common symbol or with a symbol definition. Unix
linkers allow this somewhat sloppy practice, but linkers on some other operating systems do not.
This option allows you to find potential problems from combining global symbols. Unfortunately,
some C libraries use this practice, so you may get some warnings about symbols in the libraries as
well as in your programs."
Thoughts ?
Thanks,
Mathieu
>
> Thanks,
>
> Mathieu
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH v4 1/5] glibc: Perform rseq(2) registration at nptl init and thread creation
From: Rich Felker @ 2018-11-26 17:07 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Florian Weimer, carlos, Joseph Myers, Szabolcs Nagy, libc-alpha,
Thomas Gleixner, Ben Maurer, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Will Deacon, Dave Watson, Paul Turner, linux-kernel,
linux-api
In-Reply-To: <1284855405.12857.1543249851299.JavaMail.zimbra@efficios.com>
On Mon, Nov 26, 2018 at 11:30:51AM -0500, Mathieu Desnoyers wrote:
> ----- On Nov 26, 2018, at 10:51 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
>
> > ----- On Nov 26, 2018, at 3:28 AM, Florian Weimer fweimer@redhat.com wrote:
> >
> >> * Mathieu Desnoyers:
> >>
> >>> Using a "weak" symbol in early adopter libraries is important, so they
> >>> can be loaded together into the same process without causing loader
> >>> errors due to many definitions of the same strong symbol.
> >>
> >> This is not how ELF dynamic linking works. If the symbol name is the
> >> same, one definition interposes the others.
> >>
> >> You need to ensure that the symbol has the same size everywhere, though.
> >> There are some tricky interactions with symbol versions, too. (The
> >> interposing libraries must not use symbol versioning.)
> >
> > I was under the impression that loading the same strong symbol into an
> > application multiple times would cause some kind of warning if non-weak. I did
> > some testing to figure out which case I remembered would cause this.
> >
> > When compiling with "-fno-common", dynamic and static linking work fine, but
> > trying to add multiple instances of a given symbol into a single object fails
> > with:
> >
> > /tmp/ccSakXZV.o:(.bss+0x0): multiple definition of `a'
> > /tmp/ccQBJBOo.o:(.bss+0x0): first defined here
> >
> > Even if the symbol has the same size.
> >
> > So considering that we don't care about compiling into a single object here,
> > and only care about static and dynamic linking of libraries, indeed the "weak"
> > symbol is not useful.
> >
> > So let's make __rseq_abi and __rseq_refcount strong symbols then ?
>
> Actually, looking into ld(1) --warn-common, it looks like "weak" would be cleaner
> after all, especially for __rseq_abi which we needs to be initialized to a specific
> value, which is therefore not a common symbol.
>
> " --warn-common
> Warn when a common symbol is combined with another common symbol or with a symbol definition. Unix
> linkers allow this somewhat sloppy practice, but linkers on some other operating systems do not.
> This option allows you to find potential problems from combining global symbols. Unfortunately,
> some C libraries use this practice, so you may get some warnings about symbols in the libraries as
> well as in your programs."
>
> Thoughts ?
AFAIK this has nothing to do with dynamic linking.
Rich
^ 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