Audit: close race between audit_syscall_exit and proc_loginuid_read

audit_syscall_exit() calls audit_get_context() which returns the tsk->audit_context but then also sets tsk->audit_context=NULL. A few lines later audit_syscall_exit sets the audit_context (either back to audit_context or to a different context)

During this window when tsk->audit_context is NULL it is possible that another process will try to read /proc/pid/loginuid and will get a -1. There does not appear to be a good reason to set audit_context to null in the get function so this patch merely leaves the audit_context alone so there is no period of time in which audit_context is not pointing to a valid context.

---

Easily tested, I opened 2 shells.

shell #1 I read /proc/shell2/loginuid in a very very tight loop. shell #2 I ran a tight loop adding and removing audit rules

without the patch I often saw -1 as the loginuid but with the patch I always got the correct behavior.

 kernel/auditsc.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

index 04f3ffb..62ace9b 100644
@@ -598,7 +598,6 @@ static inline struct audit_context *audit_get_context(struct task_struct *tsk,
 
 get_context:
 
-	tsk->audit_context = NULL;
 	return context;
 }
 

@@ -1135,6 +1134,8 @@ void audit_free(struct task_struct *tsk)
 	if (likely(!context))
 		return;
 
+	tsk->audit_context = NULL;
+
 	/* Check for system calls that do not go through the exit
 	 * function (e.g., exit_group), then free context block. 
 	 * We use GFP_ATOMIC here because we might be doing this 

@@ -1266,9 +1267,9 @@ void audit_syscall_exit(int valid, long return_code)
 
 	if (context->previous) {
 		struct audit_context *new_context = context->previous;
+		tsk->audit_context = new_context;
 		context->previous  = NULL;
 		audit_free_context(context);
-		tsk->audit_context = new_context;
 	} else {
 		audit_free_names(context);
 		audit_free_aux(context);

The preferred way to solve this problem is to promote loginuid to be part of the task structure. We also have another problem where if the audit system is temporarily disabled, loginuid will not be set in any new programs. What we'd like to have happen is loginuid be available and usable all the time so that we always know the auid when the audit system starts back up.

-Steve

While obviously possible I don't see a reason that this patch is bad, wrong, or shouldn't go in. If things other than audit want to make use of the loginuid I wouldn't see a problem putting it in the task struct but as it stands now this patch fixes an obvious race for a solely audit problem.

-Eric