| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Yjuai7yv7QrzgsZS@elver.google.com>
Date: Wed, 23 Mar 2022 23:09:15 +0100
From: Marco Elver <elver@...gle.com>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: RFC: Use of user space handler vs. SIG_DFL on forced signals
On Tue, Mar 22, 2022 at 12:06PM -0500, Eric W. Biederman wrote:
> Marco Elver <elver@...gle.com> writes:
[...]
> > Not quite. We need it to be synchronous, because we need to know the
> > precise instruction and potentially do some other stuff _before_
> > subsequent instructions.
> >
> > A compromise might be to deliver synchronously normally, but when
> > blocked deliver asynchronously. But if the signal was delivered
> > asynchronously, we need to let the signal handler know delivery was
> > asynchronous, so that our tracing logic can recover and give up at
> > that point.
> >
> > To do this indication if it was asynchronous, we probably need to
> > extend siginfo_t once more. Would that be reasonable?
>
> So the idea is to use normal signal delivery but to set a flag
> to indicate that the signal was blocked at the time it was sent?
>
> It should be possible to add another field that takes a non-zero
> value. On older kernels it should always have a value of zero so it
> should be safe.
>
> It might also be possible to simply ignore the signal if it is blocked.
>
> In either case it will probably take a little bit of care to get the
> races out.
So I gave the "synchronous if possible, but if async let handler know"
version a shot (below). As long as we know if the signal being delivered
is asynchronous we're good, and not terminating the process.
Does that look more reasonable?
Thanks,
-- Marco
------ >8 ------
arch/arm/kernel/signal.c | 1 +
arch/arm64/kernel/signal.c | 1 +
arch/arm64/kernel/signal32.c | 1 +
arch/m68k/kernel/signal.c | 1 +
arch/sparc/kernel/signal32.c | 1 +
arch/sparc/kernel/signal_64.c | 1 +
arch/x86/kernel/signal_compat.c | 2 ++
include/linux/compat.h | 1 +
include/linux/sched/signal.h | 2 +-
include/uapi/asm-generic/siginfo.h | 2 ++
kernel/events/core.c | 4 ++--
kernel/signal.c | 16 ++++++++++++++--
12 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index c532a6041066..5ba4e1d0813d 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -708,6 +708,7 @@ static_assert(offsetof(siginfo_t, si_upper) == 0x18);
static_assert(offsetof(siginfo_t, si_pkey) == 0x14);
static_assert(offsetof(siginfo_t, si_perf_data) == 0x10);
static_assert(offsetof(siginfo_t, si_perf_type) == 0x14);
+static_assert(offsetof(siginfo_t, si_perf_async) == 0x18);
static_assert(offsetof(siginfo_t, si_band) == 0x0c);
static_assert(offsetof(siginfo_t, si_fd) == 0x10);
static_assert(offsetof(siginfo_t, si_call_addr) == 0x0c);
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index d8aaf4b6f432..a6a3bf49ed53 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -1010,6 +1010,7 @@ static_assert(offsetof(siginfo_t, si_upper) == 0x28);
static_assert(offsetof(siginfo_t, si_pkey) == 0x20);
static_assert(offsetof(siginfo_t, si_perf_data) == 0x18);
static_assert(offsetof(siginfo_t, si_perf_type) == 0x20);
+static_assert(offsetof(siginfo_t, si_perf_async) == 0x24);
static_assert(offsetof(siginfo_t, si_band) == 0x10);
static_assert(offsetof(siginfo_t, si_fd) == 0x18);
static_assert(offsetof(siginfo_t, si_call_addr) == 0x10);
diff --git a/arch/arm64/kernel/signal32.c b/arch/arm64/kernel/signal32.c
index d984282b979f..0f72cdadd43e 100644
--- a/arch/arm64/kernel/signal32.c
+++ b/arch/arm64/kernel/signal32.c
@@ -487,6 +487,7 @@ static_assert(offsetof(compat_siginfo_t, si_upper) == 0x18);
static_assert(offsetof(compat_siginfo_t, si_pkey) == 0x14);
static_assert(offsetof(compat_siginfo_t, si_perf_data) == 0x10);
static_assert(offsetof(compat_siginfo_t, si_perf_type) == 0x14);
+static_assert(offsetof(compat_siginfo_t, si_perf_async) == 0x18);
static_assert(offsetof(compat_siginfo_t, si_band) == 0x0c);
static_assert(offsetof(compat_siginfo_t, si_fd) == 0x10);
static_assert(offsetof(compat_siginfo_t, si_call_addr) == 0x0c);
diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
index 338817d0cb3f..c6332727f82f 100644
--- a/arch/m68k/kernel/signal.c
+++ b/arch/m68k/kernel/signal.c
@@ -625,6 +625,7 @@ static inline void siginfo_build_tests(void)
/* _sigfault._perf */
BUILD_BUG_ON(offsetof(siginfo_t, si_perf_data) != 0x10);
BUILD_BUG_ON(offsetof(siginfo_t, si_perf_type) != 0x14);
+ BUILD_BUG_ON(offsetof(siginfo_t, si_perf_async) != 0x18);
/* _sigpoll */
BUILD_BUG_ON(offsetof(siginfo_t, si_band) != 0x0c);
diff --git a/arch/sparc/kernel/signal32.c b/arch/sparc/kernel/signal32.c
index 6cc124a3bb98..b127649ac224 100644
--- a/arch/sparc/kernel/signal32.c
+++ b/arch/sparc/kernel/signal32.c
@@ -780,5 +780,6 @@ static_assert(offsetof(compat_siginfo_t, si_upper) == 0x18);
static_assert(offsetof(compat_siginfo_t, si_pkey) == 0x14);
static_assert(offsetof(compat_siginfo_t, si_perf_data) == 0x10);
static_assert(offsetof(compat_siginfo_t, si_perf_type) == 0x14);
+static_assert(offsetof(compat_siginfo_t, si_perf_async) == 0x18);
static_assert(offsetof(compat_siginfo_t, si_band) == 0x0c);
static_assert(offsetof(compat_siginfo_t, si_fd) == 0x10);
diff --git a/arch/sparc/kernel/signal_64.c b/arch/sparc/kernel/signal_64.c
index 2a78d2af1265..dfa42d9347d8 100644
--- a/arch/sparc/kernel/signal_64.c
+++ b/arch/sparc/kernel/signal_64.c
@@ -590,5 +590,6 @@ static_assert(offsetof(siginfo_t, si_upper) == 0x28);
static_assert(offsetof(siginfo_t, si_pkey) == 0x20);
static_assert(offsetof(siginfo_t, si_perf_data) == 0x18);
static_assert(offsetof(siginfo_t, si_perf_type) == 0x20);
+static_assert(offsetof(siginfo_t, si_perf_async) == 0x24);
static_assert(offsetof(siginfo_t, si_band) == 0x10);
static_assert(offsetof(siginfo_t, si_fd) == 0x14);
diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c
index b52407c56000..1f1ccbc476d1 100644
--- a/arch/x86/kernel/signal_compat.c
+++ b/arch/x86/kernel/signal_compat.c
@@ -149,8 +149,10 @@ static inline void signal_compat_build_tests(void)
BUILD_BUG_ON(offsetof(siginfo_t, si_perf_data) != 0x18);
BUILD_BUG_ON(offsetof(siginfo_t, si_perf_type) != 0x20);
+ BUILD_BUG_ON(offsetof(siginfo_t, si_perf_async) != 0x24);
BUILD_BUG_ON(offsetof(compat_siginfo_t, si_perf_data) != 0x10);
BUILD_BUG_ON(offsetof(compat_siginfo_t, si_perf_type) != 0x14);
+ BUILD_BUG_ON(offsetof(compat_siginfo_t, si_perf_async) != 0x18);
CHECK_CSI_OFFSET(_sigpoll);
CHECK_CSI_SIZE (_sigpoll, 2*sizeof(int));
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 1c758b0e0359..36684b97075d 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -235,6 +235,7 @@ typedef struct compat_siginfo {
struct {
compat_ulong_t _data;
u32 _type;
+ u32 _async;
} _perf;
};
} _sigfault;
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index b6ecb9fc4cd2..9924fe7559a0 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -320,7 +320,7 @@ int send_sig_mceerr(int code, void __user *, short, struct task_struct *);
int force_sig_bnderr(void __user *addr, void __user *lower, void __user *upper);
int force_sig_pkuerr(void __user *addr, u32 pkey);
-int force_sig_perf(void __user *addr, u32 type, u64 sig_data);
+int send_sig_perf(void __user *addr, u32 type, u64 sig_data);
int force_sig_ptrace_errno_trap(int errno, void __user *addr);
int force_sig_fault_trapno(int sig, int code, void __user *addr, int trapno);
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index 3ba180f550d7..835eea023630 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -99,6 +99,7 @@ union __sifields {
struct {
unsigned long _data;
__u32 _type;
+ __u32 _async;
} _perf;
};
} _sigfault;
@@ -164,6 +165,7 @@ typedef struct siginfo {
#define si_pkey _sifields._sigfault._addr_pkey._pkey
#define si_perf_data _sifields._sigfault._perf._data
#define si_perf_type _sifields._sigfault._perf._type
+#define si_perf_async _sifields._sigfault._perf._async
#define si_band _sifields._sigpoll._band
#define si_fd _sifields._sigpoll._fd
#define si_call_addr _sifields._sigsys._call_addr
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 57c7197838db..8f2f251465e9 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6533,8 +6533,8 @@ static void perf_sigtrap(struct perf_event *event)
if (current->flags & PF_EXITING)
return;
- force_sig_perf((void __user *)event->pending_addr,
- event->attr.type, event->attr.sig_data);
+ send_sig_perf((void __user *)event->pending_addr,
+ event->attr.type, event->attr.sig_data);
}
static void perf_pending_event_disable(struct perf_event *event)
diff --git a/kernel/signal.c b/kernel/signal.c
index 38602738866e..9f8d1ed88dcb 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1804,7 +1804,7 @@ int force_sig_pkuerr(void __user *addr, u32 pkey)
}
#endif
-int force_sig_perf(void __user *addr, u32 type, u64 sig_data)
+int send_sig_perf(void __user *addr, u32 type, u64 sig_data)
{
struct kernel_siginfo info;
@@ -1816,7 +1816,16 @@ int force_sig_perf(void __user *addr, u32 type, u64 sig_data)
info.si_perf_data = sig_data;
info.si_perf_type = type;
- return force_sig_info(&info);
+ /*
+ * Signals generated by perf events should not terminate the whole
+ * process if SIGTRAP is blocked, however, delivering the signal
+ * asynchronously is better than not delivering at all. But tell user
+ * space if the signal was asynchronous, so it can clearly be
+ * distinguished from normal synchronous ones.
+ */
+ info.si_perf_async = sigismember(¤t->blocked, info.si_signo);
+
+ return send_sig_info(info.si_signo, &info, current);
}
/**
@@ -3429,6 +3438,7 @@ void copy_siginfo_to_external32(struct compat_siginfo *to,
to->si_addr = ptr_to_compat(from->si_addr);
to->si_perf_data = from->si_perf_data;
to->si_perf_type = from->si_perf_type;
+ to->si_perf_async = from->si_perf_async;
break;
case SIL_CHLD:
to->si_pid = from->si_pid;
@@ -3506,6 +3516,7 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to,
to->si_addr = compat_ptr(from->si_addr);
to->si_perf_data = from->si_perf_data;
to->si_perf_type = from->si_perf_type;
+ to->si_perf_async = from->si_perf_async;
break;
case SIL_CHLD:
to->si_pid = from->si_pid;
@@ -4719,6 +4730,7 @@ static inline void siginfo_buildtime_checks(void)
CHECK_OFFSET(si_pkey);
CHECK_OFFSET(si_perf_data);
CHECK_OFFSET(si_perf_type);
+ CHECK_OFFSET(si_perf_async);
/* sigpoll */
CHECK_OFFSET(si_band);
--
2.35.1.894.gb6a874cedc-goog
Powered by blists - more mailing lists