PERFORCE change 167187 for review

[Ilias Marinos]

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

Change 167187 by marinosi at marinosi_redrum on 2009/08/10 19:29:17

	- Several fixes.
	- Added audit_slice_lookup() function.

Affected files ...

.. //depot/projects/soc2009/marinosi_appaudit/src/sys/bsm/audit_internal.h#6 edit
.. //depot/projects/soc2009/marinosi_appaudit/src/sys/security/audit/audit.c#15 edit
.. //depot/projects/soc2009/marinosi_appaudit/src/sys/security/audit/audit_slice.h#13 edit
.. //depot/projects/soc2009/marinosi_appaudit/src/sys/security/audit/audit_worker.c#7 edit

Differences ...

==== //depot/projects/soc2009/marinosi_appaudit/src/sys/bsm/audit_internal.h#6 (text) ====

@@ -130,6 +130,9 @@
  * milliseconds of time    4 bytes/8 bytes (32-bit/64-bit value)
  *
  * XXXRW: Should use fixed-length types here rather than struct timespec.
+ * 
+ * Removed struct timespec to avoid padding. Have to check the length for the
+ * different architectures.
  */
 struct bsm_rec_hdr {
 	u_char			token_id;
@@ -137,7 +140,8 @@
 	u_char			version;
 	u_int16_t		e_type;
 	u_int16_t		e_mod;
-	struct timespec		tm;
+	time_t			tv_sec;		/* seconds */
+	long			tv_nsec;	/* and nanoseconds */
 } __packed;
 
 /*

==== //depot/projects/soc2009/marinosi_appaudit/src/sys/security/audit/audit.c#15 (text) ====

@@ -711,9 +711,9 @@
 	/*
 	 * XXXRW: Locking needed here.  Possibly we should fully initialize
 	 * the slice before inserting it on the list?
+	 * FIXED.
 	 */
 	as_ptr = as;
-	TAILQ_INSERT_TAIL(&audit_slice_q, as, as_q);
 
 	/* Initialize the base slice */
 	audit_slice_init(as, name);
@@ -729,6 +729,9 @@
 	/* Create the special device node */
 	audit_slice_cdev_init(as);
 
+	/* Insert the slice on the list */
+	TAILQ_INSERT_TAIL(&audit_slice_q, as, as_q);
+
 	AUDIT_SLICES_UNLOCK();
 
 }
@@ -806,9 +809,18 @@
  * never be the base slice as it is not a slice queue element.
  */
 int
-audit_slice_destroy(struct audit_slice *as)
+audit_slice_destroy(char *as_name)
 {
+	int error;
+	struct audit_slice *as = NULL;
 
+	error = audit_slice_lookup(as_name, as);
+	if (error)
+		return (1);
+	
+	if ( as == audit_base_slice )
+		return (1);	/* Cannot destroy base slice */
+
 	AUDIT_SLICES_LOCK();
 	/*
 	 * XXXRW: Should either assert the record queue is empty, or drain
@@ -816,11 +828,19 @@
 	 *
 	 * XXXRW: Need to mtx_destroy the lock, cv_destroy the condition
 	 * variables?
+	 * FIXED.
+	 * Note: Maybe it's better to use macros for this.
 	 */
 	if (as != NULL) {
 		AUDIT_SLICES_LOCK_ASSERT();
+		cv_destroy(&(as)->audit_worker_cv);
+		cv_destroy(&(as)->audit_watermark_cv);
+		cv_destroy(&(as)->audit_fail_cv);
+		sx_destroy(&(as)->audit_worker_lock);
+		mtx_destroy(&(as)->audit_mtx);
+		mtx_destroy(&(as)->as_dev_mtx);
+		destroy_dev(as->as_dev);
 		TAILQ_REMOVE(&audit_slice_q, as, as_q);
-		destroy_dev(as->as_dev);
 		free(as, M_AUDITSLICE);
 	}
 	AUDIT_SLICES_UNLOCK();
@@ -841,7 +861,6 @@
 	int error;
 	struct thread *td = NULL;
 
-
 	/*
 	 * XXXRW: This error value seems never to be used?  Possibly we
 	 * should validate the record before calling audit_new, and return
@@ -916,3 +935,44 @@
 	mtx_unlock(&(as->audit_mtx));
 	return (0);
 }
+
+/*
+ * audit_slice_lookup() performs a linear lookup in the audit slices queue
+ * bases on the slice name and sets up as to point to the actual slice
+ * instance.
+ * Returns '0' on success, error code on failure.
+ */
+int
+audit_slice_lookup(char *as_name, struct audit_slice *as)
+{
+	int nbytes;
+	struct audit_slice *cur = NULL;
+
+	nbytes = strlen(as_name);
+	if ( nbytes <= 0 || nbytes > AUDIT_SLICE_NAME_LEN )
+		return (EINVAL);
+
+	if ( strcmp(as_name, "audit_base_slice") == 0 ) {
+		as = audit_base_slice;
+		return (0);
+	}
+
+	/*
+	 * Use lock to prevent slice creation/removal while iterating through
+	 * the queue, searching for a slice.
+	 */
+	AUDIT_SLICES_LOCK();
+	TAILQ_FOREACH(cur, &audit_slice_q, as_q) {
+		if ( strcmp(cur->as_name, as_name) == 0 ) {
+			as = cur;
+			AUDIT_SLICES_UNLOCK();
+			return (0);
+		}
+	}
+
+	/*
+	 * On failure.(slice not found)
+	 */
+	return (1);
+
+}

==== //depot/projects/soc2009/marinosi_appaudit/src/sys/security/audit/audit_slice.h#13 (text+ko) ====

@@ -176,7 +176,6 @@
 /* Audit slices queue. */
 extern struct audit_slice_queue		audit_slice_q;
 
-
 /*
  * Functions to manage the allocation, release, and commit of kernel audit
  * records and require audit_slice struct as arguments.
@@ -191,8 +190,9 @@
 void	audit_worker_start(struct audit_slice *as);
 void	audit_slice_init(struct audit_slice *as, char *name);
 void	audit_slice_create(char *name);
-int	audit_slice_destroy(struct audit_slice *as);
+int	audit_slice_destroy(char *as_name);
 void	audit_slice_cdev_init(struct audit_slice *as);
 int	audit_slice_commit_rec(void *rec, struct audit_slice *as);
+int	audit_slice_lookup(char *as_name, struct audit_slice *as);
 
 #endif /* ! _SECURITY_AUDIT_SLICE_H_ */

==== //depot/projects/soc2009/marinosi_appaudit/src/sys/security/audit/audit_worker.c#7 (text) ====

@@ -445,6 +445,8 @@
  *
  * XXXRW: We'd like to be able to rotate for slices other than
  * audit_base_slice in the future, as well.
+ * FIXED.
+ * Note: Added slice as argument.
  */
 void
 audit_rotate_vnode(struct audit_slice *as, struct ucred *cred, struct vnode *vp)