On 02/15/2017 07:10 PM, Kees Cook wrote: > On Mon, Feb 13, 2017 at 7:55 PM, Tyler Hicks <[email protected]> wrote: >> diff --git a/kernel/seccomp.c b/kernel/seccomp.c >> index e36dfe9..270a227 100644 >> --- a/kernel/seccomp.c >> +++ b/kernel/seccomp.c >> @@ -509,6 +509,22 @@ static void seccomp_send_sigsys(int syscall, int reason) >> } >> #endif /* CONFIG_SECCOMP_FILTER */ >> >> +static u32 seccomp_log_max_action = SECCOMP_RET_KILL; >> + >> +static inline void seccomp_log(unsigned long syscall, long signr, u32 >> action) >> +{ >> + /* Force an audit message to be emitted when the action is not >> greater >> + * than the configured maximum action. >> + */ >> + if (action <= seccomp_log_max_action) >> + 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. >> + */ > > Nitpick on comment style, please use: > > /* > * line 1 > * line 2... > */
No problem.
>
>> + return audit_seccomp(syscall, signr, action);
>> +}
>> +
>> /*
>> * Secure computing mode 1 allows only read/write/exit/sigreturn.
>> * To be fully secure this must be combined with rlimit
>> @@ -534,7 +550,7 @@ static void __secure_computing_strict(int this_syscall)
>> #ifdef SECCOMP_DEBUG
>> dump_stack();
>> #endif
>> - audit_seccomp(this_syscall, SIGKILL, SECCOMP_RET_KILL);
>> + seccomp_log(this_syscall, SIGKILL, SECCOMP_RET_KILL);
>> do_exit(SIGKILL);
>> }
>>
>> @@ -633,18 +649,30 @@ static int __seccomp_filter(int this_syscall, const
>> struct seccomp_data *sd,
>> return 0;
>>
>> case SECCOMP_RET_ALLOW:
>> + /* Open-coded seccomp_log(), optimized for the RET_ALLOW hot
>> + * path.
>> + *
>> + * We only want to log RET_ALLOW actions when the admin has
>> + * configured them to be logged via the log_max_action
>> sysctl.
>> + * Therefore, call __audit_seccomp() directly so that
>> RET_ALLOW
>> + * actions are not audited simply because the task is being
>> + * audited.
>> + */
>> + if (unlikely(seccomp_log_max_action == SECCOMP_RET_ALLOW))
>> + __audit_seccomp(this_syscall, 0, action);
>> +
>> return 0;
>>
>> case SECCOMP_RET_KILL:
>> default:
>> - audit_seccomp(this_syscall, SIGSYS, action);
>> + seccomp_log(this_syscall, SIGSYS, action);
>> do_exit(SIGSYS);
>> }
>>
>> unreachable();
>>
>> skip:
>> - audit_seccomp(this_syscall, 0, action);
>> + seccomp_log(this_syscall, 0, action);
>> return -1;
>> }
>> #else
>> @@ -917,12 +945,96 @@ long seccomp_get_filter(struct task_struct *task,
>> unsigned long filter_off,
>> #define SECCOMP_RET_TRACE_NAME "trace"
>> #define SECCOMP_RET_ALLOW_NAME "allow"
>>
>> +/* Largest strlen() of all action names */
>> +#define SECCOMP_RET_MAX_NAME_LEN 5
>> +
>> static char seccomp_actions_avail[] = SECCOMP_RET_KILL_NAME " "
>> SECCOMP_RET_TRAP_NAME " "
>> SECCOMP_RET_ERRNO_NAME " "
>> SECCOMP_RET_TRACE_NAME " "
>> SECCOMP_RET_ALLOW_NAME;
>>
>> +struct seccomp_action_name {
>> + u32 action;
>> + const char *name;
>> +};
>> +
>> +static struct seccomp_action_name seccomp_action_names[] = {
>
> As long as I'm nit-picking, this can be const too. :)
I'll have to cast to a non-const pointer when assigning ctl_table.data
but I think that's fine in this case.
Tyler
>
> -Kees
>
signature.asc
Description: OpenPGP digital signature
