On Fri, Apr 27, 2018 at 3:16 PM, Tyler Hicks <tyhicks@xxxxxxxxxxxxx> wrote:
> Seccomp logging for "handled" actions such as RET_TRAP, RET_TRACE, or
> RET_ERRNO can be very noisy for processes that are being audited. This
> patch modifies the seccomp logging behavior to treat processes that are
> being inspected via the audit subsystem the same as processes that
> aren't under inspection. Handled actions will no longer be logged just
> because the process is being inspected. Since v4.14, applications have
> the ability to request logging of handled actions by using the
> SECCOMP_FILTER_FLAG_LOG flag when loading seccomp filters.
>
> With this patch, the logic for deciding if an action will be logged is:
>
> if action == RET_ALLOW:
> do not log
> else if action not in actions_logged:
> do not log
> else if action == RET_KILL:
> log
> else if action == RET_LOG:
> log
> else if filter-requests-logging:
> log
> else:
> do not log
>
> Reported-by: Steve Grubb <sgrubb@xxxxxxxxxx>
> Signed-off-by: Tyler Hicks <tyhicks@xxxxxxxxxxxxx>
> ---
> Documentation/userspace-api/seccomp_filter.rst | 7 -------
> include/linux/audit.h | 10 +---------
> kernel/auditsc.c | 2 +-
> kernel/seccomp.c | 15 +++++----------
> 4 files changed, 7 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/userspace-api/seccomp_filter.rst b/Documentation/userspace-api/seccomp_filter.rst
> index 099c412..82a468b 100644
> --- a/Documentation/userspace-api/seccomp_filter.rst
> +++ b/Documentation/userspace-api/seccomp_filter.rst
> @@ -207,13 +207,6 @@ directory. Here's a description of each file in that directory:
> to the file do not need to be in ordered form but reads from the file
> will be ordered in the same way as the actions_avail sysctl.
>
> - It is important to note that the value of ``actions_logged`` does not
> - prevent certain actions from being logged when the audit subsystem is
> - configured to audit a task. If the action is not found in
> - ``actions_logged`` list, the final decision on whether to audit the
> - action for that task is ultimately left up to the audit subsystem to
> - decide for all seccomp return values other than ``SECCOMP_RET_ALLOW``.
> -
> The ``allow`` string is not accepted in the ``actions_logged`` sysctl
> as it is not possible to log ``SECCOMP_RET_ALLOW`` actions. Attempting
> to write ``allow`` to the sysctl will result in an EINVAL being
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index b311d7d..1964fbd 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -232,7 +232,7 @@ extern void __audit_file(const struct file *);
> extern void __audit_inode_child(struct inode *parent,
> const struct dentry *dentry,
> const unsigned char type);
> -extern void __audit_seccomp(unsigned long syscall, long signr, int code);
> +extern void audit_seccomp(unsigned long syscall, long signr, int code);
> extern void audit_seccomp_actions_logged(const char *names, int res);
> extern void __audit_ptrace(struct task_struct *t);
>
> @@ -303,12 +303,6 @@ static inline void audit_inode_child(struct inode *parent,
> }
> void audit_core_dumps(long signr);
>
> -static inline void audit_seccomp(unsigned long syscall, long signr, int code)
> -{
> - if (audit_enabled && unlikely(!audit_dummy_context()))
> - __audit_seccomp(syscall, signr, code);
> -}
> -
> static inline void audit_ptrace(struct task_struct *t)
> {
> if (unlikely(!audit_dummy_context()))
> @@ -499,8 +493,6 @@ static inline void audit_inode_child(struct inode *parent,
> { }
> static inline void audit_core_dumps(long signr)
> { }
> -static inline void __audit_seccomp(unsigned long syscall, long signr, int code)
> -{ }
> static inline void audit_seccomp(unsigned long syscall, long signr, int code)
> { }
> static inline void audit_seccomp_actions_logged(const char *names, int res)
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 3496238..1e64b91 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2464,7 +2464,7 @@ void audit_core_dumps(long signr)
> audit_log_end(ab);
> }
>
> -void __audit_seccomp(unsigned long syscall, long signr, int code)
> +void audit_seccomp(unsigned long syscall, long signr, int code)
> {
> struct audit_buffer *ab;
Since it is a bit unusual, it might be nice to add a comment at the
top of audit_seccomp() that the event filtering is being done in the
seccomp_log() function, and we may need to force auditing independent
of the audit_enabled and dummy context state.
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index e28ddcc..947cc0f 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -584,18 +584,13 @@ static inline void seccomp_log(unsigned long syscall, long signr, u32 action,
> }
>
> /*
> - * Force an audit message to be emitted when the action is RET_KILL_*,
> - * RET_LOG, or the FILTER_FLAG_LOG bit was set and the action is
> - * allowed to be logged by the admin.
> + * Emit an audit message when the action is RET_KILL_*, RET_LOG, or the
> + * FILTER_FLAG_LOG bit was set. The admin has the ability to silence
> + * any action from being logged by removing the action name from the
> + * seccomp_actions_logged sysctl.
> */
> if (log)
> - return __audit_seccomp(syscall, signr, action);
> -
> - /*
> - * Let the audit subsystem decide if the action should be audited based
> - * on whether the current task itself is being audited.
> - */
> - return audit_seccomp(syscall, signr, action);
> + audit_seccomp(syscall, signr, action);
> }
>
> /*
> --
> 2.7.4
>
--
paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
