Re: [PATCH V4 (was V6) 1/2] audit: eliminate unnecessary extra layer of watch references

[Paul Moore <pmoore@redhat.com>]

On Saturday, August 01, 2015 03:41:12 PM Richard Guy Briggs wrote:
> The audit watch count was imbalanced, adding an unnecessary layer of watch
> references.  Only add the second reference when it is added to a parent.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit_watch.c |    5 ++---
>  kernel/auditfilter.c |   16 +++-------------
>  2 files changed, 5 insertions(+), 16 deletions(-)

Merged.  I'll push it out as soon as I finish up the reviews tonight.

> diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
> index 6e30024..f33f54c 100644
> --- a/kernel/audit_watch.c
> +++ b/kernel/audit_watch.c
> @@ -203,7 +203,6 @@ int audit_to_watch(struct audit_krule *krule, char
> *path, int len, u32 op) if (IS_ERR(watch))
>  		return PTR_ERR(watch);
> 
> -	audit_get_watch(watch);
>  	krule->watch = watch;
> 
>  	return 0;
> @@ -387,8 +386,7 @@ static void audit_add_to_parent(struct audit_krule
> *krule,
> 
>  		watch_found = 1;
> 
> -		/* put krule's and initial refs to temporary watch */
> -		audit_put_watch(watch);
> +		/* put krule's ref to temporary watch */
>  		audit_put_watch(watch);
> 
>  		audit_get_watch(w);
> @@ -400,6 +398,7 @@ static void audit_add_to_parent(struct audit_krule
> *krule, audit_get_parent(parent);
>  		watch->parent = parent;
> 
> +		audit_get_watch(watch);
>  		list_add(&watch->wlist, &parent->watches);
>  	}
>  	list_add(&krule->rlist, &watch->rules);
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 72e1660..4cb9b44 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -549,8 +549,6 @@ exit_nofree:
>  	return entry;
> 
>  exit_free:
> -	if (entry->rule.watch)
> -		audit_put_watch(entry->rule.watch); /* matches initial get */
>  	if (entry->rule.tree)
>  		audit_put_tree(entry->rule.tree); /* that's the temporary one */
>  	audit_free_rule(entry);
> @@ -881,7 +879,7 @@ static inline int audit_add_rule(struct audit_entry
> *entry) /* normally audit_add_tree_rule() will free it on failure */
>  		if (tree)
>  			audit_put_tree(tree);
> -		goto error;
> +		return err;
>  	}
> 
>  	if (watch) {
> @@ -895,14 +893,14 @@ static inline int audit_add_rule(struct audit_entry
> *entry) */
>  			if (tree)
>  				audit_put_tree(tree);
> -			goto error;
> +			return err;
>  		}
>  	}
>  	if (tree) {
>  		err = audit_add_tree_rule(&entry->rule);
>  		if (err) {
>  			mutex_unlock(&audit_filter_mutex);
> -			goto error;
> +			return err;
>  		}
>  	}
> 
> @@ -933,11 +931,6 @@ static inline int audit_add_rule(struct audit_entry
> *entry) #endif
>  	mutex_unlock(&audit_filter_mutex);
> 
> - 	return 0;
> -
> -error:
> -	if (watch)
> -		audit_put_watch(watch); /* tmp watch, matches initial get */
>  	return err;
>  }
> 
> @@ -945,7 +938,6 @@ error:
>  static inline int audit_del_rule(struct audit_entry *entry)
>  {
>  	struct audit_entry  *e;
> -	struct audit_watch *watch = entry->rule.watch;
>  	struct audit_tree *tree = entry->rule.tree;
>  	struct list_head *list;
>  	int ret = 0;
> @@ -986,8 +978,6 @@ static inline int audit_del_rule(struct audit_entry
> *entry) mutex_unlock(&audit_filter_mutex);
> 
>  out:
> -	if (watch)
> -		audit_put_watch(watch); /* match initial get */
>  	if (tree)
>  		audit_put_tree(tree);	/* that's the temporary one */

-- 
paul moore
security @ redhat