From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mitchell Blank Jr Subject: [PATCH] [AUDIT] auditfilter.c cleanup/const-ification Date: Mon, 3 Apr 2006 05:51:28 -0700 Message-ID: <20060403125128.GG3157@gaz.sfgoth.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mx3.redhat.com (mx3.redhat.com [172.16.48.32]) by int-mx1.corp.redhat.com (8.12.11.20060308/8.11.6) with ESMTP id k33CllnS014863 for ; Mon, 3 Apr 2006 08:47:47 -0400 Received: from gaz.sfgoth.com (gaz.sfgoth.com [69.36.241.230]) by mx3.redhat.com (8.13.1/8.13.1) with ESMTP id k33Clekv022989 for ; Mon, 3 Apr 2006 08:47:40 -0400 Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: dwmw2@infradead.org Cc: linux-audit@redhat.com List-Id: linux-audit@redhat.com (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 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)