linux-kernel - Re: [PATCH V6] audit: save signal match info in case entry passed in is the one deleted

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <31979695.S9xeMZMGso@sifl>
Date:	Wed, 05 Aug 2015 17:51:18 -0400
From:	Paul Moore <pmoore@...hat.com>
To:	Richard Guy Briggs <rgb@...hat.com>
Cc:	linux-audit@...hat.com, linux-kernel@...r.kernel.org,
	sgrubb@...hat.com, eparis@...hat.com
Subject: Re: [PATCH V6] audit: save signal match info in case entry passed in is the one deleted

On Wednesday, August 05, 2015 03:23:09 PM Richard Guy Briggs wrote:
> Move the access to the entry for audit_match_signal() to earlier in 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@...hat.com>
> ---
> Revision history:
> v6:
>     Fix a rebase manual merge error that mixed parts of two patches.
> 
> 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 |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)

Much better :)  Applied.

FYI, I did change the subject line as it didn't make sense any more.

> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 4cb9b44..018719a 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,10 +963,6 @@ 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);
> -
>  #ifdef CONFIG_AUDITSYSCALL
>  	if (!dont_count)
>  		audit_n_rules--;
> @@ -975,9 +970,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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/