[PATCH] SELinux: fix deadlock on dcache lock

This fixes a deadlock on the dcache lock detected during testing at IBM by moving the logging of the current executable information from the SELinux avc_audit function to audit_log_exit (via an audit_log_task_info helper) for processing upon syscall exit. For consistency, the patch also removes the logging of other task-related information from avc_audit, deferring handling to audit_log_exit instead. This allows simplification of the avc_audit code, allows the exe information to be obtained more reliably, always includes the comm information (useful for scripts), and avoids including bogus task information for checks performed from irq or softirq. Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> Signed-off-by: James Morris <jmorris@redhat.com> Signed-off-by: Linus Torvalds <torvalds@osdl.org>

[PATCH] SELinux: fix deadlock on dcache lock
This fixes a deadlock on the dcache lock detected during testing at IBM by moving the logging of the current executable information from the SELinux avc_audit function to audit_log_exit (via an audit_log_task_info helper) for processing upon syscall exit. For consistency, the patch also removes the logging of other task-related information from avc_audit, deferring handling to audit_log_exit instead. This allows simplification of the avc_audit code, allows the exe information to be obtained more reliably, always includes the comm information (useful for scripts), and avoids including bogus task information for checks performed from irq or softirq. Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> Signed-off-by: James Morris <jmorris@redhat.com> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Stephen Smalley · Linus Torvalds
1 parent 865108d138
Showing 2 changed files with 28 additions and 34 deletions Side-by-side Diff
kernel/auditsc.c
security/selinux/avc.c
@@ -610,6 +610,33 @@
 		printk(KERN_ERR "audit: freed %d contexts\n", count);
 }
  
+static void audit_log_task_info(struct audit_buffer *ab)
+{
+	char name[sizeof(current->comm)];
+	struct mm_struct *mm = current->mm;
+	struct vm_area_struct *vma;
+
+	get_task_comm(name, current);
+	audit_log_format(ab, " comm=%s", name);
+
+	if (!mm)
+		return;
+
+	down_read(&mm->mmap_sem);
+	vma = mm->mmap;
+	while (vma) {
+		if ((vma->vm_flags & VM_EXECUTABLE) &&
+		    vma->vm_file) {
+			audit_log_d_path(ab, "exe=",
+					 vma->vm_file->f_dentry,
+					 vma->vm_file->f_vfsmnt);
+			break;
+		}
+		vma = vma->vm_next;
+	}
+	up_read(&mm->mmap_sem);
+}
+
 static void audit_log_exit(struct audit_context *context)
 {
 	int i;
@@ -639,6 +666,7 @@
 		  context->gid,
 		  context->euid, context->suid, context->fsuid,
 		  context->egid, context->sgid, context->fsgid);
+	audit_log_task_info(ab);
 	audit_log_end(ab);
 	while (context->aux) {
 		struct audit_aux_data *aux;
@@ -532,7 +532,6 @@
                u16 tclass, u32 requested,
                struct av_decision *avd, int result, struct avc_audit_data *a)
 {
-	struct task_struct *tsk = current;
 	struct inode *inode = NULL;
 	u32 denied, audited;
 	struct audit_buffer *ab;
@@ -556,39 +555,6 @@
 	audit_log_format(ab, "avc:  %s ", denied ? "denied" : "granted");
 	avc_dump_av(ab, tclass,audited);
 	audit_log_format(ab, " for ");
-	if (a && a->tsk)
-		tsk = a->tsk;
-	if (tsk && tsk->pid) {
-		struct mm_struct *mm;
-		struct vm_area_struct *vma;
-		audit_log_format(ab, " pid=%d", tsk->pid);
-		if (tsk == current)
-			mm = current->mm;
-		else
-			mm = get_task_mm(tsk);
-		if (mm) {
-			if (down_read_trylock(&mm->mmap_sem)) {
-				vma = mm->mmap;
-				while (vma) {
-					if ((vma->vm_flags & VM_EXECUTABLE) &&
-					    vma->vm_file) {
-						audit_log_d_path(ab, "exe=",
-							vma->vm_file->f_dentry,
-							vma->vm_file->f_vfsmnt);
-						break;
-					}
-					vma = vma->vm_next;
-				}
-				up_read(&mm->mmap_sem);
-			} else {
-				audit_log_format(ab, " comm=%s", tsk->comm);
-			}
-			if (tsk != current)
-				mmput(mm);
-		} else {
-			audit_log_format(ab, " comm=%s", tsk->comm);
-		}
-	}
 	if (a) {
 		switch (a->type) {
 		case AVC_AUDIT_DATA_IPC:
...	...	@@ -610,6 +610,33 @@
610	610	printk(KERN_ERR "audit: freed %d contexts\n", count);
611	611	}
612	612
	613	+static void audit_log_task_info(struct audit_buffer *ab)
	614	+{
	615	+ char name[sizeof(current->comm)];
	616	+ struct mm_struct *mm = current->mm;
	617	+ struct vm_area_struct *vma;
	618	+
	619	+ get_task_comm(name, current);
	620	+ audit_log_format(ab, " comm=%s", name);
	621	+
	622	+ if (!mm)
	623	+ return;
	624	+
	625	+ down_read(&mm->mmap_sem);
	626	+ vma = mm->mmap;
	627	+ while (vma) {
	628	+ if ((vma->vm_flags & VM_EXECUTABLE) &&
	629	+ vma->vm_file) {
	630	+ audit_log_d_path(ab, "exe=",
	631	+ vma->vm_file->f_dentry,
	632	+ vma->vm_file->f_vfsmnt);
	633	+ break;
	634	+ }
	635	+ vma = vma->vm_next;
	636	+ }
	637	+ up_read(&mm->mmap_sem);
	638	+}
	639	+
613	640	static void audit_log_exit(struct audit_context *context)
614	641	{
615	642	int i;
...	...	@@ -639,6 +666,7 @@
639	666	context->gid,
640	667	context->euid, context->suid, context->fsuid,
641	668	context->egid, context->sgid, context->fsgid);
	669	+ audit_log_task_info(ab);
642	670	audit_log_end(ab);
643	671	while (context->aux) {
644	672	struct audit_aux_data *aux;
...	...	@@ -532,7 +532,6 @@
532	532	u16 tclass, u32 requested,
533	533	struct av_decision avd, int result, struct avc_audit_data a)
534	534	{
535		- struct task_struct *tsk = current;
536	535	struct inode *inode = NULL;
537	536	u32 denied, audited;
538	537	struct audit_buffer *ab;
...	...	@@ -556,39 +555,6 @@
556	555	audit_log_format(ab, "avc: %s ", denied ? "denied" : "granted");
557	556	avc_dump_av(ab, tclass,audited);
558	557	audit_log_format(ab, " for ");
559		- if (a && a->tsk)
560		- tsk = a->tsk;
561		- if (tsk && tsk->pid) {
562		- struct mm_struct *mm;
563		- struct vm_area_struct *vma;
564		- audit_log_format(ab, " pid=%d", tsk->pid);
565		- if (tsk == current)
566		- mm = current->mm;
567		- else
568		- mm = get_task_mm(tsk);
569		- if (mm) {
570		- if (down_read_trylock(&mm->mmap_sem)) {
571		- vma = mm->mmap;
572		- while (vma) {
573		- if ((vma->vm_flags & VM_EXECUTABLE) &&
574		- vma->vm_file) {
575		- audit_log_d_path(ab, "exe=",
576		- vma->vm_file->f_dentry,
577		- vma->vm_file->f_vfsmnt);
578		- break;
579		- }
580		- vma = vma->vm_next;
581		- }
582		- up_read(&mm->mmap_sem);
583		- } else {
584		- audit_log_format(ab, " comm=%s", tsk->comm);
585		- }
586		- if (tsk != current)
587		- mmput(mm);
588		- } else {
589		- audit_log_format(ab, " comm=%s", tsk->comm);
590		- }
591		- }
592	558	if (a) {
593	559	switch (a->type) {
594	560	case AVC_AUDIT_DATA_IPC: