[PATCH] [AUDIT] auditfilter.c cleanup/const-ification

Mitchell Blank Jr mitch at sfgoth.com
Mon Apr 3 12:51:28 UTC 2006


(Not sure if this will make it to the linux-audit mailing list or not; I
tried subscribing to it via the web page a couple days ago but haven't heard
back from mailman)

I noticed the gcc 4.X warning:
	kernel/auditfilter.c: In function "audit_filter_user"
	kernel/auditfilter.c:588: warning: "state" may be used uninitialized in this function
...and thought I'd take a quick look.

The gcc warning isn't correct (since audit_filter_user() only looked at state
if audit_filter_user_rules() returned non-zero, in which case 'state' would
have been initialized)  However the code was needlessly complex --
audit_filter_user_rules() carefully populated the "enum audit_state *state"
with various value but it's only caller just cares if it's AUDIT_DISABLED
or not.  It's shorter and simpler to just let audit_filter_user_rules()
modify its caller's return value more directly.  As an added bonus this
also removes the warning.

While I was looking at auditfilter.c I did some other minor cleanup

  * const-ified pointers where possible

  * both audit_data_to_entry() and audit_krule_to_data() had an unused
    variable called "void *bufp" which I removed

  * [minor] I changed some variables from "int" to "unsigned int" if
    they can't be negative.  Since ->field_count is unsigned I think it's
    a little cleaner to use an unsigned type to iterate through it

I'm not actually using the audit subsystem currently but this at least
compiles and boots ok.  I believe it should all be safe.

Signed-off-by: Mitchell Blank Jr <mitch at sfgoth.com>

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 1c47c59..c70049e 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -363,10 +363,11 @@ extern void		    audit_log_d_path(struct
 					     struct dentry *dentry,
 					     struct vfsmount *vfsmnt);
 				/* Private API (for audit.c only) */
-extern int audit_filter_user(struct netlink_skb_parms *cb, int type);
+extern int audit_filter_user(const struct netlink_skb_parms *cb, int type);
 extern int audit_filter_type(int type);
 extern int  audit_receive_filter(int type, int pid, int uid, int seq,
-				 void *data, size_t datasz, uid_t loginuid);
+				 const void *data, size_t datasz,
+				 uid_t loginuid);
 #else
 #define audit_log(c,g,t,f,...) do { ; } while (0)
 #define audit_log_start(c,g,t) ({ NULL; })
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index d3a8539..c9932a7 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -80,12 +80,13 @@ static __attribute__((unused)) char *aud
 }
 
 /* Common user-space to kernel rule translation. */
-static inline struct audit_entry *audit_to_entry_common(struct audit_rule *rule)
+static inline struct audit_entry *audit_to_entry_common(const struct audit_rule *rule)
 {
 	unsigned listnr;
 	struct audit_entry *entry;
 	struct audit_field *fields;
-	int i, err;
+	unsigned int i;
+	int err;
 
 	err = -EINVAL;
 	listnr = rule->flags & ~AUDIT_FILTER_PREPEND;
@@ -137,11 +138,11 @@ exit_err:
 
 /* Translate struct audit_rule to kernel's rule respresentation.
  * Exists for backward compatibility with userspace. */
-static struct audit_entry *audit_rule_to_entry(struct audit_rule *rule)
+static struct audit_entry *audit_rule_to_entry(const struct audit_rule *rule)
 {
 	struct audit_entry *entry;
 	int err = 0;
-	int i;
+	unsigned int i;
 
 	entry = audit_to_entry_common(rule);
 	if (IS_ERR(entry))
@@ -182,20 +183,18 @@ exit_free:
 }
 
 /* Translate struct audit_rule_data to kernel's rule respresentation. */
-static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data,
+static struct audit_entry *audit_data_to_entry(const struct audit_rule_data *data,
 					       size_t datasz)
 {
 	int err = 0;
 	struct audit_entry *entry;
-	void *bufp;
 	/* size_t remain = datasz - sizeof(struct audit_rule_data); */
-	int i;
+	unsigned int i;
 
-	entry = audit_to_entry_common((struct audit_rule *)data);
+	entry = audit_to_entry_common((const struct audit_rule *) data);
 	if (IS_ERR(entry))
 		goto exit_nofree;
 
-	bufp = data->buf;
 	entry->rule.vers_ops = 2;
 	for (i = 0; i < data->field_count; i++) {
 		struct audit_field *f = &entry->rule.fields[i];
@@ -223,7 +222,7 @@ exit_free:
 }
 
 /* Pack a filter field's string representation into data block. */
-static inline size_t audit_pack_string(void **bufp, char *str)
+static inline size_t audit_pack_string(void **bufp, const char *str)
 {
 	size_t len = strlen(str);
 
@@ -235,10 +234,10 @@ static inline size_t audit_pack_string(v
 
 /* Translate kernel rule respresentation to struct audit_rule.
  * Exists for backward compatibility with userspace. */
-static struct audit_rule *audit_krule_to_rule(struct audit_krule *krule)
+static struct audit_rule *audit_krule_to_rule(const struct audit_krule *krule)
 {
 	struct audit_rule *rule;
-	int i;
+	unsigned int i;
 
 	rule = kmalloc(sizeof(*rule), GFP_KERNEL);
 	if (unlikely(!rule))
@@ -265,11 +264,10 @@ static struct audit_rule *audit_krule_to
 }
 
 /* Translate kernel rule respresentation to struct audit_rule_data. */
-static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule)
+static struct audit_rule_data *audit_krule_to_data(const struct audit_krule *krule)
 {
 	struct audit_rule_data *data;
-	void *bufp;
-	int i;
+	unsigned int i;
 
 	data = kmalloc(sizeof(*data) + krule->buflen, GFP_KERNEL);
 	if (unlikely(!data))
@@ -279,7 +277,6 @@ static struct audit_rule_data *audit_kru
 	data->flags = krule->flags | krule->listnr;
 	data->action = krule->action;
 	data->field_count = krule->field_count;
-	bufp = data->buf;
 	for (i = 0; i < data->field_count; i++) {
 		struct audit_field *f = &krule->fields[i];
 
@@ -298,9 +295,10 @@ static struct audit_rule_data *audit_kru
 
 /* Compare two rules in kernel format.  Considered success if rules
  * don't match. */
-static int audit_compare_rule(struct audit_krule *a, struct audit_krule *b)
+static int audit_compare_rule(const struct audit_krule *a,
+			      const struct audit_krule *b)
 {
-	int i;
+	unsigned int i;
 
 	if (a->flags != b->flags ||
 	    a->listnr != b->listnr ||
@@ -353,7 +351,7 @@ static inline int audit_add_rule(struct 
 
 /* Remove an existing rule from filterlist.  Protected by
  * audit_netlink_mutex. */
-static inline int audit_del_rule(struct audit_entry *entry,
+static inline int audit_del_rule(const struct audit_entry *entry,
 				 struct list_head *list)
 {
 	struct audit_entry  *e;
@@ -376,8 +374,8 @@ static int audit_list(void *_dest)
 {
 	int pid, seq;
 	int *dest = _dest;
-	struct audit_entry *entry;
-	int i;
+	const struct audit_entry *entry;
+	unsigned int i;
 
 	pid = dest[0];
 	seq = dest[1];
@@ -410,8 +408,8 @@ static int audit_list_rules(void *_dest)
 {
 	int pid, seq;
 	int *dest = _dest;
-	struct audit_entry *e;
-	int i;
+	const struct audit_entry *e;
+	unsigned int i;
 
 	pid = dest[0];
 	seq = dest[1];
@@ -449,10 +447,10 @@ static int audit_list_rules(void *_dest)
  * @datasz: size of payload data
  * @loginuid: loginuid of sender
  */
-int audit_receive_filter(int type, int pid, int uid, int seq, void *data,
+int audit_receive_filter(int type, int pid, int uid, int seq, const void *data,
 			 size_t datasz, uid_t loginuid)
 {
-	struct task_struct *tsk;
+	const struct task_struct *tsk;
 	int *dest;
 	int err = 0;
 	struct audit_entry *entry;
@@ -546,14 +544,14 @@ int audit_comparator(const u32 left, con
 
 
 
-static int audit_filter_user_rules(struct netlink_skb_parms *cb,
-				   struct audit_krule *rule,
-				   enum audit_state *state)
+static int audit_filter_user_rules(const struct netlink_skb_parms *cb,
+				   const struct audit_krule *rule,
+				   int *enabledp)
 {
-	int i;
+	unsigned int i;
 
 	for (i = 0; i < rule->field_count; i++) {
-		struct audit_field *f = &rule->fields[i];
+		const struct audit_field *f = &rule->fields[i];
 		int result = 0;
 
 		switch (f->type) {
@@ -574,28 +572,20 @@ static int audit_filter_user_rules(struc
 		if (!result)
 			return 0;
 	}
-	switch (rule->action) {
-	case AUDIT_NEVER:    *state = AUDIT_DISABLED;	    break;
-	case AUDIT_POSSIBLE: *state = AUDIT_BUILD_CONTEXT;  break;
-	case AUDIT_ALWAYS:   *state = AUDIT_RECORD_CONTEXT; break;
-	}
+	if (rule->action == AUDIT_NEVER)
+		*enabledp = 0;
 	return 1;
 }
 
-int audit_filter_user(struct netlink_skb_parms *cb, int type)
+int audit_filter_user(const struct netlink_skb_parms *cb, int type)
 {
-	struct audit_entry *e;
-	enum audit_state   state;
+	const struct audit_entry *e;
 	int ret = 1;
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER], list) {
-		if (audit_filter_user_rules(cb, &e->rule, &state)) {
-			if (state == AUDIT_DISABLED)
-				ret = 0;
+	list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER], list)
+		if (audit_filter_user_rules(cb, &e->rule, &ret))
 			break;
-		}
-	}
 	rcu_read_unlock();
 
 	return ret; /* Audit by default */
@@ -603,7 +593,7 @@ int audit_filter_user(struct netlink_skb
 
 int audit_filter_type(int type)
 {
-	struct audit_entry *e;
+	const struct audit_entry *e;
 	int result = 0;
 	
 	rcu_read_lock();
@@ -612,9 +602,9 @@ int audit_filter_type(int type)
 
 	list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TYPE],
 				list) {
-		int i;
+		unsigned int i;
 		for (i = 0; i < e->rule.field_count; i++) {
-			struct audit_field *f = &e->rule.fields[i];
+			const struct audit_field *f = &e->rule.fields[i];
 			if (f->type == AUDIT_MSGTYPE) {
 				result = audit_comparator(type, f->op, f->val);
 				if (!result)




More information about the Linux-audit mailing list