svn commit: r367002 - in head/sys: compat/freebsd32 kern

[Kyle Evans]

Author: kevans
Date: Sat Oct 24 14:39:17 2020
New Revision: 367002
URL: https://svnweb.freebsd.org/changeset/base/367002

Log:
  audit: correct reporting of *execve(2) success
  
  r326145 corrected do_execve() to return EJUSTRETURN upon success so that
  important registers are not clobbered. This had the side effect of tapping
  out 'failures' for all *execve(2) audit records, which is less than useful
  for auditing purposes.
  
  Audit exec returns earlier, where we can know for sure that EJUSTRETURN
  translates to success. Note that this unsets TDP_AUDITREC as we commit the
  audit record, so the usual audit in the syscall return path will do nothing.
  
  PR:		249179
  Reported by:	Eirik Oeverby <ltning-freebsd anduin net>
  Reviewed by:	csjp, kib
  MFC after:	1 week
  Sponsored by:	Klara, Inc.
  Differential Revision:	https://reviews.freebsd.org/D26922

Modified:
  head/sys/compat/freebsd32/freebsd32_misc.c
  head/sys/kern/kern_exec.c
  head/sys/kern/subr_syscall.c

Modified: head/sys/compat/freebsd32/freebsd32_misc.c
==============================================================================
--- head/sys/compat/freebsd32/freebsd32_misc.c	Sat Oct 24 14:25:38 2020	(r367001)
+++ head/sys/compat/freebsd32/freebsd32_misc.c	Sat Oct 24 14:39:17 2020	(r367002)
@@ -442,6 +442,7 @@ freebsd32_execve(struct thread *td, struct freebsd32_e
 	if (error == 0)
 		error = kern_execve(td, &eargs, NULL, oldvmspace);
 	post_execve(td, error, oldvmspace);
+	AUDIT_SYSCALL_EXIT(error == EJUSTRETURN ? 0 : error, td);
 	return (error);
 }
 
@@ -462,6 +463,7 @@ freebsd32_fexecve(struct thread *td, struct freebsd32_
 		error = kern_execve(td, &eargs, NULL, oldvmspace);
 	}
 	post_execve(td, error, oldvmspace);
+	AUDIT_SYSCALL_EXIT(error == EJUSTRETURN ? 0 : error, td);
 	return (error);
 }
 

Modified: head/sys/kern/kern_exec.c
==============================================================================
--- head/sys/kern/kern_exec.c	Sat Oct 24 14:25:38 2020	(r367001)
+++ head/sys/kern/kern_exec.c	Sat Oct 24 14:39:17 2020	(r367002)
@@ -225,6 +225,7 @@ sys_execve(struct thread *td, struct execve_args *uap)
 	if (error == 0)
 		error = kern_execve(td, &args, NULL, oldvmspace);
 	post_execve(td, error, oldvmspace);
+	AUDIT_SYSCALL_EXIT(error == EJUSTRETURN ? 0 : error, td);
 	return (error);
 }
 
@@ -252,6 +253,7 @@ sys_fexecve(struct thread *td, struct fexecve_args *ua
 		error = kern_execve(td, &args, NULL, oldvmspace);
 	}
 	post_execve(td, error, oldvmspace);
+	AUDIT_SYSCALL_EXIT(error == EJUSTRETURN ? 0 : error, td);
 	return (error);
 }
 
@@ -280,6 +282,7 @@ sys___mac_execve(struct thread *td, struct __mac_execv
 	if (error == 0)
 		error = kern_execve(td, &args, uap->mac_p, oldvmspace);
 	post_execve(td, error, oldvmspace);
+	AUDIT_SYSCALL_EXIT(error == EJUSTRETURN ? 0 : error, td);
 	return (error);
 #else
 	return (ENOSYS);

Modified: head/sys/kern/subr_syscall.c
==============================================================================
--- head/sys/kern/subr_syscall.c	Sat Oct 24 14:25:38 2020	(r367001)
+++ head/sys/kern/subr_syscall.c	Sat Oct 24 14:39:17 2020	(r367002)
@@ -154,7 +154,18 @@ syscallenter(struct thread *td)
 			td->td_pflags &= ~TDP_NERRNO;
 		else
 			td->td_errno = error;
+
+		/*
+		 * Note that some syscall implementations (e.g., sys_execve)
+		 * will commit the audit record just before their final return.
+		 * These were done under the assumption that nothing of interest
+		 * would happen between their return and here, where we would
+		 * normally commit the audit record.  These assumptions will
+		 * need to be revisited should any substantial logic be added
+		 * above.
+		 */
 		AUDIT_SYSCALL_EXIT(error, td);
+
 #ifdef KDTRACE_HOOKS
 		/* Give the syscall:::return DTrace probe a chance to fire. */
 		if (__predict_false(sa->callp->sy_return != 0))