PERFORCE change 97963 for review

Robert Watson rwatson at FreeBSD.org
Sat May 27 12:52:06 PDT 2006


http://perforce.freebsd.org/chv.cgi?CH=97963

Change 97963 by rwatson at rwatson_sesame on 2006/05/27 19:49:48

	- Replace a 'trail' flag for the audit pipe with a preselection
	  mode, making it easier to fit future preselection modes into a
	  reasonable API.  We now support two defined operating modes --
	  trail mode, which tracks the global audit trail preselection,
	  and local mode, which defines a set of local trail paramaters
	  based on the global model, but specific to the current pipe.
	  We provide two operations -- one to set the mode, and the other
	  to query it.  Support this using a new preselection mode flag
	  on the pipe rather than a flag in the pipe flags.
	
	- Fix or expand a number of comments, and add new ones.
	
	- Flag audit pipes as MPSAFE.
	
	- Slightly refactor audit_pipe_flush() and audit_pipe_free():
	  detach the pipe from the global list in free rather than flush,
	  as we may call flush at other points, and this would otherwise
	  result in the pipe no longer receiving audit records after a
	  flush.  Assert the mutex in audit_pipe_free().
	
	- Annotate why providing partial reads in the context of
	  parallelism is tricky.  I'm not yet sure if I think
	  implementing it is worth the complexity.

Affected files ...

.. //depot/projects/trustedbsd/audit3/sys/security/audit/audit_ioctl.h#11 edit
.. //depot/projects/trustedbsd/audit3/sys/security/audit/audit_pipe.c#22 edit

Differences ...

==== //depot/projects/trustedbsd/audit3/sys/security/audit/audit_ioctl.h#11 (text+ko) ====

@@ -44,6 +44,15 @@
 };
 
 /*
+ * Possible modes of operation for audit pipe preselection.
+ */
+#define	AUDITPIPE_PRESELECT_MODE_TRAIL	1	/* Global audit trail. */
+#define	AUDITPIPE_PRESELECT_MODE_LOCAL	2	/* Local audit trail. */
+#ifdef NOTYET
+#define	AUDITPIPE_PRESELECT_MODE_PRIORITY	3	/* Prioritized trail. */
+#endif
+
+/*
  * Ioctls to read and control the behavior of individual audit pipe devices.
  */
 #define	AUDITPIPE_GET_QLEN		_IOR(AUDITPIPE_IOBASE, 1, u_int)
@@ -61,8 +70,8 @@
 					    struct auditpipe_ioctl_preselect)
 #define	AUDITPIPE_DELETE_PRESELECT_AUID	_IOW(AUDITPIPE_IOBASE, 12, au_id_t)
 #define	AUDITPIPE_FLUSH_PRESELECT_AUID	_IO(AUDITPIPE_IOBASE, 13)
-#define	AUDITPIPE_GET_PRESELECT_TRAIL	_IOR(AUDITPIPE_IOBASE, 14, int)
-#define	AUDITPIPE_SET_PRESELECT_TRAIL	_IOW(AUDITPIPE_IOBASE, 15, int)
+#define	AUDITPIPE_GET_PRESELECT_MODE	_IOR(AUDITPIPE_IOBASE, 14, int)
+#define	AUDITPIPE_SET_PRESELECT_MODE	_IOW(AUDITPIPE_IOBASE, 15, int)
 #define	AUDITPIPE_FLUSH			_IO(AUDITPIPE_IOBASE, 16)
 
 /*

==== //depot/projects/trustedbsd/audit3/sys/security/audit/audit_pipe.c#22 (text+ko) ====

@@ -88,8 +88,12 @@
  * Audit pipes allow processes to express "interest" in the set of records
  * that are delivered via the pipe.  They do this in a similar manner to the
  * mechanism for audit trail configuration, by expressing two global masks,
- * and optionally expressing per-auid masks.  The following data structures
- * define by the global masks for each pipe, and a list of per-auid masks.
+ * and optionally expressing per-auid masks.  The following data structure is
+ * the per-auid mask description.  The global state is stored in the audit
+ * pipe data structure.
+ *
+ * We may want to consider a more space/time-efficient data structure once
+ * usage patterns for per-auid specifications are clear.
  */
 struct audit_pipe_preselect {
 	au_id_t					 app_auid;
@@ -103,7 +107,6 @@
  */
 #define	AUDIT_PIPE_ASYNC	0x00000001
 #define	AUDIT_PIPE_NBIO		0x00000002
-#define	AUDIT_PIPE_TRAIL	0x00000004	/* Use trail preselection. */
 struct audit_pipe {
 	int				 ap_open;	/* Device open? */
 	u_int				 ap_flags;
@@ -124,12 +127,13 @@
 	 * processes (attributable, non-attributable), and a list of specific
 	 * interest specifications by auid.
 	 */
+	int				 ap_preselect_mode;
 	au_mask_t			 ap_preselect_flags;
 	au_mask_t			 ap_preselect_naflags;
 	TAILQ_HEAD(, audit_pipe_preselect)	ap_preselect_list;
 
 	/*
-	 * Record list.
+	 * Current pending record list.
 	 */
 	TAILQ_HEAD(, audit_pipe_entry)	 ap_queue;
 
@@ -140,15 +144,16 @@
 };
 
 /*
- * Global list of audit pipes, mutex to protect it and the pipes.  Finder
+ * Global list of audit pipes, mutex to protect it and the pipes.  Finer
  * grained locking may be desirable at some point.
  */
 static TAILQ_HEAD(, audit_pipe)	 audit_pipe_list;
 static struct mtx		 audit_pipe_mtx;
 
 /*
- * This CV is used to wakeup on an audit record write.  Eventually, it should
- * probably be per-pipe.
+ * This CV is used to wakeup on an audit record write.  Eventually, it might
+ * be per-pipe to avoid unnecessary wakeups when several pipes with different
+ * preselection masks are present.
  */
 static struct cv		 audit_pipe_cv;
 
@@ -170,7 +175,7 @@
 
 static struct cdevsw	audit_pipe_cdevsw = {
 	.d_version =	D_VERSION,
-	.d_flags =	D_PSEUDO,
+	.d_flags =	D_PSEUDO | D_NEEDGIANT,
 	.d_open =	audit_pipe_open,
 	.d_close =	audit_pipe_close,
 	.d_read =	audit_pipe_read,
@@ -215,6 +220,9 @@
 	return (NULL);
 }
 
+/*
+ * Query the per-pipe mask for a specific auid.
+ */
 static int
 audit_pipe_preselect_get(struct audit_pipe *ap, au_id_t auid,
     au_mask_t *maskp)
@@ -233,6 +241,10 @@
 	return (error);
 }
 
+/*
+ * Set the per-pipe mask for a specific auid.  Add a new entry if needed;
+ * otherwise, update the current entry.
+ */
 static void
 audit_pipe_preselect_set(struct audit_pipe *ap, au_id_t auid, au_mask_t mask)
 {
@@ -257,6 +269,9 @@
 		free(app_new, M_AUDIT_PIPE_PRESELECT);
 }
 
+/*
+ * Delete a per-auid mask on an audit pipe.
+ */
 static int
 audit_pipe_preselect_delete(struct audit_pipe *ap, au_id_t auid)
 {
@@ -276,6 +291,9 @@
 	return (error);
 }
 
+/*
+ * Delete all per-auid masks on an audit pipe.
+ */
 static void
 audit_pipe_preselect_flush(struct audit_pipe *ap)
 {
@@ -294,9 +312,9 @@
  * properties.  Algorithm is as follows:
  *
  * - If the pipe is configured to track the default trail configuration, then
- *   use that.
+ *   use the results of global preselection matching.
  * - If not, search for a specifically configured auid entry matching the
- *   event.  If it is found, use that.
+ *   event.  If an entry is found, use that.
  * - Otherwise, use the default flags or naflags configured for the pipe.
  */
 static int
@@ -307,24 +325,33 @@
 
 	mtx_assert(&audit_pipe_mtx, MA_OWNED);
 
-	if ((ap->ap_flags & AUDIT_PIPE_TRAIL) && trail_preselect)
-		return (1);
+	switch (ap->ap_preselect_mode) {
+	case AUDITPIPE_PRESELECT_MODE_TRAIL:
+		return (trail_preselect);
+
+	case AUDITPIPE_PRESELECT_MODE_LOCAL:
+		app = audit_pipe_preselect_find(ap, auid);
+		if (app == NULL) {
+			if (auid == AU_DEFAUDITID)
+				return (au_preselect(event, class,
+				    &ap->ap_preselect_naflags, sorf));
+			else
+				return (au_preselect(event, class,
+				    &ap->ap_preselect_flags, sorf));
+		} else
+			return (au_preselect(event, class, &app->app_mask,
+			    sorf));
+
+	default:
+		panic("audit_pipe_preselect_check: mode %d",
+		    ap->ap_preselect_mode);
+	}
 
-	app = audit_pipe_preselect_find(ap, auid);
-	if (app == NULL) {
-		if (auid == AU_DEFAUDITID)
-			return (au_preselect(event, class,
-			    &ap->ap_preselect_naflags, sorf));
-		else
-			return (au_preselect(event, class,
-			    &ap->ap_preselect_flags, sorf));
-	} else
-		return (au_preselect(event, class, &app->app_mask, sorf));
 	return (0);
 }
 
 /*
- * Determine whether there exists a pipe interested in a record with these
+ * Determine whether there exists a pipe interested in a record with specific
  * properties.
  */
 int
@@ -346,7 +373,7 @@
 }
 
 /*
- * Apparent individual record to a queue -- allocate queue-local buffer, and
+ * Append individual record to a queue -- allocate queue-local buffer, and
  * add to the queue.  We try to drop from the head of the queue so that more
  * recent events take precedence over older ones, but if allocation fails we
  * do drop the new event.
@@ -449,7 +476,7 @@
 
 
 /*
- * Read the next record off of an audit pipe.
+ * Pop the next record off of an audit pipe.
  */
 static struct audit_pipe_entry *
 audit_pipe_pop(struct audit_pipe *ap)
@@ -487,15 +514,15 @@
 
 	/*
 	 * Default flags, naflags, and auid-specific preselection settings to
-	 * 0.  Initialize the AUDIT_PIPE_TRAIL flag so that if praudit(1) is
-	 * run on /dev/auditpipe, it sees events associated with the default
-	 * trail.  Pipe-aware application can clear the flag, set custom
-	 * masks, and flush the pipe as needed.
+	 * 0.  Initialize the mode to the global trail so that if praudit(1)
+	 * is run on /dev/auditpipe, it sees events associated with the
+	 * default trail.  Pipe-aware application can clear the flag, set
+	 * custom masks, and flush the pipe as needed.
 	 */
 	bzero(&ap->ap_preselect_flags, sizeof(ap->ap_preselect_flags));
 	bzero(&ap->ap_preselect_naflags, sizeof(ap->ap_preselect_naflags));
-	ap->ap_flags |= AUDIT_PIPE_TRAIL;
 	TAILQ_INIT(&ap->ap_preselect_list);
+	ap->ap_preselect_mode = AUDITPIPE_PRESELECT_MODE_TRAIL;
 
 	TAILQ_INSERT_HEAD(&audit_pipe_list, ap, ap_list);
 	audit_pipe_count++;
@@ -505,7 +532,7 @@
 }
 
 /*
- * Flush all records from an audit pipe; assume mutex is held.
+ * Flush all records currently present in an audit pipe; assume mutex is held.
  */
 static void
 audit_pipe_flush(struct audit_pipe *ap)
@@ -514,7 +541,6 @@
 
 	mtx_assert(&audit_pipe_mtx, MA_OWNED);
 
-	TAILQ_REMOVE(&audit_pipe_list, ap, ap_list);
 	while ((ape = TAILQ_FIRST(&ap->ap_queue)) != NULL) {
 		TAILQ_REMOVE(&ap->ap_queue, ape, ape_queue);
 		audit_pipe_entry_free(ape);
@@ -524,14 +550,20 @@
 }
 
 /*
- * Free an audit pipe.  Assumes mutex is held, audit_pipe is still on the
- * global list.  Frees any audit pipe entries in the queue.
+ * Free an audit pipe; this means freeing all preselection state and all
+ * records in the pipe.  Assumes mutex is held to prevent any new records
+ * from being inserted during the free, and that the audit pipe is still on
+ * the global list.
  */
 static void
 audit_pipe_free(struct audit_pipe *ap)
 {
 
+	mtx_assert(&audit_pipe_mtx, MA_OWNED);
+
+	audit_pipe_preselect_flush(ap);
 	audit_pipe_flush(ap);
+	TAILQ_REMOVE(&audit_pipe_list, ap, ap_list);
 	free(ap, M_AUDIT_PIPE);
 	audit_pipe_count--;
 }
@@ -629,8 +661,8 @@
 	struct auditpipe_ioctl_preselect *aip;
 	struct audit_pipe *ap;
 	au_mask_t *maskp;
+	int error, mode;
 	au_id_t auid;
-	int error;
 
 	ap = dev->si_drv1;
 	KASSERT(ap != NULL, ("audit_pipe_ioctl: ap == NULL"));
@@ -765,21 +797,27 @@
 		error = 0;
 		break;
 
-	case AUDITPIPE_GET_PRESELECT_TRAIL:
+	case AUDITPIPE_GET_PRESELECT_MODE:
 		mtx_lock(&audit_pipe_mtx);
-		*(int *)data = (ap->ap_flags & AUDIT_PIPE_TRAIL) ? 1 : 0;
+		*(int *)data = ap->ap_preselect_mode;
 		mtx_unlock(&audit_pipe_mtx);
 		error = 0;
 		break;
 
-	case AUDITPIPE_SET_PRESELECT_TRAIL:
-		mtx_lock(&audit_pipe_mtx);
-		if (*(int *)data)
-			ap->ap_flags |= AUDIT_PIPE_TRAIL;
-		else
-			ap->ap_flags &= ~AUDIT_PIPE_TRAIL;
-		mtx_unlock(&audit_pipe_mtx);
-		error = 0;
+	case AUDITPIPE_SET_PRESELECT_MODE:
+		mode = *(int *)data;
+		switch (mode) {
+		case AUDITPIPE_PRESELECT_MODE_TRAIL:
+		case AUDITPIPE_PRESELECT_MODE_LOCAL:
+			mtx_lock(&audit_pipe_mtx);
+			ap->ap_preselect_mode = mode;
+			mtx_unlock(&audit_pipe_mtx);
+			error = 0;
+			break;
+
+		default:
+			error = EINVAL;
+		}
 		break;
 
 	case AUDITPIPE_FLUSH:
@@ -818,6 +856,17 @@
 /*
  * Audit pipe read.  Pull one record off the queue and copy to user space.
  * On error, the record is dropped.
+ *
+ * Providing more sophisticated behavior, such as partial reads, is tricky
+ * due to the potential for parallel I/O.  If partial read support is
+ * required, it will require a per-pipe "current record being read" along
+ * with an offset into that trecord which has already been read.  Threads
+ * performing partial reads will need to allocate per-thread copies of the
+ * data so that if another thread completes the read of the record, it can be
+ * freed without adding reference count logic.  If this is added, a flag to
+ * indicate that only atomic record reads are desired would be useful, as if
+ * different threads are all waiting for records on the pipe, they will want
+ * independent record reads, which is currently the behavior.
  */
 static int
 audit_pipe_read(struct cdev *dev, struct uio *uio, int flag)


More information about the trustedbsd-cvs mailing list