On Fri, 27 Sep 2019, Kees Cook wrote:
On Wed, Aug 28, 2019 at 6:30 PM Paul Walmsley paul.walmsley@sifive.com wrote:
On Mon, 26 Aug 2019, Kees Cook wrote:
On Mon, Aug 26, 2019 at 09:39:50AM -0700, David Abdurachmanov wrote:
I don't have the a build with SECCOMP for the board right now, so it will have to wait. I just finished a new kernel (almost rc6) for Fedora,
FWIW, I don't think this should block landing the code: all the tests fail without seccomp support. ;) So this patch is an improvement!
Am sympathetic to this -- we did it with the hugetlb patches for RISC-V -- but it would be good to understand a little bit more about why the test fails before we merge it.
The test is almost certainly failing due to the environmental requirements (i.e. namespaces, user ids, etc). There are some corner cases in there that we've had to fix in the past. If the other tests are passing, then I would expect all the seccomp internals are fine -- it's just the case being weird. It's just a matter of figuring out what state the test environment is in so we can cover that corner case too.
Once we merge the patch, it will probably reduce the motivation for others to either understand and fix the underlying problem with the RISC-V code -- or, if it truly is a flaky test, to drop (or fix) the test in the seccomp_bpf kselftests.
Sure, I get that point -- but I don't want to block seccomp landing for riscv for that. I suggested to David offlist that the test could just be marked with a FIXME XFAIL on riscv and once someone's in a better position to reproduce it we can fix it. (I think the test bug is almost certainly not riscv specific, but just some missing requirement that we aren't handling correctly.)
OK. It might be nice to mark the seccomp_bpf.c test as flaky in the comments for the test.
How does that sound?
Let's follow your plan. Thanks for your review and feedback.
- Paul