From fac30ba8b4fd00a95fd7fb49c4e4dc1bcdf4003c Mon Sep 17 00:00:00 2001 From: rwatson Date: Tue, 28 Jul 2009 21:52:24 +0000 Subject: [PATCH] Rework vnode argument auditing to follow the same structure, in order to avoid exposing ARG_ macros/flag values outside of the audit code in order to name which one of two possible vnodes will be audited for a system call. Approved by: re (kib) Obtained from: TrustedBSD Project MFC after: 1 month --- sys/kern/kern_exec.c | 2 +- sys/kern/vfs_lookup.c | 8 ++-- sys/kern/vfs_syscalls.c | 16 ++++---- sys/security/audit/audit.h | 15 +++++-- sys/security/audit/audit_arg.c | 66 ++++++++++++++++-------------- sys/security/audit/audit_private.h | 3 ++ 6 files changed, 63 insertions(+), 47 deletions(-) diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c index 3f366582905c..663ab649507f 100644 --- a/sys/kern/kern_exec.c +++ b/sys/kern/kern_exec.c @@ -419,7 +419,7 @@ do_execve(td, args, mac_p) goto exec_fail; vfslocked = VFS_LOCK_GIANT(binvp->v_mount); vn_lock(binvp, LK_EXCLUSIVE | LK_RETRY); - AUDIT_ARG_VNODE(binvp, ARG_VNODE1); + AUDIT_ARG_VNODE1(binvp); imgp->vp = binvp; } diff --git a/sys/kern/vfs_lookup.c b/sys/kern/vfs_lookup.c index 2f3b54e8f0ba..9fa6c9ef2aea 100644 --- a/sys/kern/vfs_lookup.c +++ b/sys/kern/vfs_lookup.c @@ -574,9 +574,9 @@ lookup(struct nameidata *ndp) ndp->ni_vp = dp; if (cnp->cn_flags & AUDITVNODE1) - AUDIT_ARG_VNODE(dp, ARG_VNODE1); + AUDIT_ARG_VNODE1(dp); else if (cnp->cn_flags & AUDITVNODE2) - AUDIT_ARG_VNODE(dp, ARG_VNODE2); + AUDIT_ARG_VNODE2(dp); if (!(cnp->cn_flags & (LOCKPARENT | LOCKLEAF))) VOP_UNLOCK(dp, 0); @@ -859,9 +859,9 @@ lookup(struct nameidata *ndp) VOP_UNLOCK(ndp->ni_dvp, 0); if (cnp->cn_flags & AUDITVNODE1) - AUDIT_ARG_VNODE(dp, ARG_VNODE1); + AUDIT_ARG_VNODE1(dp); else if (cnp->cn_flags & AUDITVNODE2) - AUDIT_ARG_VNODE(dp, ARG_VNODE2); + AUDIT_ARG_VNODE2(dp); if ((cnp->cn_flags & LOCKLEAF) == 0) VOP_UNLOCK(dp, 0); diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c index 28d95e2eb084..d24926b65aca 100644 --- a/sys/kern/vfs_syscalls.c +++ b/sys/kern/vfs_syscalls.c @@ -379,7 +379,7 @@ kern_fstatfs(struct thread *td, int fd, struct statfs *buf) vfslocked = VFS_LOCK_GIANT(vp->v_mount); vn_lock(vp, LK_SHARED | LK_RETRY); #ifdef AUDIT - AUDIT_ARG_VNODE(vp, ARG_VNODE1); + AUDIT_ARG_VNODE1(vp); #endif mp = vp->v_mount; if (mp) @@ -752,7 +752,7 @@ fchdir(td, uap) fdrop(fp, td); vfslocked = VFS_LOCK_GIANT(vp->v_mount); vn_lock(vp, LK_SHARED | LK_RETRY); - AUDIT_ARG_VNODE(vp, ARG_VNODE1); + AUDIT_ARG_VNODE1(vp); error = change_dir(vp, td); while (!error && (mp = vp->v_mountedhere) != NULL) { int tvfslocked; @@ -2779,7 +2779,7 @@ fchflags(td, uap) vfslocked = VFS_LOCK_GIANT(fp->f_vnode->v_mount); #ifdef AUDIT vn_lock(fp->f_vnode, LK_SHARED | LK_RETRY); - AUDIT_ARG_VNODE(fp->f_vnode, ARG_VNODE1); + AUDIT_ARG_VNODE1(fp->f_vnode); VOP_UNLOCK(fp->f_vnode, 0); #endif error = setfflags(td, fp->f_vnode, uap->flags); @@ -2940,7 +2940,7 @@ fchmod(td, uap) vfslocked = VFS_LOCK_GIANT(fp->f_vnode->v_mount); #ifdef AUDIT vn_lock(fp->f_vnode, LK_SHARED | LK_RETRY); - AUDIT_ARG_VNODE(fp->f_vnode, ARG_VNODE1); + AUDIT_ARG_VNODE1(fp->f_vnode); VOP_UNLOCK(fp->f_vnode, 0); #endif error = setfmode(td, fp->f_vnode, uap->mode); @@ -3117,7 +3117,7 @@ fchown(td, uap) vfslocked = VFS_LOCK_GIANT(fp->f_vnode->v_mount); #ifdef AUDIT vn_lock(fp->f_vnode, LK_SHARED | LK_RETRY); - AUDIT_ARG_VNODE(fp->f_vnode, ARG_VNODE1); + AUDIT_ARG_VNODE1(fp->f_vnode); VOP_UNLOCK(fp->f_vnode, 0); #endif error = setfown(td, fp->f_vnode, uap->uid, uap->gid); @@ -3353,7 +3353,7 @@ kern_futimes(struct thread *td, int fd, struct timeval *tptr, vfslocked = VFS_LOCK_GIANT(fp->f_vnode->v_mount); #ifdef AUDIT vn_lock(fp->f_vnode, LK_SHARED | LK_RETRY); - AUDIT_ARG_VNODE(fp->f_vnode, ARG_VNODE1); + AUDIT_ARG_VNODE1(fp->f_vnode); VOP_UNLOCK(fp->f_vnode, 0); #endif error = setutimes(td, fp->f_vnode, ts, 2, tptr == NULL); @@ -3513,7 +3513,7 @@ fsync(td, uap) lock_flags = LK_EXCLUSIVE; } vn_lock(vp, lock_flags | LK_RETRY); - AUDIT_ARG_VNODE(vp, ARG_VNODE1); + AUDIT_ARG_VNODE1(vp); if (vp->v_object != NULL) { VM_OBJECT_LOCK(vp->v_object); vm_object_page_clean(vp->v_object, 0, 0, 0); @@ -4103,7 +4103,7 @@ kern_getdirentries(struct thread *td, int fd, char *buf, u_int count, auio.uio_td = td; auio.uio_resid = count; vn_lock(vp, LK_SHARED | LK_RETRY); - AUDIT_ARG_VNODE(vp, ARG_VNODE1); + AUDIT_ARG_VNODE1(vp); loff = auio.uio_offset = fp->f_offset; #ifdef MAC error = mac_vnode_check_readdir(td->td_ucred, vp); diff --git a/sys/security/audit/audit.h b/sys/security/audit/audit.h index e8b355030277..812199609eff 100644 --- a/sys/security/audit/audit.h +++ b/sys/security/audit/audit.h @@ -163,7 +163,8 @@ void audit_arg_auid(uid_t auid); void audit_arg_auditinfo(struct auditinfo *au_info); void audit_arg_auditinfo_addr(struct auditinfo_addr *au_info); void audit_arg_upath(struct thread *td, char *upath, u_int64_t flags); -void audit_arg_vnode(struct vnode *vp, u_int64_t flags); +void audit_arg_vnode1(struct vnode *vp); +void audit_arg_vnode2(struct vnode *vp); void audit_arg_text(char *text); void audit_arg_cmd(int cmd); void audit_arg_svipc_cmd(int cmd); @@ -341,9 +342,14 @@ void audit_thread_free(struct thread *td); audit_arg_value((value)); \ } while (0) -#define AUDIT_ARG_VNODE(vp, flags) do { \ +#define AUDIT_ARG_VNODE1(vp) do { \ if (AUDITING_TD(curthread)) \ - audit_arg_vnode((vp), (flags)); \ + audit_arg_vnode1((vp)); \ +} while (0) + +#define AUDIT_ARG_VNODE2(vp) do { \ + if (AUDITING_TD(curthread)) \ + audit_arg_vnode2((vp)); \ } while (0) #define AUDIT_SYSCALL_ENTER(code, td) do { \ @@ -402,7 +408,8 @@ void audit_thread_free(struct thread *td); #define AUDIT_ARG_UID(uid) #define AUDIT_ARG_UPATH(td, upath, flags) #define AUDIT_ARG_VALUE(value) -#define AUDIT_ARG_VNODE(vp, flags) +#define AUDIT_ARG_VNODE1(vp) +#define AUDIT_ARG_VNODE2(vp) #define AUDIT_SYSCALL_ENTER(code, td) #define AUDIT_SYSCALL_EXIT(error, td) diff --git a/sys/security/audit/audit_arg.c b/sys/security/audit/audit_arg.c index bf4cb0afd7aa..cd3abb273658 100644 --- a/sys/security/audit/audit_arg.c +++ b/sys/security/audit/audit_arg.c @@ -667,7 +667,7 @@ audit_arg_file(struct proc *p, struct file *fp) vp = fp->f_vnode; vfslocked = VFS_LOCK_GIANT(vp->v_mount); vn_lock(vp, LK_SHARED | LK_RETRY); - audit_arg_vnode(vp, ARG_VNODE1); + audit_arg_vnode1(vp); VOP_UNLOCK(vp, 0); VFS_UNLOCK_GIANT(vfslocked); break; @@ -761,17 +761,11 @@ audit_arg_upath(struct thread *td, char *upath, u_int64_t flag) * * XXXAUDIT: Possibly KASSERT the path pointer is NULL? */ -void -audit_arg_vnode(struct vnode *vp, u_int64_t flags) +static int +audit_arg_vnode(struct vnode *vp, struct vnode_au_info *vnp) { - struct kaudit_record *ar; struct vattr vattr; int error; - struct vnode_au_info *vnp; - - KASSERT(vp != NULL, ("audit_arg_vnode: vp == NULL")); - KASSERT((flags == ARG_VNODE1) || (flags == ARG_VNODE2), - ("audit_arg_vnode: flags %jd", (intmax_t)flags)); /* * Assume that if the caller is calling audit_arg_vnode() on a @@ -780,27 +774,10 @@ audit_arg_vnode(struct vnode *vp, u_int64_t flags) VFS_ASSERT_GIANT(vp->v_mount); ASSERT_VOP_LOCKED(vp, "audit_arg_vnode"); - ar = currecord(); - if (ar == NULL) - return; - - /* - * XXXAUDIT: The below clears, and then resets the flags for valid - * arguments. Ideally, either the new vnode is used, or the old one - * would be. - */ - if (flags & ARG_VNODE1) { - ar->k_ar.ar_valid_arg &= (ARG_ALL ^ ARG_VNODE1); - vnp = &ar->k_ar.ar_arg_vnode1; - } else { - ar->k_ar.ar_valid_arg &= (ARG_ALL ^ ARG_VNODE2); - vnp = &ar->k_ar.ar_arg_vnode2; - } - error = VOP_GETATTR(vp, &vattr, curthread->td_ucred); if (error) { /* XXX: How to handle this case? */ - return; + return (error); } vnp->vn_mode = vattr.va_mode; @@ -810,9 +787,38 @@ audit_arg_vnode(struct vnode *vp, u_int64_t flags) vnp->vn_fsid = vattr.va_fsid; vnp->vn_fileid = vattr.va_fileid; vnp->vn_gen = vattr.va_gen; - if (flags & ARG_VNODE1) + return (0); +} + +void +audit_arg_vnode1(struct vnode *vp) +{ + struct kaudit_record *ar; + int error; + + ar = currecord(); + if (ar == NULL) + return; + + ARG_CLEAR_VALID(ar, ARG_VNODE1); + error = audit_arg_vnode(vp, &ar->k_ar.ar_arg_vnode1); + if (error == 0) ARG_SET_VALID(ar, ARG_VNODE1); - else +} + +void +audit_arg_vnode2(struct vnode *vp) +{ + struct kaudit_record *ar; + int error; + + ar = currecord(); + if (ar == NULL) + return; + + ARG_CLEAR_VALID(ar, ARG_VNODE2); + error = audit_arg_vnode(vp, &ar->k_ar.ar_arg_vnode2); + if (error == 0) ARG_SET_VALID(ar, ARG_VNODE2); } @@ -885,7 +891,7 @@ audit_sysclose(struct thread *td, int fd) vp = fp->f_vnode; vfslocked = VFS_LOCK_GIANT(vp->v_mount); vn_lock(vp, LK_SHARED | LK_RETRY); - audit_arg_vnode(vp, ARG_VNODE1); + audit_arg_vnode1(vp); VOP_UNLOCK(vp, 0); VFS_UNLOCK_GIANT(vfslocked); fdrop(fp, td); diff --git a/sys/security/audit/audit_private.h b/sys/security/audit/audit_private.h index 97433df5f5ee..ab6c1f478276 100644 --- a/sys/security/audit/audit_private.h +++ b/sys/security/audit/audit_private.h @@ -240,6 +240,9 @@ struct audit_record { #define ARG_SET_VALID(kar, arg) do { \ (kar)->k_ar.ar_valid_arg |= (arg); \ } while (0) +#define ARG_CLEAR_VALID(kar, arg) do { \ + (kar)->k_ar.ar_valid_arg &= ~(arg); \ +} while (0) /* * In-kernel version of audit record; the basic record plus queue meta-data.