From: Helge Deller <deller@gmx.de>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: Helge Deller <deller@gmx.de>, qemu-devel@nongnu.org
Subject: Re: [PATCH] Fix signal handler to detect crashes in qemu-linux-user
Date: Sat, 12 Aug 2023 20:12:47 +0200 [thread overview]
Message-ID: <ZNfLn22w8bA5X3YV@p100> (raw)
In-Reply-To: <56908f79-c883-0d83-c990-e7a86714b6da@linaro.org>
* Richard Henderson <richard.henderson@linaro.org>:
> On 8/12/23 09:43, Helge Deller wrote:
> > +/* _init and _fini are provided by the linker */
> > +extern char _init;
> > +extern char _fini;
> > +
> > static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)
> > {
> > CPUArchState *env = thread_cpu->env_ptr;
> > @@ -819,6 +848,13 @@ static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)
> > if (host_sig == SIGSEGV) {
> > bool maperr = true;
> >
> > + /* Did segfault happened in qemu source code? */
> > + if ((pc >= (uintptr_t) &_init && pc < (uintptr_t) &_fini) ||
> > + (TARGET_ABI_BITS == 32 && HOST_LONG_BITS == 64
> > + && !h2g_valid(host_addr))) {
> > + qemu_show_backtrace(info);
> > + }
>
> This is incorrect.
>
> I did mention that you should look at adjust_signal_pc,
Yes, you did.
Maybe I was blind or didn't fully understood yet.
> which has a lot of commentary on the subject.
>
> (0) Place this after the write-protect and restart check
> just below, since that is not an error of any sort.
>
> (1) If helper_retaddr == 1, then this is a fault reading
> for translation and is a guest SIGSEGV.
>
> (2) If helper_retaddr != 0, then this is a fault in some
> cpu_ldst.h operation and is a guest SIGSEGV.
(1) and (2) means host SIGSEGV can only happen if
helper_retaddr == 0.
> (3) If in_code_gen_buffer(host_signal_pc()), then this
> is a fault within generated code and is a guest SIGSEGV.
>
> Otherwise it's a host SIGSEGV.
So, basically like this?
Helge
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 748a98f3e5..9762246eec 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -23,6 +23,7 @@
#include <sys/ucontext.h>
#include <sys/resource.h>
+#include <execinfo.h>
#include "qemu.h"
#include "user-internals.h"
@@ -32,6 +33,7 @@
#include "signal-common.h"
#include "host-signal.h"
#include "user/safe-syscall.h"
+#include "tcg/tcg.h"
static struct target_sigaction sigact_table[TARGET_NSIG];
@@ -781,6 +783,31 @@ static inline void rewind_if_in_safe_syscall(void *puc)
}
}
+static void qemu_show_backtrace(siginfo_t *info)
+{
+ void *array[20];
+ char **strings;
+ int size, i;
+
+ fprintf(stderr, "---------------\n");
+ fprintf(stderr, "QEMU linux-user v" QEMU_VERSION " for target "
+ UNAME_MACHINE "\n");
+ fprintf(stderr, "QEMU internal error: signal=%d, errno=%d, "
+ "code=%d, addr=%p\n",
+ info->si_signo, info->si_errno, info->si_code,
+ info->si_addr);
+ fprintf(stderr, "while running: %s\n", exec_path);
+ size = backtrace(array, ARRAY_SIZE(array));
+ strings = backtrace_symbols(array, size);
+ if (strings) {
+ fprintf(stderr, "QEMU backtrace:\n");
+ for (i = 0; i < size; i++)
+ fprintf(stderr, "%s\n", strings[i]);
+ }
+ free (strings);
+ exit(info->si_code);
+}
+
static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)
{
CPUArchState *env = thread_cpu->env_ptr;
@@ -800,7 +827,7 @@ static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)
*/
if ((host_sig == SIGSEGV || host_sig == SIGBUS) && info->si_code > 0) {
MMUAccessType access_type;
- uintptr_t host_addr;
+ uintptr_t host_addr, host_pc;
abi_ptr guest_addr;
bool is_write;
@@ -812,7 +839,8 @@ static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)
*/
guest_addr = h2g_nocheck(host_addr);
- pc = host_signal_pc(uc);
+ host_pc = host_signal_pc(uc);
+ pc = host_pc;
is_write = host_signal_write(info, uc);
access_type = adjust_signal_pc(&pc, is_write);
@@ -837,6 +865,12 @@ static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)
}
}
+ /* Did segfault happened in qemu source code? */
+ if (helper_retaddr == 0 &&
+ !in_code_gen_buffer((void*)host_pc)) {
+ qemu_show_backtrace(info);
+ }
+
sigprocmask(SIG_SETMASK, sigmask, NULL);
cpu_loop_exit_sigsegv(cpu, guest_addr, access_type, maperr, pc);
} else {
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9353268cc1..da29d97816 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8132,6 +8132,7 @@ static int open_self_maps_1(CPUArchState *cpu_env, int fd, bool smaps)
IntervalTreeNode *s;
int count;
+*(int*)NULL = 1;
for (s = interval_tree_iter_first(map_info, 0, -1); s;
s = interval_tree_iter_next(s, 0, -1)) {
MapInfo *e = container_of(s, MapInfo, itree);
next prev parent reply other threads:[~2023-08-12 18:13 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-12 16:43 [PATCH] Fix signal handler to detect crashes in qemu-linux-user Helge Deller
2023-08-12 17:33 ` Richard Henderson
2023-08-12 18:12 ` Helge Deller [this message]
2023-08-13 2:30 ` Richard Henderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZNfLn22w8bA5X3YV@p100 \
--to=deller@gmx.de \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).