git: 2e593dd3b5e1 - main - MAC: syscalls: Factor out common label copy-in code
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 16 Dec 2024 14:45:16 UTC
The branch main has been updated by olce:
URL: https://cgit.FreeBSD.org/src/commit/?id=2e593dd3b5e1c515d57b3d3f929e544a6622b04a
commit 2e593dd3b5e1c515d57b3d3f929e544a6622b04a
Author: Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2024-07-26 14:40:22 +0000
Commit: Olivier Certner <olce@FreeBSD.org>
CommitDate: 2024-12-16 14:42:28 +0000
MAC: syscalls: Factor out common label copy-in code
Besides simplifying existing code, this will later enable the new
setcred() system call to copy MAC labels.
MFC after: 2 weeks
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46904
---
sys/security/mac/mac_syscalls.c | 201 +++++++++++++++++-----------------------
1 file changed, 83 insertions(+), 118 deletions(-)
diff --git a/sys/security/mac/mac_syscalls.c b/sys/security/mac/mac_syscalls.c
index 56aaba935442..74db8625114e 100644
--- a/sys/security/mac/mac_syscalls.c
+++ b/sys/security/mac/mac_syscalls.c
@@ -78,26 +78,67 @@ static int kern___mac_get_path(struct thread *td, const char *path_p,
static int kern___mac_set_path(struct thread *td, const char *path_p,
struct mac *mac_p, int follow);
+/*
+ * Copyin a 'struct mac', including the string pointed to by 'm_string'.
+ *
+ * On success (0 returned), fills '*mac', whose associated storage must be freed
+ * after use by calling free_copied_label() (which see). On success, 'u_string'
+ * if not NULL is filled with the userspace address for 'u_mac->m_string'.
+ */
+static int
+mac_label_copyin(const struct mac *const u_mac, struct mac *const mac,
+ char **const u_string)
+{
+ char *buffer;
+ int error;
+
+ error = copyin(u_mac, mac, sizeof(*mac));
+ if (error != 0)
+ return (error);
+
+ error = mac_check_structmac_consistent(mac);
+ if (error != 0)
+ return (error);
+
+ /* 'm_buflen' not too big checked by function call above. */
+ buffer = malloc(mac->m_buflen, M_MACTEMP, M_WAITOK);
+ error = copyinstr(mac->m_string, buffer, mac->m_buflen, NULL);
+ if (error != 0) {
+ free(buffer, M_MACTEMP);
+ return (error);
+ }
+
+ MPASS(error == 0);
+ if (u_string != NULL)
+ *u_string = mac->m_string;
+ mac->m_string = buffer;
+ return (0);
+}
+
+static void
+free_copied_label(const struct mac *const mac)
+{
+ free(mac->m_string, M_MACTEMP);
+}
+
int
sys___mac_get_pid(struct thread *td, struct __mac_get_pid_args *uap)
{
- char *elements, *buffer;
+ char *buffer, *u_buffer;
struct mac mac;
struct proc *tproc;
struct ucred *tcred;
int error;
- error = copyin(uap->mac_p, &mac, sizeof(mac));
- if (error)
- return (error);
-
- error = mac_check_structmac_consistent(&mac);
+ error = mac_label_copyin(uap->mac_p, &mac, &u_buffer);
if (error)
return (error);
tproc = pfind(uap->pid);
- if (tproc == NULL)
- return (ESRCH);
+ if (tproc == NULL) {
+ error = ESRCH;
+ goto free_mac_and_exit;
+ }
tcred = NULL; /* Satisfy gcc. */
error = p_cansee(td, tproc);
@@ -105,58 +146,40 @@ sys___mac_get_pid(struct thread *td, struct __mac_get_pid_args *uap)
tcred = crhold(tproc->p_ucred);
PROC_UNLOCK(tproc);
if (error)
- return (error);
-
- elements = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK);
- error = copyinstr(mac.m_string, elements, mac.m_buflen, NULL);
- if (error) {
- free(elements, M_MACTEMP);
- crfree(tcred);
- return (error);
- }
+ goto free_mac_and_exit;
buffer = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK | M_ZERO);
- error = mac_cred_externalize_label(tcred->cr_label, elements,
+ error = mac_cred_externalize_label(tcred->cr_label, mac.m_string,
buffer, mac.m_buflen);
if (error == 0)
- error = copyout(buffer, mac.m_string, strlen(buffer)+1);
-
+ error = copyout(buffer, u_buffer, strlen(buffer)+1);
free(buffer, M_MACTEMP);
- free(elements, M_MACTEMP);
crfree(tcred);
+
+free_mac_and_exit:
+ free_copied_label(&mac);
return (error);
}
int
sys___mac_get_proc(struct thread *td, struct __mac_get_proc_args *uap)
{
- char *elements, *buffer;
+ char *buffer, *u_buffer;
struct mac mac;
int error;
- error = copyin(uap->mac_p, &mac, sizeof(mac));
+ error = mac_label_copyin(uap->mac_p, &mac, &u_buffer);
if (error)
return (error);
- error = mac_check_structmac_consistent(&mac);
- if (error)
- return (error);
-
- elements = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK);
- error = copyinstr(mac.m_string, elements, mac.m_buflen, NULL);
- if (error) {
- free(elements, M_MACTEMP);
- return (error);
- }
-
buffer = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK | M_ZERO);
error = mac_cred_externalize_label(td->td_ucred->cr_label,
- elements, buffer, mac.m_buflen);
+ mac.m_string, buffer, mac.m_buflen);
if (error == 0)
- error = copyout(buffer, mac.m_string, strlen(buffer)+1);
+ error = copyout(buffer, u_buffer, strlen(buffer)+1);
free(buffer, M_MACTEMP);
- free(elements, M_MACTEMP);
+ free_copied_label(&mac);
return (error);
}
@@ -167,30 +190,18 @@ sys___mac_set_proc(struct thread *td, struct __mac_set_proc_args *uap)
struct label *intlabel;
struct proc *p;
struct mac mac;
- char *buffer;
int error;
if (!(mac_labeled & MPC_OBJECT_CRED))
return (EINVAL);
- error = copyin(uap->mac_p, &mac, sizeof(mac));
- if (error)
- return (error);
-
- error = mac_check_structmac_consistent(&mac);
+ error = mac_label_copyin(uap->mac_p, &mac, NULL);
if (error)
return (error);
- buffer = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK);
- error = copyinstr(mac.m_string, buffer, mac.m_buflen, NULL);
- if (error) {
- free(buffer, M_MACTEMP);
- return (error);
- }
-
intlabel = mac_cred_label_alloc();
- error = mac_cred_internalize_label(intlabel, buffer);
- free(buffer, M_MACTEMP);
+ error = mac_cred_internalize_label(intlabel, mac.m_string);
+ free_copied_label(&mac);
if (error)
goto out;
@@ -224,7 +235,7 @@ out:
int
sys___mac_get_fd(struct thread *td, struct __mac_get_fd_args *uap)
{
- char *elements, *buffer;
+ char *u_buffer, *buffer;
struct label *intlabel;
struct file *fp;
struct mac mac;
@@ -234,21 +245,10 @@ sys___mac_get_fd(struct thread *td, struct __mac_get_fd_args *uap)
cap_rights_t rights;
int error;
- error = copyin(uap->mac_p, &mac, sizeof(mac));
+ error = mac_label_copyin(uap->mac_p, &mac, &u_buffer);
if (error)
return (error);
- error = mac_check_structmac_consistent(&mac);
- if (error)
- return (error);
-
- elements = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK);
- error = copyinstr(mac.m_string, elements, mac.m_buflen, NULL);
- if (error) {
- free(elements, M_MACTEMP);
- return (error);
- }
-
buffer = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK | M_ZERO);
error = fget(td, uap->fd, cap_rights_init_one(&rights, CAP_MAC_GET),
&fp);
@@ -267,7 +267,7 @@ sys___mac_get_fd(struct thread *td, struct __mac_get_fd_args *uap)
vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
mac_vnode_copy_label(vp->v_label, intlabel);
VOP_UNLOCK(vp);
- error = mac_vnode_externalize_label(intlabel, elements,
+ error = mac_vnode_externalize_label(intlabel, mac.m_string,
buffer, mac.m_buflen);
mac_vnode_label_free(intlabel);
break;
@@ -282,7 +282,7 @@ sys___mac_get_fd(struct thread *td, struct __mac_get_fd_args *uap)
PIPE_LOCK(pipe);
mac_pipe_copy_label(pipe->pipe_pair->pp_label, intlabel);
PIPE_UNLOCK(pipe);
- error = mac_pipe_externalize_label(intlabel, elements,
+ error = mac_pipe_externalize_label(intlabel, mac.m_string,
buffer, mac.m_buflen);
mac_pipe_label_free(intlabel);
break;
@@ -297,7 +297,7 @@ sys___mac_get_fd(struct thread *td, struct __mac_get_fd_args *uap)
SOCK_LOCK(so);
mac_socket_copy_label(so->so_label, intlabel);
SOCK_UNLOCK(so);
- error = mac_socket_externalize_label(intlabel, elements,
+ error = mac_socket_externalize_label(intlabel, mac.m_string,
buffer, mac.m_buflen);
mac_socket_label_free(intlabel);
break;
@@ -306,12 +306,12 @@ sys___mac_get_fd(struct thread *td, struct __mac_get_fd_args *uap)
error = EINVAL;
}
if (error == 0)
- error = copyout(buffer, mac.m_string, strlen(buffer)+1);
+ error = copyout(buffer, u_buffer, strlen(buffer)+1);
out_fdrop:
fdrop(fp, td);
out:
free(buffer, M_MACTEMP);
- free(elements, M_MACTEMP);
+ free_copied_label(&mac);
return (error);
}
@@ -333,7 +333,7 @@ static int
kern___mac_get_path(struct thread *td, const char *path_p, struct mac *mac_p,
int follow)
{
- char *elements, *buffer;
+ char *u_buffer, *buffer;
struct nameidata nd;
struct label *intlabel;
struct mac mac;
@@ -342,21 +342,10 @@ kern___mac_get_path(struct thread *td, const char *path_p, struct mac *mac_p,
if (!(mac_labeled & MPC_OBJECT_VNODE))
return (EINVAL);
- error = copyin(mac_p, &mac, sizeof(mac));
- if (error)
- return (error);
-
- error = mac_check_structmac_consistent(&mac);
+ error = mac_label_copyin(mac_p, &mac, &u_buffer);
if (error)
return (error);
- elements = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK);
- error = copyinstr(mac.m_string, elements, mac.m_buflen, NULL);
- if (error) {
- free(elements, M_MACTEMP);
- return (error);
- }
-
buffer = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK | M_ZERO);
NDINIT(&nd, LOOKUP, LOCKLEAF | follow, UIO_USERSPACE, path_p);
error = namei(&nd);
@@ -365,18 +354,18 @@ kern___mac_get_path(struct thread *td, const char *path_p, struct mac *mac_p,
intlabel = mac_vnode_label_alloc();
mac_vnode_copy_label(nd.ni_vp->v_label, intlabel);
- error = mac_vnode_externalize_label(intlabel, elements, buffer,
+ error = mac_vnode_externalize_label(intlabel, mac.m_string, buffer,
mac.m_buflen);
vput(nd.ni_vp);
NDFREE_PNBUF(&nd);
mac_vnode_label_free(intlabel);
if (error == 0)
- error = copyout(buffer, mac.m_string, strlen(buffer)+1);
+ error = copyout(buffer, u_buffer, strlen(buffer)+1);
out:
free(buffer, M_MACTEMP);
- free(elements, M_MACTEMP);
+ free_copied_label(&mac);
return (error);
}
@@ -392,24 +381,12 @@ sys___mac_set_fd(struct thread *td, struct __mac_set_fd_args *uap)
struct vnode *vp;
struct mac mac;
cap_rights_t rights;
- char *buffer;
int error;
- error = copyin(uap->mac_p, &mac, sizeof(mac));
+ error = mac_label_copyin(uap->mac_p, &mac, NULL);
if (error)
return (error);
- error = mac_check_structmac_consistent(&mac);
- if (error)
- return (error);
-
- buffer = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK);
- error = copyinstr(mac.m_string, buffer, mac.m_buflen, NULL);
- if (error) {
- free(buffer, M_MACTEMP);
- return (error);
- }
-
error = fget(td, uap->fd, cap_rights_init_one(&rights, CAP_MAC_SET),
&fp);
if (error)
@@ -423,7 +400,7 @@ sys___mac_set_fd(struct thread *td, struct __mac_set_fd_args *uap)
goto out_fdrop;
}
intlabel = mac_vnode_label_alloc();
- error = mac_vnode_internalize_label(intlabel, buffer);
+ error = mac_vnode_internalize_label(intlabel, mac.m_string);
if (error) {
mac_vnode_label_free(intlabel);
break;
@@ -447,7 +424,7 @@ sys___mac_set_fd(struct thread *td, struct __mac_set_fd_args *uap)
goto out_fdrop;
}
intlabel = mac_pipe_label_alloc();
- error = mac_pipe_internalize_label(intlabel, buffer);
+ error = mac_pipe_internalize_label(intlabel, mac.m_string);
if (error == 0) {
pipe = fp->f_data;
PIPE_LOCK(pipe);
@@ -464,7 +441,7 @@ sys___mac_set_fd(struct thread *td, struct __mac_set_fd_args *uap)
goto out_fdrop;
}
intlabel = mac_socket_label_alloc(M_WAITOK);
- error = mac_socket_internalize_label(intlabel, buffer);
+ error = mac_socket_internalize_label(intlabel, mac.m_string);
if (error == 0) {
so = fp->f_data;
error = mac_socket_label_set(td->td_ucred, so,
@@ -479,7 +456,7 @@ sys___mac_set_fd(struct thread *td, struct __mac_set_fd_args *uap)
out_fdrop:
fdrop(fp, td);
out:
- free(buffer, M_MACTEMP);
+ free_copied_label(&mac);
return (error);
}
@@ -505,30 +482,18 @@ kern___mac_set_path(struct thread *td, const char *path_p, struct mac *mac_p,
struct nameidata nd;
struct mount *mp;
struct mac mac;
- char *buffer;
int error;
if (!(mac_labeled & MPC_OBJECT_VNODE))
return (EINVAL);
- error = copyin(mac_p, &mac, sizeof(mac));
+ error = mac_label_copyin(mac_p, &mac, NULL);
if (error)
return (error);
- error = mac_check_structmac_consistent(&mac);
- if (error)
- return (error);
-
- buffer = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK);
- error = copyinstr(mac.m_string, buffer, mac.m_buflen, NULL);
- if (error) {
- free(buffer, M_MACTEMP);
- return (error);
- }
-
intlabel = mac_vnode_label_alloc();
- error = mac_vnode_internalize_label(intlabel, buffer);
- free(buffer, M_MACTEMP);
+ error = mac_vnode_internalize_label(intlabel, mac.m_string);
+ free_copied_label(&mac);
if (error)
goto out;