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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXu5jL_7JxG5HrOp-QNY5gpA3s41d0sNuCYeD1pY0UN-zShAA@mail.gmail.com>
Date:   Fri, 10 Feb 2017 16:05:02 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Tyler Hicks <tyhicks@...onical.com>
Cc:     Paul Moore <paul@...l-moore.com>, Eric Paris <eparis@...hat.com>,
        Andy Lutomirski <luto@...capital.net>,
        Will Drewry <wad@...omium.org>, linux-audit@...hat.com,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/4] seccomp: Add sysctl to configure actions that
 should be logged

On Fri, Feb 10, 2017 at 3:56 PM, Tyler Hicks <tyhicks@...onical.com> 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