* Jeff Xu <jeffxu@xxxxxxxxxxxx> [240202 12:24]: ... > > Provide code that uses this feature. Please do this too :) > > > > Provide benchmark results where you apply mseal to 1, 2, 4, 8, 16, and > > 32 VMAs. > > > I will prepare for the benchmark tests. Thank you, please also include runs of calls that you are modifying for checking for mseal() as we are adding loops there. > > > Provide code that tests and checks the failure paths. Failures at the > > start, middle, and end of the modifications. > > > Regarding, "Failures at the start, middle, and end of the modifications." > > With the current implementation, e.g. it checks if the sealing is > applied before actual modification of VMAs, so partial modifications > are avoided in mprotect, mremap, munmap. > > There are test cases in the selftests to cover the failure path, > including the beginning, middle and end of VMAs. > test_seal_unmapped_start > test_seal_unmapped_middle > test_seal_unmapped_end > test_seal_invalid_input > test_seal_start_mprotect > test_seal_end_mprotect > etc. > > Are those what you are looking for ? Those are certainly good, but we need more checking in there. You have a seal_split test that splits the vma by mseal but you don't check the flags on the VMAs. What I'm more concerned about is what happens if you call mseal() on a range and it can mseal a portion. Like, what happens to the first vma in your test_seal_unmapped_middle case? I see it returns an error, but is the first VMA mseal()'ed? (no it's not, but test that) What about the other system calls that will be denied on an mseal() VMA? Do they still behave the same? do_mprotect_pkey() will break out of the loop on the first error it sees - but it has modified some VMAs up to that point, I believe? You have changed this to abort before anything is modified. This is probably acceptable because it won't affect existing applications unless they start using mseal(), but that's just my opinion. It would be good to state the change in behaviour because it is changing the fundamental model of changing mprotect/madvise until an issue is hit. I think you are covering this by "it blocks X" but it's doing more than, say, a flag verification. One could reasonably assume this is just another flag verification. > > > Document what happens in those failure paths. I'd like to know how this affects other system calls in the partial success cases/return error cases. Some will now return new error codes and some may change the behaviour. It may even be okay to allow munmap() to split VMAs at the start/end of the region and fail to munmap because some VMA in the middle is mseal()'ed - but maybe not? I haven't put a whole lot of thought into it. Thanks, Liam
