linux-kernel - Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5284369.V7krsaxZyN@x2>
Date:   Mon, 02 Jan 2017 12:20:53 -0500
From:   Steve Grubb <sgrubb@...hat.com>
To:     linux-audit@...hat.com
Cc:     Tyler Hicks <tyhicks@...onical.com>,
        Paul Moore <paul@...l-moore.com>,
        Eric Paris <eparis@...hat.com>,
        Kees Cook <keescook@...omium.org>,
        Andy Lutomirski <luto@...capital.net>,
        Will Drewry <wad@...omium.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values

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@...onical.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. */