Date: Sun, 16 Dec 2012 23:41:34 +0000 (UTC) From: Pawel Jakub Dawidek <pjd@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r244324 - in stable/9/sys: kern security/audit Message-ID: <201212162341.qBGNfYem074402@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: pjd Date: Sun Dec 16 23:41:34 2012 New Revision: 244324 URL: http://svnweb.freebsd.org/changeset/base/244324 Log: MFC r243719,r243720,r243722,r243723,r243726,r243727,r243746: r243719: IFp4 @208450: Remove redundant call to AUDIT_ARG_UPATH1(). Path will be remembered by the following NDINIT(AUDITVNODE1) call. Sponsored by: The FreeBSD Foundation (auditdistd) r243720: IFp4 @208381: For VOP_GETATTR() we just need vnode to be shared-locked. Sponsored by: The FreeBSD Foundation (auditdistd) r243722: IFp4 @208382: Currently on each record write we call VFS_STATFS() to get available space on the file system as well as VOP_GETATTR() to get trail file size. We can assume that trail file is only updated by the audit worker, so instead of asking for file size on every write, get file size on trail switch only (it should be zero, but it's not expensive) and use global variable audit_size protected by the audit worker lock to keep track of trail file's size. This eliminates VOP_GETATTR() call for every write. VFS_STATFS() is satisfied from in-memory data (mount->mnt_stat), so shouldn't be expensive. Sponsored by: The FreeBSD Foundation (auditdistd) r243723: IFp4 @208383: Currently when we discover that trail file is greater than configured limit we send AUDIT_TRIGGER_ROTATE_KERNEL trigger to the auditd daemon once. If for some reason auditd didn't rotate trail file it will never be rotated. Change it by sending the trigger when trail file size grows by the configured limit. For example if the limit is 1MB, we will send trigger on 1MB, 2MB, 3MB, etc. This is also needed for the auditd change that will be committed soon where auditd may ignore the trigger - it might be ignored if kernel requests the trail file to be rotated too quickly (often than once a second) which would result in overwriting previous trail file. Sponsored by: The FreeBSD Foundation (auditdistd) r243726: IFp4 @208451: Fix path handling for *at() syscalls. Before the change directory descriptor was totally ignored, so the relative path argument was appended to current working directory path and not to the path provided by descriptor, thus wrong paths were stored in audit logs. Now that we use directory descriptor in vfs_lookup, move AUDIT_ARG_UPATH1() and AUDIT_ARG_UPATH2() calls to the place where we hold file descriptors table lock, so we are sure paths will be resolved according to the same directory in audit record and in actual operation. Sponsored by: The FreeBSD Foundation (auditdistd) Reviewed by: rwatson r243727: IFp4 @208452: Audit handling for missing events: - AUE_READLINKAT - AUE_FACCESSAT - AUE_MKDIRAT - AUE_MKFIFOAT - AUE_MKNODAT - AUE_SYMLINKAT Sponsored by: FreeBSD Foundation (auditdistd) r243746: Fix one more compilation issue. Sponsored by: FreeBSD Foundation (auditdistd) Modified: stable/9/sys/kern/vfs_lookup.c stable/9/sys/kern/vfs_mount.c stable/9/sys/security/audit/audit.c stable/9/sys/security/audit/audit.h stable/9/sys/security/audit/audit_arg.c stable/9/sys/security/audit/audit_bsm.c stable/9/sys/security/audit/audit_bsm_klib.c stable/9/sys/security/audit/audit_private.h stable/9/sys/security/audit/audit_worker.c Directory Properties: stable/9/sys/ (props changed) Modified: stable/9/sys/kern/vfs_lookup.c ============================================================================== --- stable/9/sys/kern/vfs_lookup.c Sun Dec 16 23:29:56 2012 (r244323) +++ stable/9/sys/kern/vfs_lookup.c Sun Dec 16 23:41:34 2012 (r244324) @@ -163,17 +163,6 @@ namei(struct nameidata *ndp) error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN, (size_t *)&ndp->ni_pathlen); - if (error == 0) { - /* - * If we are auditing the kernel pathname, save the user - * pathname. - */ - if (cnp->cn_flags & AUDITVNODE1) - AUDIT_ARG_UPATH1(td, cnp->cn_pnbuf); - if (cnp->cn_flags & AUDITVNODE2) - AUDIT_ARG_UPATH2(td, cnp->cn_pnbuf); - } - /* * Don't allow empty pathnames. */ @@ -216,6 +205,14 @@ namei(struct nameidata *ndp) ndp->ni_rootdir = fdp->fd_rdir; ndp->ni_topdir = fdp->fd_jdir; + /* + * If we are auditing the kernel pathname, save the user pathname. + */ + if (cnp->cn_flags & AUDITVNODE1) + AUDIT_ARG_UPATH1(td, ndp->ni_dirfd, cnp->cn_pnbuf); + if (cnp->cn_flags & AUDITVNODE2) + AUDIT_ARG_UPATH2(td, ndp->ni_dirfd, cnp->cn_pnbuf); + dp = NULL; if (cnp->cn_pnbuf[0] != '/') { if (ndp->ni_startdir != NULL) { Modified: stable/9/sys/kern/vfs_mount.c ============================================================================== --- stable/9/sys/kern/vfs_mount.c Sun Dec 16 23:29:56 2012 (r244323) +++ stable/9/sys/kern/vfs_mount.c Sun Dec 16 23:41:34 2012 (r244324) @@ -1169,7 +1169,6 @@ sys_unmount(td, uap) } mtx_unlock(&mountlist_mtx); } else { - AUDIT_ARG_UPATH1(td, pathbuf); /* * Try to find global path for path argument. */ Modified: stable/9/sys/security/audit/audit.c ============================================================================== --- stable/9/sys/security/audit/audit.c Sun Dec 16 23:29:56 2012 (r244323) +++ stable/9/sys/security/audit/audit.c Sun Dec 16 23:41:34 2012 (r244324) @@ -691,7 +691,7 @@ audit_proc_coredump(struct thread *td, c if (path != NULL) { pathp = &ar->k_ar.ar_arg_upath1; *pathp = malloc(MAXPATHLEN, M_AUDITPATH, M_WAITOK); - audit_canon_path(td, path, *pathp); + audit_canon_path(td, AT_FDCWD, path, *pathp); ARG_SET_VALID(ar, ARG_UPATH1); } ar->k_ar.ar_arg_signum = td->td_proc->p_sig; Modified: stable/9/sys/security/audit/audit.h ============================================================================== --- stable/9/sys/security/audit/audit.h Sun Dec 16 23:29:56 2012 (r244323) +++ stable/9/sys/security/audit/audit.h Sun Dec 16 23:41:34 2012 (r244324) @@ -99,8 +99,8 @@ void audit_arg_sockaddr(struct thread * 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_upath1(struct thread *td, char *upath); -void audit_arg_upath2(struct thread *td, char *upath); +void audit_arg_upath1(struct thread *td, int dirfd, char *upath); +void audit_arg_upath2(struct thread *td, int dirfd, char *upath); void audit_arg_vnode1(struct vnode *vp); void audit_arg_vnode2(struct vnode *vp); void audit_arg_text(char *text); @@ -276,14 +276,14 @@ void audit_thread_free(struct thread *t audit_arg_uid((uid)); \ } while (0) -#define AUDIT_ARG_UPATH1(td, upath) do { \ +#define AUDIT_ARG_UPATH1(td, dirfd, upath) do { \ if (AUDITING_TD(curthread)) \ - audit_arg_upath1((td), (upath)); \ + audit_arg_upath1((td), (dirfd), (upath)); \ } while (0) -#define AUDIT_ARG_UPATH2(td, upath) do { \ +#define AUDIT_ARG_UPATH2(td, dirfd, upath) do { \ if (AUDITING_TD(curthread)) \ - audit_arg_upath2((td), (upath)); \ + audit_arg_upath2((td), (dirfd), (upath)); \ } while (0) #define AUDIT_ARG_VALUE(value) do { \ @@ -356,8 +356,8 @@ void audit_thread_free(struct thread *t #define AUDIT_ARG_SUID(suid) #define AUDIT_ARG_TEXT(text) #define AUDIT_ARG_UID(uid) -#define AUDIT_ARG_UPATH1(td, upath) -#define AUDIT_ARG_UPATH2(td, upath) +#define AUDIT_ARG_UPATH1(td, dirfd, upath) +#define AUDIT_ARG_UPATH2(td, dirfd, upath) #define AUDIT_ARG_VALUE(value) #define AUDIT_ARG_VNODE1(vp) #define AUDIT_ARG_VNODE2(vp) Modified: stable/9/sys/security/audit/audit_arg.c ============================================================================== --- stable/9/sys/security/audit/audit_arg.c Sun Dec 16 23:29:56 2012 (r244323) +++ stable/9/sys/security/audit/audit_arg.c Sun Dec 16 23:41:34 2012 (r244324) @@ -463,7 +463,8 @@ audit_arg_sockaddr(struct thread *td, st break; case AF_UNIX: - audit_arg_upath1(td, ((struct sockaddr_un *)sa)->sun_path); + audit_arg_upath1(td, AT_FDCWD, + ((struct sockaddr_un *)sa)->sun_path); ARG_SET_VALID(ar, ARG_SADDRUNIX); break; /* XXXAUDIT: default:? */ @@ -710,16 +711,16 @@ audit_arg_file(struct proc *p, struct fi * freed when the audit record is freed. */ static void -audit_arg_upath(struct thread *td, char *upath, char **pathp) +audit_arg_upath(struct thread *td, int dirfd, char *upath, char **pathp) { if (*pathp == NULL) *pathp = malloc(MAXPATHLEN, M_AUDITPATH, M_WAITOK); - audit_canon_path(td, upath, *pathp); + audit_canon_path(td, dirfd, upath, *pathp); } void -audit_arg_upath1(struct thread *td, char *upath) +audit_arg_upath1(struct thread *td, int dirfd, char *upath) { struct kaudit_record *ar; @@ -727,12 +728,12 @@ audit_arg_upath1(struct thread *td, char if (ar == NULL) return; - audit_arg_upath(td, upath, &ar->k_ar.ar_arg_upath1); + audit_arg_upath(td, dirfd, upath, &ar->k_ar.ar_arg_upath1); ARG_SET_VALID(ar, ARG_UPATH1); } void -audit_arg_upath2(struct thread *td, char *upath) +audit_arg_upath2(struct thread *td, int dirfd, char *upath) { struct kaudit_record *ar; @@ -740,7 +741,7 @@ audit_arg_upath2(struct thread *td, char if (ar == NULL) return; - audit_arg_upath(td, upath, &ar->k_ar.ar_arg_upath2); + audit_arg_upath(td, dirfd, upath, &ar->k_ar.ar_arg_upath2); ARG_SET_VALID(ar, ARG_UPATH2); } Modified: stable/9/sys/security/audit/audit_bsm.c ============================================================================== --- stable/9/sys/security/audit/audit_bsm.c Sun Dec 16 23:29:56 2012 (r244323) +++ stable/9/sys/security/audit/audit_bsm.c Sun Dec 16 23:41:34 2012 (r244324) @@ -724,13 +724,6 @@ kaudit_to_bsm(struct kaudit_record *kar, */ break; - case AUE_MKFIFO: - if (ARG_IS_VALID(kar, ARG_MODE)) { - tok = au_to_arg32(2, "mode", ar->ar_arg_mode); - kau_write(rec, tok); - } - /* FALLTHROUGH */ - case AUE_CHDIR: case AUE_CHROOT: case AUE_FSTATAT: @@ -743,6 +736,7 @@ kaudit_to_bsm(struct kaudit_record *kar, case AUE_LPATHCONF: case AUE_PATHCONF: case AUE_READLINK: + case AUE_READLINKAT: case AUE_REVOKE: case AUE_RMDIR: case AUE_SEARCHFS: @@ -762,6 +756,8 @@ kaudit_to_bsm(struct kaudit_record *kar, case AUE_ACCESS: case AUE_EACCESS: + case AUE_FACCESSAT: + ATFD1_TOKENS(1); UPATH1_VNODE1_TOKENS; if (ARG_IS_VALID(kar, ARG_VALUE)) { tok = au_to_arg32(2, "mode", ar->ar_arg_value); @@ -1059,6 +1055,10 @@ kaudit_to_bsm(struct kaudit_record *kar, break; case AUE_MKDIR: + case AUE_MKDIRAT: + case AUE_MKFIFO: + case AUE_MKFIFOAT: + ATFD1_TOKENS(1); if (ARG_IS_VALID(kar, ARG_MODE)) { tok = au_to_arg32(2, "mode", ar->ar_arg_mode); kau_write(rec, tok); @@ -1067,6 +1067,8 @@ kaudit_to_bsm(struct kaudit_record *kar, break; case AUE_MKNOD: + case AUE_MKNODAT: + ATFD1_TOKENS(1); if (ARG_IS_VALID(kar, ARG_MODE)) { tok = au_to_arg32(2, "mode", ar->ar_arg_mode); kau_write(rec, tok); @@ -1546,10 +1548,12 @@ kaudit_to_bsm(struct kaudit_record *kar, break; case AUE_SYMLINK: + case AUE_SYMLINKAT: if (ARG_IS_VALID(kar, ARG_TEXT)) { tok = au_to_text(ar->ar_arg_text); kau_write(rec, tok); } + ATFD1_TOKENS(1); UPATH1_VNODE1_TOKENS; break; Modified: stable/9/sys/security/audit/audit_bsm_klib.c ============================================================================== --- stable/9/sys/security/audit/audit_bsm_klib.c Sun Dec 16 23:29:56 2012 (r244323) +++ stable/9/sys/security/audit/audit_bsm_klib.c Sun Dec 16 23:41:34 2012 (r244324) @@ -462,13 +462,13 @@ auditon_command_event(int cmd) * leave the filename starting with '/' in the audit log in this case. */ void -audit_canon_path(struct thread *td, char *path, char *cpath) +audit_canon_path(struct thread *td, int dirfd, char *path, char *cpath) { struct vnode *cvnp, *rvnp; char *rbuf, *fbuf, *copy; struct filedesc *fdp; struct sbuf sbf; - int error, cwir; + int error, needslash, vfslocked; WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, "%s: at %s:%d", __func__, __FILE__, __LINE__); @@ -491,10 +491,27 @@ audit_canon_path(struct thread *td, char * path. */ if (*path != '/') { - cvnp = fdp->fd_cdir; - vhold(cvnp); + if (dirfd == AT_FDCWD) { + cvnp = fdp->fd_cdir; + vhold(cvnp); + } else { + /* XXX: fgetvp() that vhold()s vnode instead of vref()ing it would be better */ + error = fgetvp(td, dirfd, 0, &cvnp); + if (error) { + cpath[0] = '\0'; + if (rvnp != NULL) + vdrop(rvnp); + return; + } + vhold(cvnp); + vfslocked = VFS_LOCK_GIANT(cvnp->v_mount); + vrele(cvnp); + VFS_UNLOCK_GIANT(vfslocked); + } + needslash = (fdp->fd_rdir != cvnp); + } else { + needslash = 1; } - cwir = (fdp->fd_rdir == fdp->fd_cdir); FILEDESC_SUNLOCK(fdp); /* * NB: We require that the supplied array be at least MAXPATHLEN bytes @@ -536,7 +553,7 @@ audit_canon_path(struct thread *td, char (void) sbuf_cat(&sbf, rbuf); free(fbuf, M_TEMP); } - if (cwir == 0 || (cwir != 0 && cvnp == NULL)) + if (needslash) (void) sbuf_putc(&sbf, '/'); /* * Now that we have processed any alternate root and relative path Modified: stable/9/sys/security/audit/audit_private.h ============================================================================== --- stable/9/sys/security/audit/audit_private.h Sun Dec 16 23:29:56 2012 (r244323) +++ stable/9/sys/security/audit/audit_private.h Sun Dec 16 23:41:34 2012 (r244324) @@ -388,7 +388,8 @@ au_event_t audit_flags_and_error_to_ope au_event_t audit_flags_and_error_to_openatevent(int oflags, int error); au_event_t audit_msgctl_to_event(int cmd); au_event_t audit_semctl_to_event(int cmr); -void audit_canon_path(struct thread *td, char *path, char *cpath); +void audit_canon_path(struct thread *td, int dirfd, char *path, + char *cpath); au_event_t auditon_command_event(int cmd); /* Modified: stable/9/sys/security/audit/audit_worker.c ============================================================================== --- stable/9/sys/security/audit/audit_worker.c Sun Dec 16 23:29:56 2012 (r244323) +++ stable/9/sys/security/audit/audit_worker.c Sun Dec 16 23:41:34 2012 (r244324) @@ -88,6 +88,7 @@ static struct proc *audit_thread; static int audit_file_rotate_wait; static struct ucred *audit_cred; static struct vnode *audit_vp; +static off_t audit_size; static struct sx audit_worker_lock; #define AUDIT_WORKER_LOCK_INIT() sx_init(&audit_worker_lock, \ @@ -115,7 +116,6 @@ audit_record_write(struct vnode *vp, str struct statfs *mnt_stat; int error, vfslocked; static int cur_fail; - struct vattr vattr; long temp; AUDIT_WORKER_LOCK_ASSERT(); @@ -134,12 +134,6 @@ audit_record_write(struct vnode *vp, str error = VFS_STATFS(vp->v_mount, mnt_stat); if (error) goto fail; - vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); - error = VOP_GETATTR(vp, &vattr, cred); - VOP_UNLOCK(vp, 0); - if (error) - goto fail; - audit_fstat.af_currsz = vattr.va_size; /* * We handle four different space-related limits: @@ -196,11 +190,11 @@ audit_record_write(struct vnode *vp, str * to the daemon. This is only approximate, which is fine as more * records may be generated before the daemon rotates the file. */ - if ((audit_fstat.af_filesz != 0) && (audit_file_rotate_wait == 0) && - (vattr.va_size >= audit_fstat.af_filesz)) { + if (audit_fstat.af_filesz != 0 && + audit_size >= audit_fstat.af_filesz * (audit_file_rotate_wait + 1)) { AUDIT_WORKER_LOCK_ASSERT(); - audit_file_rotate_wait = 1; + audit_file_rotate_wait++; (void)audit_send_trigger(AUDIT_TRIGGER_ROTATE_KERNEL); } @@ -239,6 +233,8 @@ audit_record_write(struct vnode *vp, str goto fail_enospc; else if (error) goto fail; + AUDIT_WORKER_LOCK_ASSERT(); + audit_size += len; /* * Catch completion of a queue drain here; if we're draining and the @@ -452,10 +448,20 @@ audit_rotate_vnode(struct ucred *cred, s struct ucred *old_audit_cred; struct vnode *old_audit_vp; int vfslocked; + struct vattr vattr; KASSERT((cred != NULL && vp != NULL) || (cred == NULL && vp == NULL), ("audit_rotate_vnode: cred %p vp %p", cred, vp)); + if (vp != NULL) { + vn_lock(vp, LK_SHARED | LK_RETRY); + if (VOP_GETATTR(vp, &vattr, cred) != 0) + vattr.va_size = 0; + VOP_UNLOCK(vp, 0); + } else { + vattr.va_size = 0; + } + /* * Rotate the vnode/cred, and clear the rotate flag so that we will * send a rotate trigger if the new file fills. @@ -465,6 +471,7 @@ audit_rotate_vnode(struct ucred *cred, s old_audit_vp = audit_vp; audit_cred = cred; audit_vp = vp; + audit_size = vattr.va_size; audit_file_rotate_wait = 0; audit_enabled = (audit_vp != NULL); AUDIT_WORKER_UNLOCK();
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201212162341.qBGNfYem074402>