Re: [PATCH v2 0/4] Increase mseal test coverage

[Jeff Xu <jeffxu@chromium.org>]

Hi Pedro

On Fri, Aug 30, 2024 at 5:31 AM Pedro Falcato <pedro.falcato@gmail.com> wrote:
>
> On Thu, Aug 29, 2024 at 09:43:48PM GMT, jeffxu@chromium.org wrote:
> > From: Jeff Xu <jeffxu@chromium.org>
> >
> > This series increase the test coverage of mseal_test by:
> >
> > Add check for vma_size, prot, and error code for existing tests.
> > Add more testcases for madvise, munmap, mmap and mremap to cover
> > sealing in different scenarios.
> >
> > The increase test coverage hopefully help to prevent future regression.
> > It doesn't change any existing mm api's semantics, i.e. it will pass on
> > linux main and 6.10 branch.
>
> I do want to be clear that we shouldn't confuse "test coverage" with being unequivocally good
> if it has the possibility to paint ourselves into an API corner where details that should be left
> unspecified are instead set in stone (e.g do we want to test how mprotect behaves if it finds an msealed
> vma midway? no, apart from the property that really matters in this case (that sealed vmas remain untouched)).
>
I do not disagree with this. Let's look through code and comment on
the case directly if there is such a case.

Thanks.
-Jeff

> >
> > Note: in order to pass this test in mm-unstable, mm-unstable must have
> > Liam's fix on mmap [1]
> >
> > [1] https://lore.kernel.org/linux-kselftest/vyllxuh5xbqmaoyl2mselebij5ox7cseekjcvl5gmzoxxwd2he@hxi4mpjanxzt/#t
> >
> > History:
> > V2:
> > - remove the mmap fix (Liam R. Howlett will fix it separately)
> > - Add cover letter (Lorenzo Stoakes)
> > - split the testcase for ease of review (Mark Brown)
> >
> > V1:
> > - https://lore.kernel.org/linux-kselftest/20240828225522.684774-1-jeffxu@chromium.org/
> >
> > Jeff Xu (4):
> >   selftests/mm: mseal_test, add vma size check
> >   selftests/mm: mseal_test add sealed madvise type
> >   selftests/mm: mseal_test add more tests for mmap
> >   selftests/mm: mseal_test add more tests for mremap
> >
>
> nit: Please follow a more standard commit naming scheme like
>         selftests/mm: <change description>
> or
>         selftests/mseal: <change description>
>
> --
> Pedro