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. Obtained from: TrustedBSD Project MFC after: 1 month Index: kern/vfs_syscalls.c =================================================================== --- kern/vfs_syscalls.c (revision 195897) +++ kern/vfs_syscalls.c (working copy) @@ -379,7 +379,7 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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); Index: kern/vfs_lookup.c =================================================================== --- kern/vfs_lookup.c (revision 195897) +++ kern/vfs_lookup.c (working copy) @@ -569,9 +574,9 @@ 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); @@ -854,9 +859,9 @@ 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); Index: kern/kern_exec.c =================================================================== --- kern/kern_exec.c (revision 195897) +++ kern/kern_exec.c (working copy) @@ -419,7 +419,7 @@ 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; } Index: security/audit/audit_arg.c =================================================================== --- security/audit/audit_arg.c (revision 195897) +++ security/audit/audit_arg.c (working copy) @@ -641,7 +667,7 @@ 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; @@ -735,18 +761,12 @@ * * 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 * non-MPSAFE vnode, then it will have acquired Giant. @@ -754,27 +774,10 @@ 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; @@ -784,9 +787,38 @@ 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); } @@ -859,7 +891,7 @@ 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); Index: security/audit/audit_private.h =================================================================== --- security/audit/audit_private.h (revision 195897) +++ security/audit/audit_private.h (working copy) @@ -238,6 +240,9 @@ #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. Index: security/audit/audit.h =================================================================== --- security/audit/audit.h (revision 195897) +++ security/audit/audit.h (working copy) @@ -159,7 +163,8 @@ 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); @@ -327,11 +342,16 @@ 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 { \ if (audit_enabled) { \ audit_syscall_enter(code, td); \ @@ -386,7 +408,8 @@ #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)