Add a cap_iab_dup() function and make IAB access atomic.

Embed mutex locked operation into the IAB API. The idea being that
while libcap operates on an IAB tuple, it cannot be operated on by
a thread running in parallel. This makes IAB access thread safe (but
not reentrant).

The only potential API behavioral change is that the IAB tuple
associated with a cap_launcher_t is now locked for the duration of
its association with that launcher. This prevents a race condition
with launching and another thread changing that IAB tuple.

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

10 files changed, 97 insertions, 16 deletions

diff --git a/doc/Makefile b/doc/Makefile
index 4fc9ea5..f014b28 100644
--- a/doc/Makefile
+++ b/doc/Makefile
@@ -22,7 +22,7 @@ MAN3S = cap_init.3 cap_free.3 cap_dup.3 \
 	cap_launcher_set_chroot.3 cap_launcher_set_mode.3 \
 	cap_launcher_setgroups.3 cap_launcher_setuid.3 \
 	cap_launcher_set_iab.3 cap_new_launcher.3 \
-	cap_iab.3 cap_iab_init.3 cap_iab_compare.3 \
+	cap_iab.3 cap_iab_init.3 cap_iab_dup.3 cap_iab_compare.3 \
 	cap_iab_get_proc.3 cap_iab_get_pid.3 cap_iab_set_proc.3 \
 	cap_iab_to_text.3 cap_iab_from_text.3 cap_iab_get_vector.3 \
 	cap_iab_set_vector.3 cap_iab_fill.3 \
diff --git a/doc/cap_iab.3 b/doc/cap_iab.3
index a0ec988..4a55d0b 100644
--- a/doc/cap_iab.3
+++ b/doc/cap_iab.3
@@ -5,6 +5,8 @@
 
 cap_iab_t cap_iab_init(void);
 
+cap_iab_t cap_iab_dup(cap_iab_t iab);
+
 cap_iab_t cap_iab_get_proc(void);
 
 cap_iab_t cap_iab_get_pid(pid_t pid);
@@ -72,6 +74,11 @@ won't bestow any either. The returned \fIcap_iab_t\fP should be freed
 with
 .BR cap_free (3).
 .sp
+.BR cap_iab_dup ()
+returns a copy of the specified IAB value.  The returned cap_iab_t
+should be freed with
+.BR cap_free (3).
+.sp
 .BR cap_iab_get_proc ()
 returns a copy of the IAB value for the current process.  The returned
 cap_iab_t should be freed with
diff --git a/doc/cap_iab_dup.3 b/doc/cap_iab_dup.3
new file mode 100644
index 0000000..3e730b1
--- /dev/null
+++ b/doc/cap_iab_dup.3
@@ -0,0 +1 @@
+.so man3/cap_iab.3
diff --git a/doc/cap_launch.3 b/doc/cap_launch.3
index 03d50f4..786bca3 100644
--- a/doc/cap_launch.3
+++ b/doc/cap_launch.3
@@ -124,9 +124,11 @@ the launcher. Calling this function with an IAB value of NULL will
 configure the launcher to not set an IAB value (the default).  See
 \fBcap_iab\fP(3) for details on the IAB set. Note, the launcher is
 associated directly with the supplied \fIiab\fP value, and does not
-make a copy of it. Set with NULL to regain control over the memory
-associated with that IAB value, otherwise the IAB value will be
-\fBcap_free\fI()\fP'd when the launcher is.
+make a copy of it. This iab value is locked to the laucher and cannot
+be modified while associated with the launcher. Set with NULL to
+regain control over the memory associated with that IAB value,
+otherwise the IAB value will be \fBcap_free\fI()\fP'd when the
+launcher is.
 .sp
 .BR cap_launcher_set_chroot ()
 This function causes the launched program executable to be invoked
diff --git a/libcap/cap_alloc.c b/libcap/cap_alloc.c
index 6a6ca2c..6a674f4 100644
--- a/libcap/cap_alloc.c
+++ b/libcap/cap_alloc.c
@@ -129,8 +129,7 @@ cap_t cap_dup(cap_t cap_d)
 {
     cap_t result;
 
-    __u32 *magic_p = -2 + (__u32 *) cap_d;
-    if (*magic_p != CAP_T_MAGIC) {
+    if (!good_cap_t(cap_d)) {
 	_cap_debug("bad argument");
 	errno = EINVAL;
 	return NULL;
@@ -163,6 +162,35 @@ cap_iab_t cap_iab_init(void)
 }
 
 /*
+ * This function duplicates an internal iab tuple with calloc()'d
+ * memory. It is the responsibility of the user to call cap_free() to
+ * liberate it.
+ */
+cap_iab_t cap_iab_dup(cap_iab_t iab)
+{
+    cap_iab_t result;
+
+    if (!good_cap_iab_t(iab)) {
+	_cap_debug("bad argument");
+	errno = EINVAL;
+	return NULL;
+    }
+
+    result = cap_iab_init();
+    if (result == NULL) {
+	_cap_debug("out of memory");
+	return NULL;
+    }
+
+    _cap_mu_lock(&iab->mutex);
+    memcpy(result, iab, sizeof(*iab));
+    _cap_mu_unlock(&iab->mutex);
+    _cap_mu_unlock(&result->mutex);
+
+    return result;
+}
+
+/*
  * cap_new_launcher allocates some memory for a launcher and
  * initializes it.  To actually launch a program with this launcher,
  * use cap_launch(). By default, the launcher is a no-op from a
@@ -236,8 +264,11 @@ int cap_free(void *data_p)
     case CAP_IAB_MAGIC:
 	break;
     case CAP_LAUNCH_MAGIC:
-	if (cap_free(data->u.launcher.iab) != 0) {
-	    return -1;
+	if (data->u.launcher.iab != NULL) {
+	    _cap_mu_unlock(&data->u.launcher.iab->mutex);
+	    if (cap_free(data->u.launcher.iab) != 0) {
+		return -1;
+	    }
 	}
 	data->u.launcher.iab = NULL;
 	if (cap_free(data->u.launcher.chroot) != 0) {
diff --git a/libcap/cap_flag.c b/libcap/cap_flag.c
index 39d8646..94afd1e 100644
--- a/libcap/cap_flag.c
+++ b/libcap/cap_flag.c
@@ -217,20 +217,25 @@ cap_flag_value_t cap_iab_get_vector(cap_iab_t iab, cap_iab_vector_t vec,
 
     unsigned o = (bit >> 5);
     __u32 mask = 1u << (bit & 31);
+    cap_flag_value_t ret;
 
+    _cap_mu_lock(&iab->mutex);
     switch (vec) {
     case CAP_IAB_INH:
-	return !!(iab->i[o] & mask);
+	ret = !!(iab->i[o] & mask);
 	break;
     case CAP_IAB_AMB:
-	return !!(iab->a[o] & mask);
+	ret = !!(iab->a[o] & mask);
 	break;
     case CAP_IAB_BOUND:
-	return !!(iab->nb[o] & mask);
+	ret = !!(iab->nb[o] & mask);
 	break;
     default:
-	return 0;
+	ret = 0;
     }
+    _cap_mu_unlock(&iab->mutex);
+
+    return ret;
 }
 
 /*
@@ -250,6 +255,7 @@ int cap_iab_set_vector(cap_iab_t iab, cap_iab_vector_t vec, cap_value_t bit,
     __u32 on = 1u << (bit & 31);
     __u32 mask = ~on;
 
+    _cap_mu_lock(&iab->mutex);
     switch (vec) {
     case CAP_IAB_INH:
 	iab->i[o] = (iab->i[o] & mask) | (raised ? on : 0);
@@ -264,9 +270,10 @@ int cap_iab_set_vector(cap_iab_t iab, cap_iab_vector_t vec, cap_value_t bit,
 	break;
     default:
 	errno = EINVAL;
-	return -1;
+	_cap_mu_unlock_return(&iab->mutex, -1);
     }
 
+    _cap_mu_unlock(&iab->mutex);
     return 0;
 }
 
@@ -306,6 +313,7 @@ int cap_iab_fill(cap_iab_t iab, cap_iab_vector_t vec,
 	return -1;
     }
 
+    _cap_mu_lock(&iab->mutex);
     for (i = 0; !ret && i < _LIBCAP_CAPABILITY_U32S; i++) {
 	switch (vec) {
 	case CAP_IAB_INH:
@@ -325,6 +333,7 @@ int cap_iab_fill(cap_iab_t iab, cap_iab_vector_t vec,
 	    break;
 	}
     }
+    _cap_mu_unlock(&iab->mutex);
 
     cap_free(cap_d);
     return ret;
@@ -341,11 +350,20 @@ int cap_iab_compare(cap_iab_t a, cap_iab_t b)
 	errno = EINVAL;
 	return -1;
     }
+    b = cap_iab_dup(b);
+    if (b == NULL) {
+	return -1;
+    }
+
+    _cap_mu_lock(&a->mutex);
     for (j=0, result=0; j<_LIBCAP_CAPABILITY_U32S; j++) {
 	result |=
 	    (a->i[j] == b->i[j] ? 0 : (1 << CAP_IAB_INH)) |
 	    (a->a[j] == b->a[j] ? 0 : (1 << CAP_IAB_AMB)) |
 	    (a->nb[j] == b->nb[j] ? 0 : (1 << CAP_IAB_BOUND));
     }
+    _cap_mu_unlock(&a->mutex);
+    cap_free(b);
+
     return result;
 }
diff --git a/libcap/cap_proc.c b/libcap/cap_proc.c
index a26fe5a..8a10f75 100644
--- a/libcap/cap_proc.c
+++ b/libcap/cap_proc.c
@@ -745,6 +745,7 @@ cap_iab_t cap_iab_get_proc(void)
 
 /*
  * _cap_iab_set_proc sets the iab collection using the requested syscaller.
+ * The iab value is locked by the caller.
  */
 static int _cap_iab_set_proc(struct syscaller_s *sc, cap_iab_t iab)
 {
@@ -818,7 +819,15 @@ defer:
  */
 int cap_iab_set_proc(cap_iab_t iab)
 {
-    return _cap_iab_set_proc(&multithread, iab);
+    int retval;
+    if (!good_cap_iab_t(iab)) {
+	errno = EINVAL;
+	return -1;
+    }
+    _cap_mu_lock(&iab->mutex);
+    retval = _cap_iab_set_proc(&multithread, iab);
+    _cap_mu_unlock(&iab->mutex);
+    return retval;
 }
 
 /*
@@ -871,13 +880,22 @@ void cap_launcher_set_mode(cap_launch_t attr, cap_mode_t flavor)
 }
 
 /*
- * cap_launcher_set_iab primes the launcher to attempt to change the iab bits of
- * the launched child.
+ * cap_launcher_set_iab primes the launcher to attempt to change the
+ * IAB values of the launched child. The launcher locks iab while it
+ * is owned by the launcher: this prevents the user from
+ * asynchronously changing its value while it is associated with the
+ * launcher.
  */
 cap_iab_t cap_launcher_set_iab(cap_launch_t attr, cap_iab_t iab)
 {
     cap_iab_t old = attr->iab;
     attr->iab = iab;
+    if (old != NULL) {
+	_cap_mu_unlock(&old->mutex);
+    }
+    if (iab != NULL) {
+	_cap_mu_lock(&iab->mutex);
+    }
     return old;
 }
 
diff --git a/libcap/cap_text.c b/libcap/cap_text.c
index 40bd59a..8dfe9f8 100644
--- a/libcap/cap_text.c
+++ b/libcap/cap_text.c
@@ -520,6 +520,7 @@ char *cap_iab_to_text(cap_iab_t iab)
     int first = 1;
 
     if (good_cap_iab_t(iab)) {
+	_cap_mu_lock(&iab->mutex);
 	for (c = 0; c < cmb; c++) {
 	    int keep = 0;
 	    int o = c >> 5;
@@ -553,6 +554,7 @@ char *cap_iab_to_text(cap_iab_t iab)
 		first = 0;
 	    }
 	}
+	_cap_mu_unlock(&iab->mutex);
     }
     *p = '\0';
     return _libcap_strdup(buf);
diff --git a/libcap/include/sys/capability.h b/libcap/include/sys/capability.h
index 20209d5..4f499dc 100644
--- a/libcap/include/sys/capability.h
+++ b/libcap/include/sys/capability.h
@@ -127,6 +127,7 @@ typedef unsigned cap_mode_t;
 extern cap_t      cap_dup(cap_t);
 extern int        cap_free(void *);
 extern cap_t      cap_init(void);
+extern cap_iab_t  cap_iab_dup(cap_iab_t);
 extern cap_iab_t  cap_iab_init(void);
 
 /* libcap/cap_flag.c */
diff --git a/libcap/libcap.h b/libcap/libcap.h
index 0b57a53..374ee7c 100644
--- a/libcap/libcap.h
+++ b/libcap/libcap.h
@@ -253,6 +253,7 @@ extern int capsetp(pid_t pid, cap_t cap_d);
  * applied.
  */
 struct cap_iab_s {
+    __u8 mutex;
     __u32 i[_LIBCAP_CAPABILITY_U32S];
     __u32 a[_LIBCAP_CAPABILITY_U32S];
     __u32 nb[_LIBCAP_CAPABILITY_U32S];