Re: [PATCH 4/7] mm/mremap: Replace can_modify_mm with can_modify_vma

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 6, 2024 at 5:59 PM Pedro Falcato <pedro.falcato@xxxxxxxxx> wrote:
>
> On Wed, Aug 7, 2024 at 12:09 AM Jeff Xu <jeffxu@xxxxxxxxxx> wrote:
> >
> > On Tue, Aug 6, 2024 at 2:28 PM Pedro Falcato <pedro.falcato@xxxxxxxxx> wrote:
> > >
> > > Delegate all can_modify checks to the proper places. Unmap checks are
> > > done in do_unmap (et al).
> > >
> > > This patch allows for mremap partial failure in certain cases (for
> > > instance, when destination VMAs aren't sealed, but the source VMA is).
> > > It shouldn't be too troublesome, as you'd need to go out of your way to
> > > do illegal operations on a VMA.
> > >
> > > Signed-off-by: Pedro Falcato <pedro.falcato@xxxxxxxxx>
> > > ---
> > >  mm/mremap.c | 33 +++++++--------------------------
> > >  1 file changed, 7 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/mm/mremap.c b/mm/mremap.c
> > > index e7ae140fc64..8af877d7bb0 100644
> > > --- a/mm/mremap.c
> > > +++ b/mm/mremap.c
> > > @@ -676,6 +676,9 @@ static unsigned long move_vma(struct vm_area_struct *vma,
> > >         if (unlikely(flags & MREMAP_DONTUNMAP))
> > >                 to_account = new_len;
> > >
> > > +       if (!can_modify_vma(vma))
> > > +               return -EPERM;
> > > +
> > I m not 100% sure, but I suspect you don't need this check? Is
> > vma_to_resize already checking the src address ?
>
> Hmm, yes, good point.
>
> >
> > PS. Is it possible to consolidate all the related changes (except the
> > fix for madvise) to a single commit ?
>
> The patch set is organized logically, in simple clear steps. As a
> maintainer, I would prefer to review something like this vs a big
> confusing patch that touches many things at once.
> Of course if the maintainers think this is too coarse (it's not
> exactly a large patch set, just moves a lot of code back and forth),
> I'm happy to merge these into larger chunks.
>
Yes. The patch is not exactly large. (The original mseal patch  has
one single patch for adding mseal code logic, and one for test).

Also the mseal has been backported to  the 6.6 kernel for ChromeOS,
if you keep the code change in one , it will make future backport
easier for me.

> >  It would be easier to look for dependency, e.g. the remap depends on munmap().
>
> All patches depend on the previous (and a single patch would make it
> harder to see these dependencies). The kernel should build and the
> selftests should pass for every patch in the set.
>
> >
> > Also selftest is helpful to prove the correctness of the change. (And
> > I can also test it)
>
> I have run selftests, and it is.
>
> --
> Pedro





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux