On 2017-01-02 12:20:53, Steve Grubb wrote:
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(a)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:
Thanks for having a look at the field name I was using. Although I
prefer "errno" over "exit" in terms of clarity, I agree that it makes
sense to be consistent with the field names across record types. "exit"
works for me.
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.
This patch goes against your normalization efforts in more ways than
just the placement of the "exit" field. If the action is
SECCOMP_RET_KILL, a "sig" field is present but if the action is
SECCOMP_RET_ERRNO, the "sig" field will not be present but the
"errno"
field will be present. This happens all within the AUDIT_SECCOMP record
type. How would you suggest normalizing AUDIT_SECCOMP records for
different seccomp return actions?
Tyler
-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. */