Skip site navigation (1)Skip section navigation (2)
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>