Sophie

Sophie

distrib > Scientific%20Linux > 5x > x86_64 > by-pkgid > 4b9945308a4ba163aa4bbd43d0a1135d > files > 48

conga-0.12.2-32.el5_7.1.src.rpm

From: Jan Pokorny <jpokorny@redhat.com>
Date: Tue, 13 Sep 2011 20:15:33 +0200
Subject: [PATCH 1/4] (bz732483) avoid using uninitialized var if waitpid not succeeded

Actually, modified lines, suspected as a source of undesired behavior observed
regarding bz732483, were brought in as a fix for bz564490.

This patch author's hypothesis about how things go wrong:

1. int status;
   - uninitialized local variable
2. waitpid(pid, &status, WNOHANG);
   - disregarding of return value leading to ...
3. if (WIFEXITED(status))
   - ... use of still uninitialized value in case waitpid did not succeeded
     because of error or a case that process being wait for has not yet
     changed the state

1. + 2. + 3. -> (almost) unpredictably taking this or the other branch
of computation according to unitialized value ("random data"), leading to...

4. parent execution gets out of "while (true)" loop as in these correct cases:
   a. waitpid (see 2.) works/used correctly this time
   b. both remote ends of "stdout and stderr pipes" were closed;
   then waits for child to finish, no longer reads anything from pipes
5. child (actual execution of the required program, e.g. "yum" as in bz732483)
   wants to write something to stdout/stderr, but once pipe buffer is full,
   it is blocked

4. + 5. -> deadlock

Note: The fact that bad behavior had not been observed for quite a long time
can be explained this way:
    Content of uninitialized variable is (presumably) influenced by stack
    contents in respective point of computation that in turn maybe influenced
    by the compiler (i.e. instructions or maybe even memory addresses
    [their patterns across similar environments?] being on the stack at that
    very moment)

Signed-off-by: Jan Pokorny <jpokorny@redhat.com>

--- a/ricci/common/executils.cpp
+++ b/ricci/common/executils.cpp
@@ -185,8 +185,7 @@
 		int ret = poll(poll_data, s, 500);
 		if (ret == 0) {
 			int status;
-			waitpid(pid, &status, WNOHANG);
-			if (WIFEXITED(status))
+			if (waitpid(pid, &status, WNOHANG) > 0 && WIFEXITED(status))
 				break;
 			continue;
 		} else if (ret == -1) {