[Qemu-devel] [PATCH] vl.c: Implement SIGILL signal handler for triggering SIGSEGV

[Qemu-devel] [PATCH] vl.c: Implement SIGILL signal handler for triggering SIGSEGV

[Michal Novotny]

This is the patch to introduce SIGILL handler to be able to trigger
SIGSEGV signal in qemu. This has been written to help debugging
state when qemu crashes by SIGSEGV as a simple reproducer to
emulate such situation in case of need.

Signed-off-by: Michal Novotny <minovotn@redhat.com>
---
 vl.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/vl.c b/vl.c
index 7e04641..3966271 100644
--- a/vl.c
+++ b/vl.c
@@ -2897,6 +2897,26 @@ static int object_create(QemuOpts *opts, void *opaque)
     return 0;
 }
 
+#ifdef CONFIG_POSIX
+static void signal_handler(int signal)
+{
+    int *p = NULL;
+
+    *p = 0xDEADBEEF;
+}
+
+static void setup_signal_handlers(void)
+{
+    struct sigaction action;
+
+    memset(&action, 0, sizeof(action));
+    sigfillset(&action.sa_mask);
+    action.sa_handler = signal_handler;
+    action.sa_flags = 0;
+    sigaction(SIGILL, &action, NULL);
+}
+#endif
+
 int main(int argc, char **argv, char **envp)
 {
     int i;
@@ -2945,6 +2965,10 @@ int main(int argc, char **argv, char **envp)
 #endif
     }
 
+#ifdef CONFIG_POSIX
+    setup_signal_handlers();
+#endif
+
     module_call_init(MODULE_INIT_QOM);
 
     qemu_add_opts(&qemu_drive_opts);
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH] vl.c: Implement SIGILL signal handler for triggering SIGSEGV

[Paolo Bonzini]

Il 05/09/2013 14:19, Michal Novotny ha scritto:
> This is the patch to introduce SIGILL handler to be able to trigger
> SIGSEGV signal in qemu. This has been written to help debugging
> state when qemu crashes by SIGSEGV as a simple reproducer to
> emulate such situation in case of need.

What's wrong with "kill -11" or, within gdb, "j *0x1234"?  Why do you
need a SIGILL handler for this?  In fact, SIGILL is a pretty bad choice:
QEMU includes a JIT compiler, so a SIGILL is a relatively common thing
to happen while debugging it.

Also:

(1) there is a known bug in qemu-thread-posix.c, which should not block
SIGILL, SIGBUS, SIGSEGV, SIGFPE and SIGSYS.  Without fixing that, this
trick will only work for the iothread and not for the VCPU threads.  If
you can produce a patch for this, it would be very nice.

> 
> +    int *p = NULL;
> +
> +    *p = 0xDEADBEEF;

(2) This is undefined behavior.  You probably want something like
"volatile int *p = (volatile int *)(intptr_t)4;" instead.

Paolo

Re: [Qemu-devel] [PATCH] vl.c: Implement SIGILL signal handler for triggering SIGSEGV

[Laszlo Ersek]

On 09/05/13 15:26, Paolo Bonzini wrote:
> Il 05/09/2013 14:19, Michal Novotny ha scritto:
>> This is the patch to introduce SIGILL handler to be able to trigger
>> SIGSEGV signal in qemu. This has been written to help debugging
>> state when qemu crashes by SIGSEGV as a simple reproducer to
>> emulate such situation in case of need.
> 
> What's wrong with "kill -11" or, within gdb, "j *0x1234"?  Why do you
> need a SIGILL handler for this?  In fact, SIGILL is a pretty bad choice:
> QEMU includes a JIT compiler, so a SIGILL is a relatively common thing
> to happen while debugging it.
> 
> Also:
> 
> (1) there is a known bug in qemu-thread-posix.c, which should not block
> SIGILL, SIGBUS, SIGSEGV, SIGFPE and SIGSYS.  Without fixing that, this
> trick will only work for the iothread and not for the VCPU threads.  If
> you can produce a patch for this, it would be very nice.
> 
>>
>> +    int *p = NULL;
>> +
>> +    *p = 0xDEADBEEF;
> 
> (2) This is undefined behavior.  You probably want something like
> "volatile int *p = (volatile int *)(intptr_t)4;" instead.

What's wrong with raise(SIGSEGV)?

I don't understand the motivation BTW -- what sense does it make to turn
SIGILL into SIGSEGV?

If someone just wants to force a "coredump due to signal" interactively,
SIGQUIT was invented *exactly* for that. You can even send it from the
controlling terminal directly, with Ctrl-\. (More precisely, by entering
QUIT character, see eg. the stty manual.)

(Also, in this specific case it would be no problem if all but one
threads blocked SIGQUIT -- the terminal driver or the "kill" utility
would generate the signal for the entire process, not a specific thread,
and then the signal would be delivered to some thread among those
threads that are not blocking the signal.)

Laszlo

Re: [Qemu-devel] [PATCH] vl.c: Implement SIGILL signal handler for triggering SIGSEGV

[Anthony Liguori]

On Thu, Sep 5, 2013 at 7:20 AM, Michal Novotny <minovotn@redhat.com> wrote:
> This is the patch to introduce SIGILL handler to be able to trigger
> SIGSEGV signal in qemu. This has been written to help debugging
> state when qemu crashes by SIGSEGV as a simple reproducer to
> emulate such situation in case of need.
>
> Signed-off-by: Michal Novotny <minovotn@redhat.com>
> ---
>  vl.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index 7e04641..3966271 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2897,6 +2897,26 @@ static int object_create(QemuOpts *opts, void *opaque)
>      return 0;
>  }
>
> +#ifdef CONFIG_POSIX
> +static void signal_handler(int signal)
> +{
> +    int *p = NULL;
> +
> +    *p = 0xDEADBEEF;

I won't repeat the questions from Paolo and Lazlo (I share their
confusion) but will simply add that you cannot rely on NULL address
accessing causing a SEGV.  Even with all the use of volatile in the
world, there's no guarantee this is going to crash.

Regards,

Anthony Liguori

> +}
> +
> +static void setup_signal_handlers(void)
> +{
> +    struct sigaction action;
> +
> +    memset(&action, 0, sizeof(action));
> +    sigfillset(&action.sa_mask);
> +    action.sa_handler = signal_handler;
> +    action.sa_flags = 0;
> +    sigaction(SIGILL, &action, NULL);
> +}
> +#endif
> +
>  int main(int argc, char **argv, char **envp)
>  {
>      int i;
> @@ -2945,6 +2965,10 @@ int main(int argc, char **argv, char **envp)
>  #endif
>      }
>
> +#ifdef CONFIG_POSIX
> +    setup_signal_handlers();
> +#endif
> +
>      module_call_init(MODULE_INIT_QOM);
>
>      qemu_add_opts(&qemu_drive_opts);
> --
> 1.7.11.7
>

Re: [Qemu-devel] [PATCH] vl.c: Implement SIGILL signal handler for triggering SIGSEGV

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 850 bytes --]

On 09/05/2013 04:50 PM, Anthony Liguori wrote:
>> +    int *p = NULL;
>> +
>> +    *p = 0xDEADBEEF;
> 
> I won't repeat the questions from Paolo and Lazlo (I share their
> confusion) but will simply add that you cannot rely on NULL address
> accessing causing a SEGV.  Even with all the use of volatile in the
> world, there's no guarantee this is going to crash.

If you want to guarantee that a write would cause a SEGV, then you have
to use mmap(MAP_ANONYMOUS|MAP_PRIVATE) + mprotect(PROT_NONE) to get a
valid unwritable pointer that will reliably fault, rather than hoping
that NULL (or any other low-valued intptr_t cast to void*) is
sufficiently protected.  But I also echo the question: why is raise()
insufficient?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] vl.c: Implement SIGILL signal handler for triggering SIGSEGV

[Michal Novotny]

On 09/06/2013 12:50 AM, Anthony Liguori wrote:
> On Thu, Sep 5, 2013 at 7:20 AM, Michal Novotny <minovotn@redhat.com> wrote:
>> This is the patch to introduce SIGILL handler to be able to trigger
>> SIGSEGV signal in qemu. This has been written to help debugging
>> state when qemu crashes by SIGSEGV as a simple reproducer to
>> emulate such situation in case of need.
>>
>> Signed-off-by: Michal Novotny <minovotn@redhat.com>
>> ---
>>  vl.c | 24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/vl.c b/vl.c
>> index 7e04641..3966271 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2897,6 +2897,26 @@ static int object_create(QemuOpts *opts, void *opaque)
>>      return 0;
>>  }
>>
>> +#ifdef CONFIG_POSIX
>> +static void signal_handler(int signal)
>> +{
>> +    int *p = NULL;
>> +
>> +    *p = 0xDEADBEEF;
> I won't repeat the questions from Paolo and Lazlo (I share their
> confusion) but will simply add that you cannot rely on NULL address
> accessing causing a SEGV.  Even with all the use of volatile in the
> world, there's no guarantee this is going to crash.
>
> Regards,
>
> Anthony Liguori

The idea was to trigger SIGSEGV (working at least at test conditions) to
find out current qemu state. Of course, using gdb is also an option.

Please ignore this patch, it was rather one purpose patch used in testing...

Thanks,
Michal
>
>> +}
>> +
>> +static void setup_signal_handlers(void)
>> +{
>> +    struct sigaction action;
>> +
>> +    memset(&action, 0, sizeof(action));
>> +    sigfillset(&action.sa_mask);
>> +    action.sa_handler = signal_handler;
>> +    action.sa_flags = 0;
>> +    sigaction(SIGILL, &action, NULL);
>> +}
>> +#endif
>> +
>>  int main(int argc, char **argv, char **envp)
>>  {
>>      int i;
>> @@ -2945,6 +2965,10 @@ int main(int argc, char **argv, char **envp)
>>  #endif
>>      }
>>
>> +#ifdef CONFIG_POSIX
>> +    setup_signal_handlers();
>> +#endif
>> +
>>      module_call_init(MODULE_INIT_QOM);
>>
>>      qemu_add_opts(&qemu_drive_opts);
>> --
>> 1.7.11.7
>>

-- 
Michal Novotny <minovotn@redhat.com>, RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org