Re: [PATCH v2 2/4] seccomp: Add sysctl to configure actions that should be logged

[Kees Cook]

On Fri, Feb 10, 2017 at 3:56 PM, Tyler Hicks <tyhicks@xxxxxxxxxxxxx> wrote:
> On 02/07/2017 06:24 PM, Kees Cook wrote:
>> case SECCOMP_RET_ALLOW:
>>     /* Open-coded seccomp_log(), optimized for RET_ALLOW. */
>>     if (unlikely(seccomp_max_action_to_log == 0))
>>         __audit_seccomp(syscall, signr, action);
>>     return 0;
>
> That makes sense.

And, heh, reading it again now, my example should be ==
SECCOMP_RET_ALLOW (which is 0, yes, but eek raw number, bad me).

>>> +/* Largest strlen() of all action names */
>>> +#define SECCOMP_RET_MAX_NAME_LEN       5
>>
>> This feels fragile... though I don't have a good suggestion yet. :P
>
> I agree and I also don't have a good solution. I didn't like having to
> hard code it.

Yeah. Hrmpf. I mean, it could be sizeof(seccomp_actions_avail) ...
that'll always be long enough. :) But it's a bit of stack over-kill,
but ... is that so bad? I dunno.

>> In the hopes of some day making the sysctl table entirely read-only,
>> can you add some fancy crap here for me? :) See
>> security/yama/yama_lsm.c's yama_dointvec_minmax(), which uses a copy
>> of the sysctl table on the stack.
>
> Will do. I'll deviate slightly from yama_dointvec_minmax(). To make it
> clear that the ctl_table param shouldn't be modified, I'm going to name
> it ro_table and then the stack variable will be named table.

Sounds great, thanks!

-Kees

-- 
Kees Cook
Pixel Security