Re: PATCH [1/1]: audit: acquire creds selectively to reduce atomic op overhead

On Fri, Mar 11, 2011 at 04:33:52PM +0000, David Howells wrote: > get_task_cred() and task_cred_xxxx() call __task_cred() which uses Sorry, I thought you were referring to the remainder of auditsc.c > It's possible that the credentials being used in audit_filter_rules() are > incorrect under most circumstances and should be task->cred, not > task->real_cred. Agree. Also I believe it is safe to use tsk->cred directly as tsk == current or tsk is being created by copy_process.

Eric, can you ACK/NACK? The following patch corresponds to the above. Tony --- Commit c69e8d9c01db added calls to get_task_cred and put_cred in audit_filter_rules. Profiling with a large number of audit rules active on the exit chain shows that we are spending upto 48% in this routine for syscall intensive tests, most of which is in the atomic ops. The code should be accessing tsk->cred rather than tsk->real_cred. Also, since tsk is current (or tsk is being created by copy_process) direct access to tsk->cred is possible. Signed-off-by: Tony Jones <tonyj(a)suse.de&gt;

--- kernel/auditsc.c | 18 ++++++++++-------- 1 files changed, 10 insertions(+), 8 deletions(-) diff --git a/kernel/auditsc.c b/kernel/auditsc.c index f49a031..750c08ef 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -441,16 +441,21 @@ static int match_tree_refs(struct audit_context *ctx, struct audit_tree *tree) return 0; } -/* Determine if any context name data matches a rule's watch data */ -/* Compare a task_struct with an audit_rule. Return 1 on match, 0 - * otherwise. */ +/* + * Determine if any context name data matches a rule's watch data + * Compare a task_struct with an audit_rule. Return 1 on match, 0 + * otherwise. + * + * Note: tsk==current or we are being indirectly called from copy_process() + * so direct access to tsk->cred is allowed. + */ static int audit_filter_rules(struct task_struct *tsk, struct audit_krule *rule, struct audit_context *ctx, struct audit_names *name, enum audit_state *state) { - const struct cred *cred = get_task_cred(tsk); + const struct cred *cred = tsk->cred; int i, j, need_sid = 1; u32 sid; @@ -637,10 +642,8 @@ static int audit_filter_rules(struct task_struct *tsk, break; } - if (!result) { - put_cred(cred); + if (!result) return 0; - } } if (ctx) { @@ -656,7 +659,6 @@ static int audit_filter_rules(struct task_struct *tsk, case AUDIT_NEVER: *state = AUDIT_DISABLED; break; case AUDIT_ALWAYS: *state = AUDIT_RECORD_CONTEXT; break; } - put_cred(cred); return 1; }