Merge ~cascardo/qa-regression-testing:bpf into qa-regression-testing:master
Proposed by
Thadeu Lima de Souza Cascardo
Status: | Needs review |
---|---|
Proposed branch: | ~cascardo/qa-regression-testing:bpf |
Merge into: | qa-regression-testing:master |
Diff against target: |
407 lines (+385/-0) 3 files modified
scripts/kernel-security/capbpf/Makefile (+5/-0) scripts/kernel-security/capbpf/capbpf.c (+373/-0) scripts/test-kernel-security.py (+7/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Steve Beattie | Pending | ||
Review via email: mp+410245@code.staging.launchpad.net |
To post a comment you must log in.
There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.
Thanks for putting this together.
I'm pretty confused about the variety of tests; some of the function names don't seem to match what's in the function bodies, and some of the test functions are pretty short and easy to reason about, but some of them are quite long and seem to test five or six different things.
Can the various test_unprivileg ed(), test_only_ privileged( ), test_root_ unprivileged( ), test_root_ only_privileged (), functions be boiled down a bit?
I think I'd find it easier to read if there were a table showing desired outcomes, something like:
struct test {
uid_t uid,
bool cap_sys_admin,
bool cap_bpf,
int expected_result,
}
test t[] = {
{ 0, true, true, 0},
{ 0, true, false, -1},
{ 0, false, true, 0},
{ 0, false, false, -1},
{ 1000, true, true, 0},
{ 1000, true, false, -1},
{ 1000, false, true, 1},
{ 1000, false, false, -1}};
or something similar that we can easily inspect to make sure that eg all eight cases we care about are covered. (Or, however many cases we've actually got.)
The other thing is how far we go with this test to handle the time between when the syscall was introduced and before the capability was introduced, and how we'd handle that. Maybe the CAP_IS_SUPPORTED() macro is the right way to tell -- maybe it isn't! -- and maybe we need two tables like the above, one for the "before cap_bpf" and one for "after cap_bpf".
I hope something here is useful to you.
Thanks