Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values

[Steve Grubb <sgrubb@redhat.com>]

On Monday, January 2, 2017 4:53:10 PM EST Tyler Hicks wrote:
> Generate audit records for SECCOMP_RET_ERRNO actions, which were
> previously not audited.
> 
> Additionally, include the errno value that will be set in the audit
> message.
> 
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> ---
>  include/linux/audit.h | 19 ++++++++++++++++++-
>  kernel/auditsc.c      |  3 +++
>  kernel/seccomp.c      |  4 +++-
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 8c588c3..6815812 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -87,7 +87,10 @@ struct audit_field {
> 
>  struct audit_seccomp_info {
>  	int		code;
> -	long		signr;
> +	union {
> +		int	errno;
> +		long	signr;
> +	};
>  };
> 
>  extern int is_audit_feature_set(int which);
> @@ -319,6 +322,20 @@ static inline void audit_inode_child(struct inode
> *parent, }
>  void audit_core_dumps(long signr);
> 
> +static inline void audit_seccomp_errno(unsigned long syscall, int errno,
> +				       int code)
> +{
> +	if (!audit_enabled)
> +		return;
> +
> +	if (errno || unlikely(!audit_dummy_context())) {
> +		struct audit_seccomp_info info = { .code = code,
> +						   .errno = errno };
> +
> +		__audit_seccomp(syscall, &info);
> +	}
> +}
> +
>  static inline void audit_seccomp_signal(unsigned long syscall, long signr,
>  					int code)
>  {
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index b3472f2..db5fc9d 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2426,6 +2426,9 @@ void __audit_seccomp(unsigned long syscall, struct
> audit_seccomp_info *info) audit_log_task(ab);
> 
>  	switch (info->code) {
> +	case SECCOMP_RET_ERRNO:
> +		audit_log_format(ab, " errno=%d", info->errno);
> +		break;

"exit" is the field name that syscalls use to return errno to user space. I'd
rather not see another field created that maps to the same thing. You can check
the translation with the auformat utility:

http://people.redhat.com/sgrubb/files/auformat.tar.gz

$ ausearch --start today --just-one -m syscall -sv no --raw | ./auformat "%EXIT\n"

Also, I am working to normalize all the records. That mean every event record
of the same type has the same fields, in the same order, with the same
representation. I would think "exit" could be added to the current record after
syscall so that its ordered similarly to a syscall record.

-Steve

>  	case SECCOMP_RET_KILL:
>  		audit_log_format(ab, " sig=%ld", info->signr);
>  		break;
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 54c01b6..e99c566 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -576,9 +576,11 @@ static int __seccomp_filter(int this_syscall, const
> struct seccomp_data *sd, /* Set low-order bits as an errno, capped at
> MAX_ERRNO. */
>  		if (data > MAX_ERRNO)
>  			data = MAX_ERRNO;
> +
> +		audit_seccomp_errno(this_syscall, data, action);
>  		syscall_set_return_value(current, task_pt_regs(current),
>  					 -data, 0);
> -		goto skip;
> +		return -1;
> 
>  	case SECCOMP_RET_TRAP:
>  		/* Show the handler the original registers. */