Re: [PATCH V5] audit: save signal match info in case entry passed in is the one deleted

[Paul Moore <pmoore@redhat.com>]

On Wednesday, August 05, 2015 05:23:10 AM Richard Guy Briggs wrote:
> Move the access to the entry for audit_match_signal() to the beginning of
> the function in case the entry found is the same one passed in.  This will
> enable it to be used by audit_remove_mark_rule().
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> 
> Revision history:
> v4 -> v5:
>     Move mutex_unlock after out label.
>     Move list_del group after test for signal to remove temp variable.
> 
> ---
> This patch was split out from the audit by executable path patch set due to
> the potential to use it elsewhere.
> 
> In particular, some questions came up while assessing the potential for code
> reuse:
> 
> 	Why does audit_remove_parent_watches() not call audit_del_rule() for
> 	each entry found?
>                 Is audit_signals not properly decremented?
>                 Is audit_n_rules not properly decremented?
> 
>         Why does kill_rules() not call audit_del_rule() for each entry
> found? Is audit_signals not properly decremented?
>                 Is audit_n_rules not properly decremented?
> 
>  kernel/auditfilter.c |   13 ++++++++-----
>  1 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 4cb9b44..1b110fb 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -953,7 +953,6 @@ static inline int audit_del_rule(struct audit_entry
> *entry) mutex_lock(&audit_filter_mutex);
>  	e = audit_find_rule(entry, &list);
>  	if (!e) {
> -		mutex_unlock(&audit_filter_mutex);
>  		ret = -ENOENT;
>  		goto out;
>  	}
> @@ -964,9 +963,8 @@ static inline int audit_del_rule(struct audit_entry
> *entry) if (e->rule.tree)
>  		audit_remove_tree_rule(&e->rule);
> 
> -	list_del_rcu(&e->list);
> -	list_del(&e->rule.list);
> -	call_rcu(&e->rcu, audit_free_rule_rcu);
> +	if (e->rule.exe)
> +		audit_remove_mark_rule(&e->rule);

What?

I think you munged a cut n' paste somehow.  This code doesn't even compile.

>  #ifdef CONFIG_AUDITSYSCALL
>  	if (!dont_count)
> @@ -975,9 +973,14 @@ static inline int audit_del_rule(struct audit_entry
> *entry) if (!audit_match_signal(entry))
>  		audit_signals--;
>  #endif
> -	mutex_unlock(&audit_filter_mutex);
> +
> +	list_del_rcu(&e->list);
> +	list_del(&e->rule.list);
> +	call_rcu(&e->rcu, audit_free_rule_rcu);
> 
>  out:
> +	mutex_unlock(&audit_filter_mutex);
> +
>  	if (tree)
>  		audit_put_tree(tree);	/* that's the temporary one */

-- 
paul moore
security @ redhat