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)
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.
Revision history for this message
Seth Arnold (seth-arnold) wrote :

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_unprivileged(), 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

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches