Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1705952

Re: [PATCH 1/4] seccomp: Provide matching filter for introspection

From Kees Cook <keescook@chromium.org>
Newsgroups linux.kernel
Subject Re: [PATCH 1/4] seccomp: Provide matching filter for introspection
Date 2017-08-08 03:50 +0200
Message-ID <uc1OG-E4-11@gated-at.bofh.it> (permalink)
References <uaeQ1-2rm-3@gated-at.bofh.it> <uaeZJ-2uR-3@gated-at.bofh.it> <uc1bY-7m-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, Aug 7, 2017 at 6:03 PM, Tyler Hicks <tyhicks@canonical.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@canonical.com>

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/4] seccomp: Add SECCOMP_FILTER_FLAG_KILL_PROCESS Kees Cook <keescook@chromium.org> - 2017-08-03 05:20 +0200
  [PATCH 1/4] seccomp: Provide matching filter for introspection Kees Cook <keescook@chromium.org> - 2017-08-03 05:30 +0200
    Re: [PATCH 1/4] seccomp: Provide matching filter for introspection Tyler Hicks <tyhicks@canonical.com> - 2017-08-08 03:10 +0200
      Re: [PATCH 1/4] seccomp: Provide matching filter for introspection Tyler Hicks <tyhicks@canonical.com> - 2017-08-08 03:40 +0200
      Re: [PATCH 1/4] seccomp: Provide matching filter for introspection Kees Cook <keescook@chromium.org> - 2017-08-08 03:50 +0200
  [PATCH 3/4] selftests/seccomp: Refactor RET_ERRNO tests Kees Cook <keescook@chromium.org> - 2017-08-03 05:30 +0200
    Re: [PATCH 3/4] selftests/seccomp: Refactor RET_ERRNO tests Tyler Hicks <tyhicks@canonical.com> - 2017-08-08 03:30 +0200
  [PATCH 2/4] seccomp: Add SECCOMP_FILTER_FLAG_KILL_PROCESS Kees Cook <keescook@chromium.org> - 2017-08-03 05:30 +0200
    Re: [PATCH 2/4] seccomp: Add SECCOMP_FILTER_FLAG_KILL_PROCESS Tyler Hicks <tyhicks@canonical.com> - 2017-08-08 03:30 +0200
      Re: [PATCH 2/4] seccomp: Add SECCOMP_FILTER_FLAG_KILL_PROCESS Kees Cook <keescook@chromium.org> - 2017-08-08 04:00 +0200

csiph-web