[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 7 Aug 2017 18:46:03 -0700
From: Kees Cook <keescook@...omium.org>
To: Tyler Hicks <tyhicks@...onical.com>
Cc: LKML <linux-kernel@...r.kernel.org>,
Fabricio Voznika <fvoznika@...gle.com>,
Andy Lutomirski <luto@...capital.net>,
Will Drewry <wad@...omium.org>, Shuah Khan <shuah@...nel.org>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@...r.kernel.org>,
linux-security-module <linux-security-module@...r.kernel.org>
Subject: Re: [PATCH 1/4] seccomp: Provide matching filter for introspection
On Mon, Aug 7, 2017 at 6:03 PM, Tyler Hicks <tyhicks@...onical.com> wrote:
>> -static u32 seccomp_run_filters(const struct seccomp_data *sd)
>> +static u32 seccomp_run_filters(const struct seccomp_data *sd,
>> + struct seccomp_filter **match)
>> {
>> struct seccomp_data sd_local;
>> u32 ret = SECCOMP_RET_ALLOW;
>
> My version of this patch initialized *match to f here. The reason I did
> that is because if BPF_PROG_RUN() returns RET_ALLOW for all
> filters, I didn't want *match to remain NULL when seccomp_run_filters()
> returns. FILTER_FLAG_LOG nor FILTER_FLAG_KILL_PROCESS would be affected
> by this because they don't care about RET_ALLOW actions but there could
> conceivably be a filter flag in the future that cares about RET_ALLOW
> and not initializing *match to the first filter could result in a latent
> bug for that filter flag.
Very true, yes. I did intentionally adjust this because I wanted to
keep the hot path as untouched as possible.
> I'm fine with not adding the initialization since this is a hot path and
> it doesn't help any of the currently existing/planned filter flags but I
> wanted to at least mention it.
Yeah, and while I doubt I'll want to ever check "match" for RET_ALLOW,
I'll add a big comment there to explain it.
> Reviewed-by: Tyler Hicks <tyhicks@...onical.com>
Thanks!
-Kees
--
Kees Cook
Pixel Security
Powered by blists - more mailing lists