MAC/do: sysctl_rules(): Always copy the rules specification string

We are not guaranteed that the 'rules' storage stays stable if we don't
hold the prison lock.  For this reason, always copy the specification
string (under the lock).

Reviewed by:    bapt
Approved by:    markj (mentor)
Sponsored by:   The FreeBSD Foundation
Differential Revision:  https://reviews.freebsd.org/D47600

1 files changed, 6 insertions, 14 deletions

diff --git a/sys/security/mac_do/mac_do.c b/sys/security/mac_do/mac_do.c
index ed4c984ff559..94fe7b99fc9d 100644
--- a/sys/security/mac_do/mac_do.c
+++ b/sys/security/mac_do/mac_do.c
@@ -309,30 +309,22 @@ parse_and_set_rules(struct prison *const pr, const char *rules_string)
 static int
 sysctl_rules(SYSCTL_HANDLER_ARGS)
 {
-	char *new_string;
+	char *const buf = malloc(MAC_RULE_STRING_LEN, M_DO, M_WAITOK);
 	struct prison *pr;
 	struct rules *rules;
 	int error;
 
 	rules = find_rules(req->td->td_ucred->cr_prison, &pr);
+	strlcpy(buf, rules->string, MAC_RULE_STRING_LEN);
 	prison_unlock(pr);
-	if (req->newptr == NULL)
-		return (sysctl_handle_string(oidp, rules->string, MAC_RULE_STRING_LEN, req));
 
-	new_string = malloc(MAC_RULE_STRING_LEN, M_DO,
-	    M_WAITOK|M_ZERO);
-	prison_lock(pr);
-	strlcpy(new_string, rules->string, MAC_RULE_STRING_LEN);
-	prison_unlock(pr);
-
-	error = sysctl_handle_string(oidp, new_string, MAC_RULE_STRING_LEN, req);
-	if (error)
+	error = sysctl_handle_string(oidp, buf, MAC_RULE_STRING_LEN, req);
+	if (error != 0 || req->newptr == NULL)
 		goto out;
 
-	error = parse_and_set_rules(pr, new_string);
-
+	error = parse_and_set_rules(pr, buf);
 out:
-	free(new_string, M_DO);
+	free(buf, M_DO);
 	return (error);
 }