On Mon, Sep 21, 2020 at 12:47 PM Jann Horn <jannh@xxxxxxxxxx> wrote: > Is this actually necessary, or can we just bail out on any branch that > we can't statically resolve? I think after we do enumerate the arch numbers it would make much more sense. Since if there is a branch after arch number and syscall numbers are fixed we can assume that the return values will be different if one or the other case is followed. > Also: If it turns out that the time spent in seccomp_cache_prepare() > is measurable for large filters, a possible improvement would be to > keep track of the last syscall number for which the result would be > the same as for the current one, such that instead of evaluating the > filter for one instruction at a time, it would effectively be > evaluated for a range at a time. That should be pretty straightforward > to implement, I think. My concern was more of the possibly-exponential amount of time & memory needed to evaluate an adversarial filter containing full of unresolveable branches, hence the max pending states. If we never follow both branches then evaluation should not be much of a concern. > > + depends on SECCOMP > > + depends on SECCOMP_FILTER > > SECCOMP_FILTER already depends on SECCOMP, so the "depends on SECCOMP" > line is unnecessary. The reason that this is here is because of the looks in menuconfig. SECCOMP is the direct previous entry, so if this depends on SECCOMP then the config would be indented. Is this looks not worth keeping or is there some better way to do this? > > + help > > + Seccomp filters can potentially incur large overhead for each > > + system call. This can alleviate some of the overhead. > > + > > + If in doubt, select 'none'. > > This should not be in arch/x86. Other architectures, such as arm64, > should also be able to use this without extra work. In the initial RFC patch I only added to x86. I could add it to any arch that has seccomp filters. Though, I'm wondering, why is SECCOMP in the arch-specific Kconfigs? > I think we should probably just bail out if we see anything that's > BPF_ST/BPF_STX. I've never seen seccomp filters that actually use that > part of cBPF. > > But in case we do need this, maybe instead of using "2 +" for all > these things, the cBPF memory slots should be in a separate array. Ok I'll just bail. YiFei Zhu
