All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mitchell Blank Jr <mitch@sfgoth.com>
To: dwmw2@infradead.org
Cc: linux-audit@redhat.com
Subject: [PATCH] [AUDIT] auditfilter.c cleanup/const-ification
Date: Mon, 3 Apr 2006 05:51:28 -0700	[thread overview]
Message-ID: <20060403125128.GG3157@gaz.sfgoth.com> (raw)

(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@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)

             reply	other threads:[~2006-04-03 12:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-04-03 12:51 Mitchell Blank Jr [this message]
2006-04-03 20:56 ` [PATCH] [AUDIT] auditfilter.c cleanup/const-ification Steve Grubb
2006-04-03 23:46   ` Mitchell Blank Jr
2006-04-04 14:37 ` Amy Griffis
2006-04-05 11:41   ` Mitchell Blank Jr
2006-04-05 12:29     ` Steve Grubb
2006-04-05 13:50       ` Mitchell Blank Jr
2006-04-06 13:43         ` Mitchell Blank Jr
2006-04-06 15:41           ` Alexander Viro
2006-04-05 13:30     ` Amy Griffis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060403125128.GG3157@gaz.sfgoth.com \
    --to=mitch@sfgoth.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-audit@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.