On Thu, Oct 1, 2020 at 4:05 PM Kees Cook <keesc...@chromium.org> wrote: > Right, but we depend on that test always doing the correct thing (and > continuing to do so into the future). I'm looking at this from the > perspective of future changes, maintenance, etc. I want the actions to > match the design principles as closely as possible so that future > evolutions of the code have lower risk to bugs causing security > failures. Right now, the code is simple. I want to design this so that > when it is complex, it will still fail toward safety in the face of > bugs. > > I'd prefer this way because for the loop, the tests, and the results only > make the bitmap more restrictive. The worst thing a bug in here can do is > leave the bitmap unchanged (which is certainly bad), but it can't _undo_ > an earlier restriction. > > The proposed loop's leading test_bit() becomes only an optimization, > rather than being required for policy enforcement. > > In other words, I prefer: > > inherit all prior prior bitmap restrictions > for all syscalls > if this filter not restricted > continue > set bitmap restricted > > within this loop (where the bulk of future logic may get added), > the worse-case future bug-induced failure mode for the syscall > bitmap is "skip *this* filter". > > > Instead of: > > set bitmap all restricted > for all syscalls > if previous bitmap not restricted and > filter not restricted > set bitmap unrestricted > > within this loop the worst-case future bug-induced failure mode > for the syscall bitmap is "skip *all* filters". > > > > > Or, to reword again, this: > > retain restrictions from previous caching decisions > for all syscalls > [evaluate this filter, maybe continue] > set restricted > > instead of: > > set new cache to all restricted > for all syscalls > [evaluate prior cache and this filter, maybe continue] > set unrestricted > > I expect the future code changes for caching to be in the "evaluate" > step, so I'd like the code designed to make things MORE restrictive not > less from the start, and remove any prior cache state tests from the > loop. > > At the end of the day I believe changing the design like this now lays > the groundwork to the caching mechanism being more robust against having > future bugs introduce security flaws. >
I see. Makes sense. Thanks. Will do that in the v4 YiFei Zhu
