[PATCH 4/5] mm/mseal: separate out and simplify VMA gap check

Lorenzo Stoakes posted 5 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH 4/5] mm/mseal: separate out and simplify VMA gap check
Posted by Lorenzo Stoakes 2 months, 3 weeks ago
The check_mm_seal() function is doing something general - checking whether
a range contains only VMAs (or rather that it does NOT contain any unmapped
regions).

Generalise this and put the logic in mm/vma.c - introducing
range_contains_unmapped(). Additionally we can simplify the logic, we are
simply checking whether the last vma->vm_end has either a VMA starting
after it or ends before the end parameter.

No functional change intended.

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
 mm/mseal.c | 38 +++-----------------------------------
 mm/vma.c   | 18 ++++++++++++++++++
 mm/vma.h   |  3 +++
 3 files changed, 24 insertions(+), 35 deletions(-)

diff --git a/mm/mseal.c b/mm/mseal.c
index adbcc65e9660..8e4c605af700 100644
--- a/mm/mseal.c
+++ b/mm/mseal.c
@@ -37,34 +37,6 @@ static int mseal_fixup(struct vma_iterator *vmi, struct vm_area_struct *vma,
 	return ret;
 }

-/*
- * Check for do_mseal:
- * 1> start is part of a valid vma.
- * 2> end is part of a valid vma.
- * 3> No gap (unallocated address) between start and end.
- * 4> map is sealable.
- */
-static int check_mm_seal(unsigned long start, unsigned long end)
-{
-	struct vm_area_struct *vma;
-	unsigned long nstart = start;
-	VMA_ITERATOR(vmi, current->mm, start);
-
-	/* going through each vma to check. */
-	for_each_vma_range(vmi, vma, end) {
-		if (vma->vm_start > nstart)
-			/* unallocated memory found. */
-			return -ENOMEM;
-
-		if (vma->vm_end >= end)
-			return 0;
-
-		nstart = vma->vm_end;
-	}
-
-	return -ENOMEM;
-}
-
 /*
  * Apply sealing.
  */
@@ -184,14 +156,10 @@ int do_mseal(unsigned long start, size_t len_in, unsigned long flags)
 	if (mmap_write_lock_killable(mm))
 		return -EINTR;

-	/*
-	 * First pass, this helps to avoid
-	 * partial sealing in case of error in input address range,
-	 * e.g. ENOMEM error.
-	 */
-	ret = check_mm_seal(start, end);
-	if (ret)
+	if (range_contains_unmapped(mm, start, end)) {
+		ret = -ENOMEM;
 		goto out;
+	}

 	/*
 	 * Second pass, this should success, unless there are errors
diff --git a/mm/vma.c b/mm/vma.c
index b3d880652359..b57545568ae6 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -3203,3 +3203,21 @@ int insert_vm_struct(struct mm_struct *mm, struct vm_area_struct *vma)

 	return 0;
 }
+
+/* Does the [start, end) range contain any unmapped memory? */
+bool range_contains_unmapped(struct mm_struct *mm,
+		unsigned long start, unsigned long end)
+{
+	struct vm_area_struct *vma;
+	unsigned long prev_end = start;
+	VMA_ITERATOR(vmi, current->mm, start);
+
+	for_each_vma_range(vmi, vma, end) {
+		if (vma->vm_start > prev_end)
+			return true;
+
+		prev_end = vma->vm_end;
+	}
+
+	return prev_end < end;
+}
diff --git a/mm/vma.h b/mm/vma.h
index d17f560cf53d..bfe9a04e6018 100644
--- a/mm/vma.h
+++ b/mm/vma.h
@@ -598,4 +598,7 @@ int create_init_stack_vma(struct mm_struct *mm, struct vm_area_struct **vmap,
 int relocate_vma_down(struct vm_area_struct *vma, unsigned long shift);
 #endif

+bool range_contains_unmapped(struct mm_struct *mm,
+		unsigned long start, unsigned long end);
+
 #endif	/* __MM_VMA_H */
--
2.50.1
Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check
Posted by Liam R. Howlett 2 months, 3 weeks ago
* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250714 09:08]:
> The check_mm_seal() function is doing something general - checking whether
> a range contains only VMAs (or rather that it does NOT contain any unmapped
> regions).
> 
> Generalise this and put the logic in mm/vma.c - introducing
> range_contains_unmapped(). Additionally we can simplify the logic, we are
> simply checking whether the last vma->vm_end has either a VMA starting
> after it or ends before the end parameter.
> 
> No functional change intended.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

I do fear that people will find this function and try and use it
internally, it may make our jobs of avoiding this being expanded more
annoying.

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  mm/mseal.c | 38 +++-----------------------------------
>  mm/vma.c   | 18 ++++++++++++++++++
>  mm/vma.h   |  3 +++
>  3 files changed, 24 insertions(+), 35 deletions(-)
> 
> diff --git a/mm/mseal.c b/mm/mseal.c
> index adbcc65e9660..8e4c605af700 100644
> --- a/mm/mseal.c
> +++ b/mm/mseal.c
> @@ -37,34 +37,6 @@ static int mseal_fixup(struct vma_iterator *vmi, struct vm_area_struct *vma,
>  	return ret;
>  }
> 
> -/*
> - * Check for do_mseal:
> - * 1> start is part of a valid vma.
> - * 2> end is part of a valid vma.
> - * 3> No gap (unallocated address) between start and end.
> - * 4> map is sealable.
> - */
> -static int check_mm_seal(unsigned long start, unsigned long end)
> -{
> -	struct vm_area_struct *vma;
> -	unsigned long nstart = start;
> -	VMA_ITERATOR(vmi, current->mm, start);
> -
> -	/* going through each vma to check. */
> -	for_each_vma_range(vmi, vma, end) {
> -		if (vma->vm_start > nstart)
> -			/* unallocated memory found. */
> -			return -ENOMEM;
> -
> -		if (vma->vm_end >= end)
> -			return 0;
> -
> -		nstart = vma->vm_end;
> -	}
> -
> -	return -ENOMEM;
> -}
> -
>  /*
>   * Apply sealing.
>   */
> @@ -184,14 +156,10 @@ int do_mseal(unsigned long start, size_t len_in, unsigned long flags)
>  	if (mmap_write_lock_killable(mm))
>  		return -EINTR;
> 
> -	/*
> -	 * First pass, this helps to avoid
> -	 * partial sealing in case of error in input address range,
> -	 * e.g. ENOMEM error.
> -	 */
> -	ret = check_mm_seal(start, end);
> -	if (ret)
> +	if (range_contains_unmapped(mm, start, end)) {
> +		ret = -ENOMEM;
>  		goto out;
> +	}
> 
>  	/*
>  	 * Second pass, this should success, unless there are errors
> diff --git a/mm/vma.c b/mm/vma.c
> index b3d880652359..b57545568ae6 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -3203,3 +3203,21 @@ int insert_vm_struct(struct mm_struct *mm, struct vm_area_struct *vma)
> 
>  	return 0;
>  }
> +
> +/* Does the [start, end) range contain any unmapped memory? */
> +bool range_contains_unmapped(struct mm_struct *mm,
> +		unsigned long start, unsigned long end)
> +{
> +	struct vm_area_struct *vma;
> +	unsigned long prev_end = start;
> +	VMA_ITERATOR(vmi, current->mm, start);
> +
> +	for_each_vma_range(vmi, vma, end) {
> +		if (vma->vm_start > prev_end)
> +			return true;
> +
> +		prev_end = vma->vm_end;
> +	}
> +
> +	return prev_end < end;
> +}
> diff --git a/mm/vma.h b/mm/vma.h
> index d17f560cf53d..bfe9a04e6018 100644
> --- a/mm/vma.h
> +++ b/mm/vma.h
> @@ -598,4 +598,7 @@ int create_init_stack_vma(struct mm_struct *mm, struct vm_area_struct **vmap,
>  int relocate_vma_down(struct vm_area_struct *vma, unsigned long shift);
>  #endif
> 
> +bool range_contains_unmapped(struct mm_struct *mm,
> +		unsigned long start, unsigned long end);
> +
>  #endif	/* __MM_VMA_H */
> --
> 2.50.1
Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check
Posted by Lorenzo Stoakes 2 months, 3 weeks ago
On Mon, Jul 14, 2025 at 11:35:44AM -0400, Liam R. Howlett wrote:
> * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250714 09:08]:
> > The check_mm_seal() function is doing something general - checking whether
> > a range contains only VMAs (or rather that it does NOT contain any unmapped
> > regions).
> >
> > Generalise this and put the logic in mm/vma.c - introducing
> > range_contains_unmapped(). Additionally we can simplify the logic, we are
> > simply checking whether the last vma->vm_end has either a VMA starting
> > after it or ends before the end parameter.
> >
> > No functional change intended.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>
> I do fear that people will find this function and try and use it
> internally, it may make our jobs of avoiding this being expanded more
> annoying.

Hmm, surely we should have some ability to dissuade within mm :)

Thing is I don't love having this function in mm/mseal.c when it has
nothing to do with mseal()'ing.

If people want to be weird about gaps they can pretty trivially implement
something like this anyway. Probably. Possibly. Maybe?

>
> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

Thanks!

>
> > ---
> >  mm/mseal.c | 38 +++-----------------------------------
> >  mm/vma.c   | 18 ++++++++++++++++++
> >  mm/vma.h   |  3 +++
> >  3 files changed, 24 insertions(+), 35 deletions(-)
> >
> > diff --git a/mm/mseal.c b/mm/mseal.c
> > index adbcc65e9660..8e4c605af700 100644
> > --- a/mm/mseal.c
> > +++ b/mm/mseal.c
> > @@ -37,34 +37,6 @@ static int mseal_fixup(struct vma_iterator *vmi, struct vm_area_struct *vma,
> >  	return ret;
> >  }
> >
> > -/*
> > - * Check for do_mseal:
> > - * 1> start is part of a valid vma.
> > - * 2> end is part of a valid vma.
> > - * 3> No gap (unallocated address) between start and end.
> > - * 4> map is sealable.
> > - */
> > -static int check_mm_seal(unsigned long start, unsigned long end)
> > -{
> > -	struct vm_area_struct *vma;
> > -	unsigned long nstart = start;
> > -	VMA_ITERATOR(vmi, current->mm, start);
> > -
> > -	/* going through each vma to check. */
> > -	for_each_vma_range(vmi, vma, end) {
> > -		if (vma->vm_start > nstart)
> > -			/* unallocated memory found. */
> > -			return -ENOMEM;
> > -
> > -		if (vma->vm_end >= end)
> > -			return 0;
> > -
> > -		nstart = vma->vm_end;
> > -	}
> > -
> > -	return -ENOMEM;
> > -}
> > -
> >  /*
> >   * Apply sealing.
> >   */
> > @@ -184,14 +156,10 @@ int do_mseal(unsigned long start, size_t len_in, unsigned long flags)
> >  	if (mmap_write_lock_killable(mm))
> >  		return -EINTR;
> >
> > -	/*
> > -	 * First pass, this helps to avoid
> > -	 * partial sealing in case of error in input address range,
> > -	 * e.g. ENOMEM error.
> > -	 */
> > -	ret = check_mm_seal(start, end);
> > -	if (ret)
> > +	if (range_contains_unmapped(mm, start, end)) {
> > +		ret = -ENOMEM;
> >  		goto out;
> > +	}
> >
> >  	/*
> >  	 * Second pass, this should success, unless there are errors
> > diff --git a/mm/vma.c b/mm/vma.c
> > index b3d880652359..b57545568ae6 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -3203,3 +3203,21 @@ int insert_vm_struct(struct mm_struct *mm, struct vm_area_struct *vma)
> >
> >  	return 0;
> >  }
> > +
> > +/* Does the [start, end) range contain any unmapped memory? */
> > +bool range_contains_unmapped(struct mm_struct *mm,
> > +		unsigned long start, unsigned long end)
> > +{
> > +	struct vm_area_struct *vma;
> > +	unsigned long prev_end = start;
> > +	VMA_ITERATOR(vmi, current->mm, start);
> > +
> > +	for_each_vma_range(vmi, vma, end) {
> > +		if (vma->vm_start > prev_end)
> > +			return true;
> > +
> > +		prev_end = vma->vm_end;
> > +	}
> > +
> > +	return prev_end < end;
> > +}
> > diff --git a/mm/vma.h b/mm/vma.h
> > index d17f560cf53d..bfe9a04e6018 100644
> > --- a/mm/vma.h
> > +++ b/mm/vma.h
> > @@ -598,4 +598,7 @@ int create_init_stack_vma(struct mm_struct *mm, struct vm_area_struct **vmap,
> >  int relocate_vma_down(struct vm_area_struct *vma, unsigned long shift);
> >  #endif
> >
> > +bool range_contains_unmapped(struct mm_struct *mm,
> > +		unsigned long start, unsigned long end);
> > +
> >  #endif	/* __MM_VMA_H */
> > --
> > 2.50.1
Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check
Posted by Liam R. Howlett 2 months, 3 weeks ago
* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250714 11:40]:
> On Mon, Jul 14, 2025 at 11:35:44AM -0400, Liam R. Howlett wrote:
> > * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250714 09:08]:
> > > The check_mm_seal() function is doing something general - checking whether
> > > a range contains only VMAs (or rather that it does NOT contain any unmapped
> > > regions).
> > >
> > > Generalise this and put the logic in mm/vma.c - introducing
> > > range_contains_unmapped(). Additionally we can simplify the logic, we are
> > > simply checking whether the last vma->vm_end has either a VMA starting
> > > after it or ends before the end parameter.
> > >
> > > No functional change intended.
> > >
> > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> >
> > I do fear that people will find this function and try and use it
> > internally, it may make our jobs of avoiding this being expanded more
> > annoying.
> 
> Hmm, surely we should have some ability to dissuade within mm :)
> 
> Thing is I don't love having this function in mm/mseal.c when it has
> nothing to do with mseal()'ing.
> 
> If people want to be weird about gaps they can pretty trivially implement
> something like this anyway. Probably. Possibly. Maybe?

Yes, but the existence of a function legitimizes their thought prior to
sending it for review.  That is, seeing a function that already does it
makes okay to include the option in the planning.

> 
> >
> > Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
> 
> Thanks!
>
Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check
Posted by Lorenzo Stoakes 2 months, 3 weeks ago
On Mon, Jul 14, 2025 at 11:43:57AM -0400, Liam R. Howlett wrote:
> Yes, but the existence of a function legitimizes their thought prior to
> sending it for review.  That is, seeing a function that already does it
> makes okay to include the option in the planning.

True.

OK, based on what you're saying and what Pedro's saying, next respin/fixpatch
I"ll put this in mseal.c as a static function, even if it makes me cringe a bit
to do it.

And then Pedro can remove it in his series :)

For the record I don't agree with this check being performed... it seems silly
to me.
Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check
Posted by Pedro Falcato 2 months, 3 weeks ago
On Mon, Jul 14, 2025 at 02:00:39PM +0100, Lorenzo Stoakes wrote:
> The check_mm_seal() function is doing something general - checking whether
> a range contains only VMAs (or rather that it does NOT contain any unmapped
> regions).
> 
> Generalise this and put the logic in mm/vma.c - introducing
> range_contains_unmapped(). Additionally we can simplify the logic, we are
> simply checking whether the last vma->vm_end has either a VMA starting
> after it or ends before the end parameter.
>

I don't like this. Unless you have any other user for this in mind,
we'll proliferate this awful behavior (and add this into core vma code).

I have some patches locally to fully remove this upfront check, and AFAIK
we're somewhat in agreement that we can simply nuke this check (for
various reasons, including that we *still* don't have a man page for the
syscall). I can send them for proper discussion after your series lands.

-- 
Pedro
Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check
Posted by Lorenzo Stoakes 2 months, 3 weeks ago
On Mon, Jul 14, 2025 at 04:17:23PM +0100, Pedro Falcato wrote:
> On Mon, Jul 14, 2025 at 02:00:39PM +0100, Lorenzo Stoakes wrote:
> > The check_mm_seal() function is doing something general - checking whether
> > a range contains only VMAs (or rather that it does NOT contain any unmapped
> > regions).
> >
> > Generalise this and put the logic in mm/vma.c - introducing
> > range_contains_unmapped(). Additionally we can simplify the logic, we are
> > simply checking whether the last vma->vm_end has either a VMA starting
> > after it or ends before the end parameter.
> >
>
> I don't like this. Unless you have any other user for this in mind,
> we'll proliferate this awful behavior (and add this into core vma code).

I'm not sure how putting it in an internal-only mm file perpetuates
anything.

I'm naming the function by what it does, and putting it where it belongs in
the VMA logic, and additionally making the function less horrible.

Let's not please get stuck on the isues with mseal implementation which
will catch-22 us into not being able to refactor.

We can do the refactoring first and it's fine to just yank this if it's not
used.

I'm not having a function like this sat in mm/mseal.c when it has
absolutely nothing to do with mseal specifically though.

>
> I have some patches locally to fully remove this upfront check, and AFAIK
> we're somewhat in agreement that we can simply nuke this check (for
> various reasons, including that we *still* don't have a man page for the
> syscall). I can send them for proper discussion after your series lands.

Yes I agree this check is odd, I don't really see why on earth we're
concerned with whether there are gaps or not, you'd surely want to just
seal whatever VMAs exist in the range?

But this belongs as something separate (a _functional_ change), let's get
the code sane first.

And ack on manpage, that's a horrible oversight that needs addressing!

>
> --
> Pedro
Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check
Posted by David Hildenbrand 2 months, 3 weeks ago
On 14.07.25 17:23, Lorenzo Stoakes wrote:
> On Mon, Jul 14, 2025 at 04:17:23PM +0100, Pedro Falcato wrote:
>> On Mon, Jul 14, 2025 at 02:00:39PM +0100, Lorenzo Stoakes wrote:
>>> The check_mm_seal() function is doing something general - checking whether
>>> a range contains only VMAs (or rather that it does NOT contain any unmapped
>>> regions).
>>>
>>> Generalise this and put the logic in mm/vma.c - introducing
>>> range_contains_unmapped(). Additionally we can simplify the logic, we are
>>> simply checking whether the last vma->vm_end has either a VMA starting
>>> after it or ends before the end parameter.
>>>
>>
>> I don't like this. Unless you have any other user for this in mind,
>> we'll proliferate this awful behavior (and add this into core vma code).
> 
> I'm not sure how putting it in an internal-only mm file perpetuates
> anything.
> 
> I'm naming the function by what it does, and putting it where it belongs in
> the VMA logic, and additionally making the function less horrible.
> 
> Let's not please get stuck on the isues with mseal implementation which
> will catch-22 us into not being able to refactor.
> 
> We can do the refactoring first and it's fine to just yank this if it's not
> used.
> 
> I'm not having a function like this sat in mm/mseal.c when it has
> absolutely nothing to do with mseal specifically though.
> 
>>
>> I have some patches locally to fully remove this upfront check, and AFAIK
>> we're somewhat in agreement that we can simply nuke this check (for
>> various reasons, including that we *still* don't have a man page for the
>> syscall). I can send them for proper discussion after your series lands.
> 
> Yes I agree this check is odd, I don't really see why on earth we're
> concerned with whether there are gaps or not, you'd surely want to just
> seal whatever VMAs exist in the range?

Probably because GAPs cannot be sealed. So user space could assume that 
in fact, nothing in that area can change after a successful mseal, while 
it can ...

Not sure, though ...

-- 
Cheers,

David / dhildenb
Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check
Posted by Lorenzo Stoakes 2 months, 3 weeks ago
On Mon, Jul 14, 2025 at 05:25:59PM +0200, David Hildenbrand wrote:
> On 14.07.25 17:23, Lorenzo Stoakes wrote:
> > On Mon, Jul 14, 2025 at 04:17:23PM +0100, Pedro Falcato wrote:
> > > On Mon, Jul 14, 2025 at 02:00:39PM +0100, Lorenzo Stoakes wrote:
> > > > The check_mm_seal() function is doing something general - checking whether
> > > > a range contains only VMAs (or rather that it does NOT contain any unmapped
> > > > regions).
> > > >
> > > > Generalise this and put the logic in mm/vma.c - introducing
> > > > range_contains_unmapped(). Additionally we can simplify the logic, we are
> > > > simply checking whether the last vma->vm_end has either a VMA starting
> > > > after it or ends before the end parameter.
> > > >
> > >
> > > I don't like this. Unless you have any other user for this in mind,
> > > we'll proliferate this awful behavior (and add this into core vma code).
> >
> > I'm not sure how putting it in an internal-only mm file perpetuates
> > anything.
> >
> > I'm naming the function by what it does, and putting it where it belongs in
> > the VMA logic, and additionally making the function less horrible.
> >
> > Let's not please get stuck on the isues with mseal implementation which
> > will catch-22 us into not being able to refactor.
> >
> > We can do the refactoring first and it's fine to just yank this if it's not
> > used.
> >
> > I'm not having a function like this sat in mm/mseal.c when it has
> > absolutely nothing to do with mseal specifically though.
> >
> > >
> > > I have some patches locally to fully remove this upfront check, and AFAIK
> > > we're somewhat in agreement that we can simply nuke this check (for
> > > various reasons, including that we *still* don't have a man page for the
> > > syscall). I can send them for proper discussion after your series lands.
> >
> > Yes I agree this check is odd, I don't really see why on earth we're
> > concerned with whether there are gaps or not, you'd surely want to just
> > seal whatever VMAs exist in the range?
>
> Probably because GAPs cannot be sealed. So user space could assume that in
> fact, nothing in that area can change after a successful mseal, while it can
> ...
>
> Not sure, though ...

Yeah maybe a sekuriteh thing where you want to be sure the range is in fact
_all_ sealed.

I'm not sure that really makes much sense in practice honestly though, because
if another thread can fiddle with things, then surely you've already 'lost'.

if you expected to touch a VMA where in fact aa gap exists your software is
_already_ broken because you're going to segfault.

So it just seems overly theoretical to me.

I think we should error out if there's _no_ VMAs at all, but otherwise succeed.

The semantics of 'all VMAs which exist in the range are sealed' would still be
maintained.

>
> --
> Cheers,
>
> David / dhildenb
>
Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check
Posted by Liam R. Howlett 2 months, 3 weeks ago
* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250714 11:32]:
> On Mon, Jul 14, 2025 at 05:25:59PM +0200, David Hildenbrand wrote:
> > On 14.07.25 17:23, Lorenzo Stoakes wrote:
> > > On Mon, Jul 14, 2025 at 04:17:23PM +0100, Pedro Falcato wrote:
> > > > On Mon, Jul 14, 2025 at 02:00:39PM +0100, Lorenzo Stoakes wrote:
> > > > > The check_mm_seal() function is doing something general - checking whether
> > > > > a range contains only VMAs (or rather that it does NOT contain any unmapped
> > > > > regions).
> > > > >
> > > > > Generalise this and put the logic in mm/vma.c - introducing
> > > > > range_contains_unmapped(). Additionally we can simplify the logic, we are
> > > > > simply checking whether the last vma->vm_end has either a VMA starting
> > > > > after it or ends before the end parameter.
> > > > >
> > > >
> > > > I don't like this. Unless you have any other user for this in mind,
> > > > we'll proliferate this awful behavior (and add this into core vma code).
> > >
> > > I'm not sure how putting it in an internal-only mm file perpetuates
> > > anything.
> > >
> > > I'm naming the function by what it does, and putting it where it belongs in
> > > the VMA logic, and additionally making the function less horrible.
> > >
> > > Let's not please get stuck on the isues with mseal implementation which
> > > will catch-22 us into not being able to refactor.
> > >
> > > We can do the refactoring first and it's fine to just yank this if it's not
> > > used.
> > >
> > > I'm not having a function like this sat in mm/mseal.c when it has
> > > absolutely nothing to do with mseal specifically though.
> > >
> > > >
> > > > I have some patches locally to fully remove this upfront check, and AFAIK
> > > > we're somewhat in agreement that we can simply nuke this check (for
> > > > various reasons, including that we *still* don't have a man page for the
> > > > syscall). I can send them for proper discussion after your series lands.
> > >
> > > Yes I agree this check is odd, I don't really see why on earth we're
> > > concerned with whether there are gaps or not, you'd surely want to just
> > > seal whatever VMAs exist in the range?
> >
> > Probably because GAPs cannot be sealed. So user space could assume that in
> > fact, nothing in that area can change after a successful mseal, while it can
> > ...
> >
> > Not sure, though ...
> 
> Yeah maybe a sekuriteh thing where you want to be sure the range is in fact
> _all_ sealed.
> 
> I'm not sure that really makes much sense in practice honestly though, because
> if another thread can fiddle with things, then surely you've already 'lost'.
> 
> if you expected to touch a VMA where in fact aa gap exists your software is
> _already_ broken because you're going to segfault.

Since this is applying a seal to a range that already exists, we are in
a race condition anyways.

If a gap exists it might be better to fail and exit as it is insecure,
but really, if someone has created a gap then they could have mapped
whatever they want..

> 
> So it just seems overly theoretical to me.

I don't see a theoretical means to justify doing this, so I must be
missing something.

> 
> I think we should error out if there's _no_ VMAs at all, but otherwise succeed.
> 
> The semantics of 'all VMAs which exist in the range are sealed' would still be
> maintained.
> 
> >
> > --
> > Cheers,
> >
> > David / dhildenb
> >
Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check
Posted by David Hildenbrand 2 months, 3 weeks ago
On 14.07.25 15:00, Lorenzo Stoakes wrote:
> The check_mm_seal() function is doing something general - checking whether
> a range contains only VMAs (or rather that it does NOT contain any unmapped
> regions).
> 
> Generalise this and put the logic in mm/vma.c - introducing
> range_contains_unmapped(). Additionally we can simplify the logic, we are
> simply checking whether the last vma->vm_end has either a VMA starting
> after it or ends before the end parameter.
> 
> No functional change intended.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb