On Wed, Apr 18, 2012 at 04:30:48PM -0300, Luiz Capitulino wrote:
> A child created by bios_supports_mode() could terminate during the call
> to close() or g_free(). This could cause the SIGCHLD signal to be
> deliveried in the midle of their execution. Possible problems range from
> resource leak to segfault. Fix that by blocking SIGCHLD during those calls.
>
> Also, tries to explain why bios_supports_mode() got so complex...
>
> Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
Looks good.
> ---
> qga/commands-posix.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index faf970d..41ba0c5 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -521,14 +521,18 @@ static void guest_fsfreeze_cleanup(void)
> * This function forks twice and the information about the mode support
> * status is passed to the qemu-ga process via a pipe.
> *
> - * This approach allows us to keep the way we reap terminated children
> - * in qemu-ga quite simple.
> + * XXX: This approach is a bit complex, it's implemented this way to avoid
> + * calling waitpid() from the main qemu-ga process, as this could cause
> + * interference with other commands that create new processes. The
> + * solution to this problem is to introduce an internal API to safely
> + * create & wait for children processes.
> */
> static void bios_supports_mode(const char *pmutils_bin, const char
> *pmutils_arg,
> const char *sysfile_str, Error **err)
> {
> pid_t pid;
> ssize_t ret;
> + sigset_t sigset;
> char *pmutils_path;
> int status, pipefds[2];
>
> @@ -603,9 +607,15 @@ static void bios_supports_mode(const char *pmutils_bin,
> const char *pmutils_arg,
> _exit(EXIT_SUCCESS);
> }
>
> + sigemptyset(&sigset);
> + sigaddset(&sigset, SIGCHLD);
> + pthread_sigmask(SIG_BLOCK, &sigset, NULL);
> +
> close(pipefds[1]);
> g_free(pmutils_path);
>
> + pthread_sigmask(SIG_UNBLOCK, &sigset, NULL);
> +
> if (pid < 0) {
> error_set(err, QERR_UNDEFINED_ERROR);
> goto out;
> --
> 1.7.9.2.384.g4a92a
>
>
