Source: brasero
Version: 3.4.1-2
Severity: normal
Tags: upstream patch

Dear Maintainer,

Please consider answering these questions, where appropriate ***

In libbrasero-burn/burn-process.c:brasero_process_watch_child (gpointer data)

waitpid() could have returned positive value (child's pid) due to the
child's change of state, e.g. child being signalled for any (obscure)
reason (SIGPIPE, SIGSTOP, etc), and also child's exit status is being
blindly "examined" by flat WEXITSTATUS(status), instead of first checking
WIFEXITED(status) for returning true, i.e. that the child has actually exited.

This could lead to unnoticed problem with the child process state. For 
instance, I think the log message [1] of "process finished with status
0" could be pretty much bogus in this particular case shown in the log with
a burn failure halfway around ~49-50%. (see the tail of waitpid(2) for a good
example of a diligent parent waiting for their possibly naughty child)

[1] https://launchpadlibrarian.net/71440716/brasero_log.txt


-- System Information:
Debian Release: wheezy/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing')
Architecture: amd64 (x86_64)

Kernel: Linux 3.2.0-3-amd64 (SMP w/2 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

-- no debconf information
--- brasero-3.4.1.orig/libbrasero-burn/burn-process.c
+++ brasero-3.4.1/libbrasero-burn/burn-process.c
@@ -294,12 +294,40 @@ brasero_process_watch_child (gpointer da
 	 * brasero_job_finished/_error is called before the pipes are closed so
 	 * as to let plugins read stderr / stdout till the end and set a better
 	 * error message or simply decide all went well, in one word override */
-	priv->return_status = WEXITSTATUS (status);
+/*
+	priv->return_status =  WEXITSTATUS (status);
+*/
 	priv->watch = 0;
 	priv->pid = 0;
-
+	if (WIFEXITED(status)) { /* WEXITSTATUS macro should only be used if WIFEXITED returned true */
+		priv->return_status = WEXITSTATUS (status);
+		BRASERO_JOB_LOG (data, "child process exited with status %d", WEXITSTATUS (status));
+	}
+	else if (WIFSIGNALED(status)) { /* there is no meaningful child return status */
+		int errsv = errno; /* unrecoverable error */
+		priv->error = g_error_new (BRASERO_BURN_ERROR, BRASERO_BURN_ERROR_GENERAL,
+			_("child process killed by signal %d. Error (%s)"), WTERMSIG(status), g_strerror (errsv));
+		BRASERO_JOB_LOG (data, "child process killed by signal %d", WTERMSIG(status));
+		return FALSE; /* not to be called by g_timeout_add() anymore */
+	}
+	else if (WIFSTOPPED(status)) { /* there is no meaningful child return status */
+		int errsv = errno; /* unrecoverable error or maybe try to send SIGCONT to the child? */
+		priv->error = g_error_new (BRASERO_BURN_ERROR, BRASERO_BURN_ERROR_GENERAL,
+			_("child process stopped by signal %d. Error (%s)"), WSTOPSIG(status), g_strerror (errsv));
+		BRASERO_JOB_LOG (data, "child process stopped by signal %d", WSTOPSIG(status));
+		return FALSE; /* not to be called by g_timeout_add() anymore */
+	}
+	else {
+		int errsv = errno; /* unrecoverable error */
+		priv->error = g_error_new (BRASERO_BURN_ERROR, BRASERO_BURN_ERROR_GENERAL,
+			_("child process unhandled Error (%s)"), g_strerror (errsv));
+		BRASERO_JOB_LOG (data, "child process unhandled Error (%s)", g_strerror (errsv));
+		return FALSE; /* not to be called by g_timeout_add() anymore */
+	}
+   
+/*
 	BRASERO_JOB_LOG (data, "process finished with status %i", WEXITSTATUS (status));
-
+*/
 	result = brasero_process_finished (data);
 	if (result == BRASERO_BURN_RETRY) {
 		GError *error = NULL;

Reply via email to