From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933653AbdABRV4 (ORCPT ); Mon, 2 Jan 2017 12:21:56 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46574 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755854AbdABRUx (ORCPT ); Mon, 2 Jan 2017 12:20:53 -0500 From: Steve Grubb To: linux-audit@redhat.com Cc: Tyler Hicks , Paul Moore , Eric Paris , Kees Cook , Andy Lutomirski , Will Drewry , linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values Date: Mon, 02 Jan 2017 12:20:53 -0500 Message-ID: <5284369.V7krsaxZyN@x2> Organization: Red Hat User-Agent: KMail/5.3.3 (Linux/4.8.15-200.fc24.x86_64; KDE/5.27.0; x86_64; ; ) In-Reply-To: <1483375990-14948-3-git-send-email-tyhicks@canonical.com> References: <1483375990-14948-1-git-send-email-tyhicks@canonical.com> <1483375990-14948-3-git-send-email-tyhicks@canonical.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Mon, 02 Jan 2017 17:20:54 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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. */ From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steve Grubb Subject: Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values Date: Mon, 02 Jan 2017 12:20:53 -0500 Message-ID: <5284369.V7krsaxZyN@x2> References: <1483375990-14948-1-git-send-email-tyhicks@canonical.com> <1483375990-14948-3-git-send-email-tyhicks@canonical.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1483375990-14948-3-git-send-email-tyhicks@canonical.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: linux-audit@redhat.com Cc: Will Drewry , linux-kernel@vger.kernel.org, Andy Lutomirski List-Id: linux-audit@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 > --- > 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. */