Re: [Qemu-devel] [PATCH RFC 1/5] Fix segmentation fault when qemu_signal_init fails

[Fei Li]

On 11/28/2018 08:53 PM, Markus Armbruster wrote:

Fei Li <f...@suse.com> writes:

When qemu_signal_init() fails in qemu_init_main_loop(), we return
without setting an error.  Its callers crash then when they try to
report the error with error_report_err().

Yes, that's a bug.  Broken in 2f78e491d7b, v2.2.0.  Has escaped notice
since qemu_signalfd() is quite unlikely to fail.  Could go into 3.1 as a
bug fix, but I think punting it to the next release is just fine.

Thanks. :) BTW, should I send the next version only includes patch 1/5
and 2/5 separately so that you can merge? (I guess Dave will help to
merge the other three migration related patches)

To avoid such segmentation fault, add a new Error parameter to make
the call trace to propagate the err to the final caller.

Let's add:

   Fixes: 2f78e491d7b46542158ce0b8132ee4e05bc0ade4

ok.

Cc: Markus Armbruster <arm...@redhat.com>
Cc: Fam Zheng <f...@redhat.com>
Signed-off-by: Fei Li <f...@suse.com>
Reviewed-by: Fam Zheng <f...@redhat.com>
---
  util/main-loop.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/util/main-loop.c b/util/main-loop.c
index affe0403c5..443cb4cfe8 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
      }
  }

-static int qemu_signal_init(void)

+static int qemu_signal_init(Error **errp)
  {
      int sigfd;
      sigset_t set;
@@ -96,7 +96,7 @@ static int qemu_signal_init(void)
      sigdelset(&set, SIG_IPI);
      sigfd = qemu_signalfd(&set);
      if (sigfd == -1) {
-        fprintf(stderr, "failed to create signalfd\n");
+        error_setg_errno(errp, errno, "failed to create signalfd");
          return -errno;
      }

@@ -109,7 +109,7 @@ static int qemu_signal_init(void) #else /* _WIN32 */ -static int qemu_signal_init(void)

+static int qemu_signal_init(Error **errp)
  {
      return 0;
  }
@@ -148,7 +148,7 @@ int qemu_init_main_loop(Error **errp)

init_clocks(qemu_timer_notify_cb); - ret = qemu_signal_init();

+    ret = qemu_signal_init(errp);
      if (ret) {
          return ret;
      }

Reviewed-by: Markus Armbruster <arm...@redhat.com>

Thanks again for the review.

Have a nice day
Fei