Make cap_launcher_t operations atomic.

Modify the cap_launch() behavior when chroot is set. Now, the
launcher code will force the post chroot() environment to
chdir("/").

Modify the API for many of the cap_launch_*() functions that
previously were void, to returning int (0=OK, -1=see errno).
I'm confident that this should be code backwardly compatible,
since the return values are new and prior code would have been
assuming success.

Signed-off-by: Andrew G. Morgan <morgan@kernel.org>

4 files changed, 72 insertions, 21 deletions

diff --git a/doc/cap_launch.3 b/doc/cap_launch.3
index 786bca3..95313ec 100644
--- a/doc/cap_launch.3
+++ b/doc/cap_launch.3
@@ -8,17 +8,18 @@ cap_launch_t cap_new_launcher(const char *arg0, const char * const *argv,
 
 cap_launch_t cap_func_launcher(int (callback_fn)(void *detail));
 
-void cap_launcher_callback(cap_launch_t attr,
+int cap_launcher_callback(cap_launch_t attr,
     int (callback_fn)(void *detail));
-void cap_launcher_set_mode(cap_launch_t attr, cap_mode_t flavor);
+int cap_launcher_set_mode(cap_launch_t attr, cap_mode_t flavor);
 cap_iab_t cap_launcher_set_iab(cap_launch_t attr, cap_iab_t iab);
-void cap_launcher_set_chroot(cap_launch_t attr, const char *chroot);
+int cap_launcher_set_chroot(cap_launch_t attr, const char *chroot);
 
 #include <sys/types.h>
 
 pid_t cap_launch(cap_launch_t attr, void *detail);
-void cap_launcher_setuid(cap_launch_t attr, uid_t uid);
-void cap_launcher_setgroups(cap_launch_t attr, gid_t gid,
+int cap_launcher_setuid(cap_launch_t attr, uid_t uid);
+int cap_launcher_setgroups(cap_launch_t attr, gid_t gid,
+    int ngroups, const gid_t *groups);
 .fi
 .sp
 Link with \fI\-lcap\fP.
@@ -155,7 +156,8 @@ should be considered an error.
 .BR cap_launch ()
 returns -1 in the case of an error.
 .PP
-In all such cases consult
+In all such cases a return value of 0 implies success. In other cases,
+consult
 .BR errno (3)
 for further details.
 .SH "HISTORY"
diff --git a/libcap/cap_proc.c b/libcap/cap_proc.c
index 8a10f75..22a307e 100644
--- a/libcap/cap_proc.c
+++ b/libcap/cap_proc.c
@@ -842,41 +842,69 @@ int cap_iab_set_proc(cap_iab_t iab)
  * considered to have failed and the launch will be aborted - further,
  * errno will be communicated to the parent.
  */
-void cap_launcher_callback(cap_launch_t attr, int (callback_fn)(void *detail))
+int cap_launcher_callback(cap_launch_t attr, int (callback_fn)(void *detail))
 {
+    if (!good_cap_launch_t(attr)) {
+	errno = EINVAL;
+	return -1;
+    }
+    _cap_mu_lock(&attr->mutex);
     attr->custom_setup_fn = callback_fn;
+    _cap_mu_unlock(&attr->mutex);
+    return 0;
 }
 
 /*
  * cap_launcher_setuid primes the launcher to attempt a change of uid.
  */
-void cap_launcher_setuid(cap_launch_t attr, uid_t uid)
+int cap_launcher_setuid(cap_launch_t attr, uid_t uid)
 {
+    if (!good_cap_launch_t(attr)) {
+	errno = EINVAL;
+	return -1;
+    }
+    _cap_mu_lock(&attr->mutex);
     attr->uid = uid;
     attr->change_uids = 1;
+    _cap_mu_unlock(&attr->mutex);
+    return 0;
 }
 
 /*
  * cap_launcher_setgroups primes the launcher to attempt a change of
  * gid and groups.
  */
-void cap_launcher_setgroups(cap_launch_t attr, gid_t gid,
-			    int ngroups, const gid_t *groups)
+int cap_launcher_setgroups(cap_launch_t attr, gid_t gid,
+			   int ngroups, const gid_t *groups)
 {
+    if (!good_cap_launch_t(attr)) {
+	errno = EINVAL;
+	return -1;
+    }
+    _cap_mu_lock(&attr->mutex);
     attr->gid = gid;
     attr->ngroups = ngroups;
     attr->groups = groups;
     attr->change_gids = 1;
+    _cap_mu_unlock(&attr->mutex);
+    return 0;
 }
 
 /*
  * cap_launcher_set_mode primes the launcher to attempt a change of
  * mode.
  */
-void cap_launcher_set_mode(cap_launch_t attr, cap_mode_t flavor)
+int cap_launcher_set_mode(cap_launch_t attr, cap_mode_t flavor)
 {
+    if (!good_cap_launch_t(attr)) {
+	errno = EINVAL;
+	return -1;
+    }
+    _cap_mu_lock(&attr->mutex);
     attr->mode = flavor;
     attr->change_mode = 1;
+    _cap_mu_unlock(&attr->mutex);
+    return 0;
 }
 
 /*
@@ -888,6 +916,11 @@ void cap_launcher_set_mode(cap_launch_t attr, cap_mode_t flavor)
  */
 cap_iab_t cap_launcher_set_iab(cap_launch_t attr, cap_iab_t iab)
 {
+    if (!good_cap_launch_t(attr)) {
+	errno = EINVAL;
+	return NULL;
+    }
+    _cap_mu_lock(&attr->mutex);
     cap_iab_t old = attr->iab;
     attr->iab = iab;
     if (old != NULL) {
@@ -896,6 +929,7 @@ cap_iab_t cap_launcher_set_iab(cap_launch_t attr, cap_iab_t iab)
     if (iab != NULL) {
 	_cap_mu_lock(&iab->mutex);
     }
+    _cap_mu_unlock(&attr->mutex);
     return old;
 }
 
@@ -903,9 +937,16 @@ cap_iab_t cap_launcher_set_iab(cap_launch_t attr, cap_iab_t iab)
  * cap_launcher_set_chroot sets the intended chroot for the launched
  * child.
  */
-void cap_launcher_set_chroot(cap_launch_t attr, const char *chroot)
+int cap_launcher_set_chroot(cap_launch_t attr, const char *chroot)
 {
+    if (!good_cap_launch_t(attr)) {
+	errno = EINVAL;
+	return -1;
+    }
+    _cap_mu_lock(&attr->mutex);
     attr->chroot = _libcap_strdup(chroot);
+    _cap_mu_unlock(&attr->mutex);
+    return 0;
 }
 
 static int _cap_chroot(struct syscaller_s *sc, const char *root)
@@ -929,6 +970,9 @@ static int _cap_chroot(struct syscaller_s *sc, const char *root)
 	} else {
 	    ret = chroot(root);
 	}
+	if (ret == 0) {
+	    ret = chdir("/");
+	}
     }
     int olderrno = errno;
     (void) cap_clear_flag(working, CAP_EFFECTIVE);
@@ -1026,16 +1070,17 @@ pid_t cap_launch(cap_launch_t attr, void *detail) {
 	errno = EINVAL;
 	return -1;
     }
+    _cap_mu_lock(&attr->mutex);
 
     /* The launch must have a purpose */
     if (attr->custom_setup_fn == NULL &&
 	(attr->arg0 == NULL || attr->argv == NULL)) {
 	errno = EINVAL;
-	return -1;
+	_cap_mu_unlock_return(&attr->mutex, -1);
     }
 
     if (pipe2(ps, O_CLOEXEC) != 0) {
-	return -1;
+	_cap_mu_unlock_return(&attr->mutex, -1);
     }
 
     child = fork();
@@ -1047,6 +1092,9 @@ pid_t cap_launch(cap_launch_t attr, void *detail) {
 	_cap_launch(ps[1], attr, detail);
 	/* no return from above function */
     }
+
+    /* child has its own copy, and parent no longer needs it locked. */
+    _cap_mu_unlock(&attr->mutex);
     close(ps[1]);
     if (child < 0) {
 	goto defer;
diff --git a/libcap/include/sys/capability.h b/libcap/include/sys/capability.h
index 4f499dc..8719f61 100644
--- a/libcap/include/sys/capability.h
+++ b/libcap/include/sys/capability.h
@@ -218,14 +218,14 @@ typedef struct cap_launch_s *cap_launch_t;
 extern cap_launch_t cap_new_launcher(const char *arg0, const char * const *argv,
 				     const char * const *envp);
 extern cap_launch_t cap_func_launcher(int (callback_fn)(void *detail));
-extern void cap_launcher_callback(cap_launch_t attr,
-				  int (callback_fn)(void *detail));
-extern void cap_launcher_setuid(cap_launch_t attr, uid_t uid);
-extern void cap_launcher_setgroups(cap_launch_t attr, gid_t gid,
-				   int ngroups, const gid_t *groups);
-extern void cap_launcher_set_mode(cap_launch_t attr, cap_mode_t flavor);
+extern int cap_launcher_callback(cap_launch_t attr,
+				 int (callback_fn)(void *detail));
+extern int cap_launcher_setuid(cap_launch_t attr, uid_t uid);
+extern int cap_launcher_setgroups(cap_launch_t attr, gid_t gid,
+				  int ngroups, const gid_t *groups);
+extern int cap_launcher_set_mode(cap_launch_t attr, cap_mode_t flavor);
 extern cap_iab_t cap_launcher_set_iab(cap_launch_t attr, cap_iab_t iab);
-extern void cap_launcher_set_chroot(cap_launch_t attr, const char *chroot);
+extern int cap_launcher_set_chroot(cap_launch_t attr, const char *chroot);
 extern pid_t cap_launch(cap_launch_t attr, void *detail);
 
 /*
diff --git a/libcap/libcap.h b/libcap/libcap.h
index 374ee7c..a22f69a 100644
--- a/libcap/libcap.h
+++ b/libcap/libcap.h
@@ -270,6 +270,7 @@ struct cap_iab_s {
  * multithreaded applications.
  */
 struct cap_launch_s {
+    __u8 mutex;
     /*
      * Once forked but before active privilege is changed, this
      * function (if non-NULL) is called.