[PATCH] audit handling of waitpid() status codes

Martin Pool mbp at samba.org
Fri Jan 10 00:40:29 GMT 2003


On  9 Jan 2003, jra at dp.samba.org wrote:

Thanks for checking it.

> Part of this (the smbd/chgpasswd.c patch) is incorrect I think.
> 
> You have changed the line :
> 
> if (WIFEXITED(wstat) == 0) {
> 	...
> 	return False;
> }
> 
> to
> 
> if (WIFEXITED(wstat)) {
> 	...
> 	return False;
> }
> 
> The man page states :
> 
>        WIFEXITED(status)
>               is non-zero if the child exited normally.

"exited normally" in this context means "called _exit()", rather than
being terminated by a signal.  It doesn't necessarily mean "exited 0".
To determine that you need to evaluate WIFEXITED(s) && (WEXITSTATUS(s)
== 0).

> This particular clause is meant to catch an error condition
> (not a normal exit). I agree it's not good code and could be cleaned
> up but this change reverses the return code on password change
> success.

You're right, I misunderstood what it was trying to do, because the
"process exited while we were waiting" message is printed only when
it's not true.  Here's an updated patch which corrects the messages
and returns the same values.


Index: client/smbmount.c
===================================================================
RCS file: /data/cvs/samba/source/client/smbmount.c,v
retrieving revision 1.57
diff -u -u -r1.57 smbmount.c
--- client/smbmount.c	13 Nov 2002 02:21:55 -0000	1.57
+++ client/smbmount.c	9 Jan 2003 23:11:13 -0000
@@ -79,7 +79,11 @@
 			break;
 		}
 		/* If we get here - the child exited with some error status */
-		exit(status);
+		if (WIFSIGNALLED(status)) {
+			exit(128 + WTERMSIG(status));
+		} else {
+			exit(WEXITSTATUS(status));
+		}
 	}
 
 	signal( SIGTERM, SIG_DFL );
@@ -499,6 +503,9 @@
 	if (WIFEXITED(status) && WEXITSTATUS(status) != 0) {
 		fprintf(stderr,"smbmnt failed: %d\n", WEXITSTATUS(status));
 		/* FIXME: do some proper error handling */
+		exit(1);
+	} else if (WIFSIGNALLED(status)) {
+		fprintf(stderr, "smbmnt killed by signal %d\n", WTERMSIG(status));
 		exit(1);
 	}
 
Index: lib/smbrun.c
===================================================================
RCS file: /data/cvs/samba/source/lib/smbrun.c,v
retrieving revision 1.20
diff -u -u -r1.20 smbrun.c
--- lib/smbrun.c	28 Jul 2002 02:20:15 -0000	1.20
+++ lib/smbrun.c	9 Jan 2003 23:11:13 -0000
@@ -130,6 +130,11 @@
 			return WEXITSTATUS(status);
 		}
 #endif
+#if defined(WIFSIGNALLED) && defined(WTERMSIG)
+		if (WIFSIGNALLED(status)) {
+			return 128 + WTERMSIG(status);
+		}
+#endif
 
 		return status;
 	}
Index: lib/util_file.c
===================================================================
RCS file: /data/cvs/samba/source/lib/util_file.c,v
retrieving revision 1.36
diff -u -u -r1.36 util_file.c
--- lib/util_file.c	28 Jun 2002 03:19:20 -0000	1.36
+++ lib/util_file.c	9 Jan 2003 23:11:13 -0000
@@ -362,7 +362,7 @@
 	while ((n = read(fd, buf, sizeof(buf))) > 0) {
 		tp = Realloc(p, total + n + 1);
 		if (!tp) {
-		        DEBUG(0,("file_pload: failed to exand buffer!\n"));
+		        DEBUG(0,("file_pload: failed to expand buffer!\n"));
 			close(fd);
 			SAFE_FREE(p);
 			return NULL;
@@ -372,6 +372,8 @@
 	}
 	if (p) p[total] = 0;
 
+	/* FIXME: Perhaps ought to check that the command completed
+	 * successfully; if not the data may be truncated. */
 	sys_pclose(fd);
 
 	if (size) *size = total;
Index: smbd/chgpasswd.c
===================================================================
RCS file: /data/cvs/samba/source/smbd/chgpasswd.c,v
retrieving revision 1.98
diff -u -u -r1.98 chgpasswd.c
--- smbd/chgpasswd.c	9 Jan 2003 06:58:07 -0000	1.98
+++ smbd/chgpasswd.c	9 Jan 2003 23:11:14 -0000
@@ -408,20 +408,22 @@
 			      ("We were waiting for the wrong process ID\n"));
 			return (False);
 		}
-		if (WIFEXITED(wstat) == 0)
+		
+		if (WIFEXITED(wstat) && WEXITSTATUS(wstat) != 0)
 		{
 			DEBUG(3,
-			      ("The process exited while we were waiting\n"));
+			      ("The process exited with code %d while we were waiting\n",
+			       WEXITSTATUS(wstat)));
 			return (False);
 		}
-		if (WEXITSTATUS(wstat) != 0)
+		else if (WIFSIGNALED(wstat))
 		{
 			DEBUG(3,
-			      ("The status of the process exiting was %d\n",
-			       wstat));
+			      ("The process was killed by signal %d while we were waiting\n",
+			      WTERMSIG(wstat)));
 			return (False);
 		}
-
+		/* otherwise successful completion */ 
 	}
 	else
 	{
Index: tdb/tdbtorture.c
===================================================================
RCS file: /data/cvs/samba/source/tdb/tdbtorture.c,v
retrieving revision 1.16
diff -u -u -r1.16 tdbtorture.c
--- tdb/tdbtorture.c	11 Dec 2001 08:31:58 -0000	1.16
+++ tdb/tdbtorture.c	9 Jan 2003 23:11:14 -0000
@@ -203,9 +203,14 @@
 				       (int)pids[i+1]);
 				exit(1);
 			}
-			if (WEXITSTATUS(status) != 0) {
+			if (WIFEXITED(status) && WEXITSTATUS(status) != 0) {
 				printf("child %d exited with status %d\n",
 				       (int)pids[i+1], WEXITSTATUS(status));
+				exit(1);
+			}
+			else if (WIFSIGNALLED(status)) {
+				printf("child %d killed by signal %d\n",
+				       (int)pids[i+1], WTERMSIG(status));
 				exit(1);
 			}
 		}
-- 
Martin 





More information about the samba-technical mailing list