Page MenuHomeFreeBSD
Files F162181197

D58014.id.diff
No OneTemporary
Actions

D58014.id.diff
View Options

diff --git a/sys/security/mac_do/mac_do.c b/sys/security/mac_do/mac_do.c
--- a/sys/security/mac_do/mac_do.c
+++ b/sys/security/mac_do/mac_do.c
@@ -394,7 +394,11 @@
STAILQ_FOREACH_SAFE(rule, head, r_entries, rule_next) {
free(rule->uids, M_MAC_DO);
free(rule->gids, M_MAC_DO);
+ for (u_int i = 0; i < rule->exec_paths_nb; i++)
+ free(rule->exec_paths[i], M_MAC_DO);
+ free(rule->exec_paths, M_MAC_DO);
free(rule, M_MAC_DO);
+
}
STAILQ_INIT(head);
}
@@ -1183,6 +1187,11 @@
return (0);
einval:
+ if (new->exec_paths_nb > 0) {
+ for (u_int i = 0; i < new->exec_paths_nb; i++)
+ free(new->exec_paths[i], M_MAC_DO);
+ free(new->exec_paths, M_MAC_DO);
+ }
free(new->gids, M_MAC_DO);
free(new->uids, M_MAC_DO);
free(new, M_MAC_DO);
@@ -2252,6 +2261,19 @@
static void
dealloc_thread_osd(void *const value)
{
+ struct mac_do_setcred_data *const d = value;
+
+ /*
+ * Drop exec_conf if the thread exits before execve().
+ *
+ * clear_data() in setcred_exit releases hdr.conf but not exec_conf,
+ * which holds its own reference taken in priv_grant. The normal
+ * consumer is vnode_check_exec, but if the thread dies between
+ * setcred returning and execve (e.g., execve fails and mdo calls
+ * _exit()), nobody else will release it.
+ */
+ if (d != NULL && d->has_exec_constraint)
+ drop_conf(d->exec_conf);
free(value, M_MAC_DO);
}
@@ -2727,6 +2749,32 @@
data = fetch_data();
if (!is_data_reusable(data, sizeof(*data))) {
data = alloc_data(data, sizeof(*data));
+ /* our stale-exec-constraint cleanup reads has_exec_constraint
+ * before anything writes it — so we must initialize these fields
+ * now, before the cleanup check below.
+ */
+ data->has_exec_constraint = false;
+ data->exec_blacklist = false;
+ data->exec_paths_nb = 0;
+ data->exec_paths = NULL;
+ data->exec_conf = NULL;
+ }
+
+ /*
+ * If a previous setcred's exec constraint was never consumed
+ * (exec didn't happen, or failed before the MAC hook), drop the
+ * stale exec_conf reference.
+ * check_setcred also handles this, but this catches the case where
+ * check_setcred runs on stale data before it's reset.
+ *
+ * Safe to read has_exec_constraint here: on fresh allocation it was
+ * just set to false above; on reused data it retains its previous value.
+ */
+ if (data != NULL && data->has_exec_constraint) {
+ drop_conf(data->exec_conf);
+ data->has_exec_constraint = false;
+ }
+
set_data_header(data, sizeof(*data), PRIV_CRED_SETCRED, conf);
/* Not really necessary, but helps to catch programming errors. */
data->new_cred = NULL;
@@ -2756,6 +2804,24 @@
*/
data->new_cred = new_cred;
data->setcred_flags = flags;
+ /* If a previous exec constraint was never consumed (exec didn't
+ * happen), drop the stale exec_conf reference. This is the same
+ * safety net as in setcred_enter — but this hook runs AFTER
+ * setcred_enter has already cleaned up, so in practice
+ * has_exec_constraint is always false here. We check anyway for
+ * defense in depth.
+ *
+ * This hook always runs before priv_grant, so resetting here
+ * guarantees each setcred call starts with a clean slate.
+ */
+ if (data->has_exec_constraint) {
+ drop_conf(data->exec_conf);
+ data->has_exec_constraint = false;
+ data->exec_blacklist = false;
+ }
+ data->exec_paths_nb = 0;
+ data->exec_paths = NULL;
+ data->exec_conf = NULL;
return (0);
}

File Metadata

Mime Type
text/plain
Expires
Sat, Jul 11, 3:49 PM (9 h, 33 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
34637364
Default Alt Text
D58014.id.diff (3 KB)

Event Timeline