|
From: | Fei Li |
Subject: | Re: [Qemu-devel] [PATCH RFC v7 1/9] Fix segmentation fault when qemu_signal_init fails |
Date: | Tue, 6 Nov 2018 13:08:26 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
Hi, On 11/05/2018 09:32 PM, Juan Quintela wrote:
We have discussed this in the first round of this patch series, just as DanielFei Li <address@hidden> wrote: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(). To avoid such segmentation fault, add a new Error parameter to make the call trace to propagate the err to the final caller.Hi I agree that there is a bug that exist here. But I think that the patch is not 100% correct. What is the warrantee that when we call qemu_signal_init() errp is not *already* assigned. I think that we need to use here the same code that in the call to aio_context_new() ... i.e. intsead of thisinit_clocks(qemu_timer_notify_cb);- ret = qemu_signal_init();+ ret = qemu_signal_init(errp); if (ret) { return ret; }init_clocks(qemu_timer_notify_cb); ret = qemu_signal_init(); ret = qemu_signal_init(&local_error); if (ret) { error_propagate(errp, local_error); return ret; } This way it works correctly if errp is NULL, errp is already assigned, etc, etc, Or I am missing something? Later, Juan.
and Fam said, we only need the local_err & error_propagate() when functionslike object_new_with_propv() returns void, in that way we need the &local_err to
check whether that function succeeds.But in qemu_signal_init, we have the "if (ret) {...}" to judge whether it succeeds.
For more details, the following threads can be referred: 09/04/2018 07:26 PMRe: [Qemu-devel] [PATCH 1/5] Fix segmentation fault when qemu_signal_init fails
BTW, if qemu_signalfd() fails, we use an "error_setg_errno()" to handle: - for NULL errp, we just set the error message to errp;- for not-NULL errp, besides the error_setv() we have the error_handle_fatal(errp, err). If the passed errp is &error_fatal/&error_abort, qemu will exit(1) right here.
Have a nice day, thanks :) Fei
[Prev in Thread] | Current Thread | [Next in Thread] |