Re: [PATCH v8 0/4] Introduce mseal

[Jeff Xu <jeffxu@chromium.org>]

On Fri, Feb 2, 2024 at 7:13 AM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
>
> * Jeff Xu <jeffxu@google.com> [240201 22:15]:
> > On Thu, Feb 1, 2024 at 12:45 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > >
> > > * Jeff Xu <jeffxu@chromium.org> [240131 20:27]:
> > > > On Wed, Jan 31, 2024 at 11:34 AM Liam R. Howlett
> > > > <Liam.Howlett@oracle.com> wrote:
> > > > >
> > >
> > > Having to opt-in to allowing mseal will probably not work well.
> > I'm leaving the opt-in discussion in Linus's thread.
> >
> > > Initial library mappings happen in one huge chunk then it's cut up into
> > > smaller VMAs, at least that's what I see with my maple tree tracing.  If
> > > you opt-in, then the entire library will have to opt-in and so the
> > > 'discourage inadvertent sealing' argument is not very strong.
> > >
> > Regarding "The initial library mappings happen in one huge chunk then
> > it is cut up into smaller VMAS", this is not a problem.
> >
> > As example of elf loading (fs/binfmt_elf.c), there is just a few
> > places to pass in what type of memory to be allocated, e.g.
> > MAP_PRIVATE, MAP_FIXED_NOREPLACE, we can  add MAP_SEALABLE at those
> > places.
> > If glic does additional splitting on the memory range, by using
> > mprotect(), then the MAP_SEALABLE is automatically applied after
> > splitting.
> > If glic uses mmap(MAP_FIXED), then it should use mmap(MAP_FIXED|MAP_SEALABLE).
>
> You are adding a flag that requires a new glibc.  When I try to point
> out how this is unnecessary and excessive, you tell me it's fine and
> probably not a whole lot of work.
>
> This isn't working with developers, you are dismissing the developers
> who are trying to help you.
>
> Can you please:
>
> Provide code that uses this feature.
>
> Provide benchmark results where you apply mseal to 1, 2, 4, 8, 16, and
> 32 VMAs.
>
I will prepare for the benchmark tests.

> 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 ?

> Document what happens in those failure paths.
>
> And, most importantly: keep an open mind and allow your opinion to
> change when presented with new information.
>
> All of these things are to help you.  We need to know what needs fixing
> so you can be successful.
>
Thanks for those feedbacks.

I sincerely wish for more of those help so this syscall can be useful.

Thanks.
Best Regards,
-Jeff

>
> Thanks,
> Liam