[01/22] mm: Add vma_lookup()
diff mbox series

Message ID 20210510165839.2692974-2-Liam.Howlett@Oracle.com
State New, archived
Headers show
Series
  • mm: Add vma_lookup()
Related show

Commit Message

Liam Howlett May 10, 2021, 4:58 p.m. UTC
Many places in the kernel use find_vma() to get a vma and then check the
start address of the vma to ensure the next vma was not returned.

Other places use the find_vma_intersection() call with add, addr + 1 as
the range; looking for just the vma at a specific address.

The third use of find_vma() is by developers who do not know that the
function starts searching at the provided address upwards for the next
vma.  This results in a bug that is often overlooked for a long time.

Adding the new vma_lookup() function will allow for cleaner code by
removing the find_vma() calls which check limits, making
find_vma_intersection() calls of a single address to be shorter, and
potentially reduce the incorrect uses of find_vma().

Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
---
 include/linux/mm.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

David Hildenbrand May 14, 2021, 9:57 a.m. UTC | #1
On 10.05.21 18:58, Liam Howlett wrote:
> he third use of find_vma() is by developers who do not know that the
> function starts searching at the provided address upwards for the next
> vma.  This results in a bug that is often overlooked for a long time.
> 
> Adding the new vma_lookup() function will allow for cleaner code by

Sounds helpful to me

Acked-by: David Hildenbrand <david@redhat.com>
Davidlohr Bueso May 21, 2021, 3:32 a.m. UTC | #2
On Mon, 10 May 2021, Liam Howlett wrote:

>Many places in the kernel use find_vma() to get a vma and then check the
>start address of the vma to ensure the next vma was not returned.
>
>Other places use the find_vma_intersection() call with add, addr + 1 as
>the range; looking for just the vma at a specific address.
>
>The third use of find_vma() is by developers who do not know that the
>function starts searching at the provided address upwards for the next
>vma.  This results in a bug that is often overlooked for a long time.
>
>Adding the new vma_lookup() function will allow for cleaner code by
>removing the find_vma() calls which check limits, making
>find_vma_intersection() calls of a single address to be shorter, and
>potentially reduce the incorrect uses of find_vma().
>
>Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
>---
> include/linux/mm.h | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
>diff --git a/include/linux/mm.h b/include/linux/mm.h
>index 25b9041f9925..5f2a15e702ff 100644
>--- a/include/linux/mm.h
>+++ b/include/linux/mm.h
>@@ -2689,6 +2689,24 @@ static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * m
>	return vma;
> }

While at it can we clean up find_vma_intersection? I'm not particularly
user/fan of checkpatch.pl, but this one is kind of ridiculous.

Thanks,
Davidlohr

diff --git a/include/linux/mm.h b/include/linux/mm.h
index c274f75efcf9..16eddedf783f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2678,9 +2678,14 @@ extern struct vm_area_struct * find_vma(struct mm_struct * mm, unsigned long add
  extern struct vm_area_struct * find_vma_prev(struct mm_struct * mm, unsigned long addr,
					     struct vm_area_struct **pprev);

-/* Look up the first VMA which intersects the interval start_addr..end_addr-1,
-   NULL if none.  Assume start_addr < end_addr. */
-static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * mm, unsigned long start_addr, unsigned long end_addr)
+/*
+ * Look up the first VMA which intersects the interval start_addr..end_addr-1,
+ * NULL if none.  Assume start_addr < end_addr.
+ */
+static inline
+struct vm_area_struct *find_vma_intersection(struct mm_struct * mm,
+                                            unsigned long start_addr,
+                                            unsigned long end_addr)
  {
	struct vm_area_struct * vma = find_vma(mm,start_addr);
Liam Howlett May 21, 2021, 2:24 p.m. UTC | #3
* Davidlohr Bueso <dave@stgolabs.net> [210520 23:32]:
> On Mon, 10 May 2021, Liam Howlett wrote:
> 
> > Many places in the kernel use find_vma() to get a vma and then check the
> > start address of the vma to ensure the next vma was not returned.
> > 
> > Other places use the find_vma_intersection() call with add, addr + 1 as
> > the range; looking for just the vma at a specific address.
> > 
> > The third use of find_vma() is by developers who do not know that the
> > function starts searching at the provided address upwards for the next
> > vma.  This results in a bug that is often overlooked for a long time.
> > 
> > Adding the new vma_lookup() function will allow for cleaner code by
> > removing the find_vma() calls which check limits, making
> > find_vma_intersection() calls of a single address to be shorter, and
> > potentially reduce the incorrect uses of find_vma().
> > 
> > Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
> > ---
> > include/linux/mm.h | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> > 
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 25b9041f9925..5f2a15e702ff 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -2689,6 +2689,24 @@ static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * m
> > 	return vma;
> > }
> 
> While at it can we clean up find_vma_intersection? I'm not particularly
> user/fan of checkpatch.pl, but this one is kind of ridiculous.

Agreed.  This addition is worth re-spin.  I will change the comment into
a kernel documentation style comment at the same time.

> 
> Thanks,
> Davidlohr
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index c274f75efcf9..16eddedf783f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2678,9 +2678,14 @@ extern struct vm_area_struct * find_vma(struct mm_struct * mm, unsigned long add
>  extern struct vm_area_struct * find_vma_prev(struct mm_struct * mm, unsigned long addr,
> 					     struct vm_area_struct **pprev);
> 
> -/* Look up the first VMA which intersects the interval start_addr..end_addr-1,
> -   NULL if none.  Assume start_addr < end_addr. */
> -static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * mm, unsigned long start_addr, unsigned long end_addr)
> +/*
> + * Look up the first VMA which intersects the interval start_addr..end_addr-1,
> + * NULL if none.  Assume start_addr < end_addr.
> + */
> +static inline
> +struct vm_area_struct *find_vma_intersection(struct mm_struct * mm,
> +                                            unsigned long start_addr,
> +                                            unsigned long end_addr)
>  {
> 	struct vm_area_struct * vma = find_vma(mm,start_addr);

Patch
diff mbox series

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 25b9041f9925..5f2a15e702ff 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2689,6 +2689,24 @@  static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * m
 	return vma;
 }
 
+/**
+ * vma_lookup() - Find a VMA at a specific address
+ * @mm: The process address space.
+ * @addr: The user address.
+ *
+ * Return: The vm_area_struct at the given address, %NULL otherwise.
+ */
+static inline
+struct vm_area_struct *vma_lookup(struct mm_struct *mm, unsigned long addr)
+{
+	struct vm_area_struct * vma = find_vma(mm, addr);
+
+	if (vma && addr < vma->vm_start)
+		vma = NULL;
+
+	return vma;
+}
+
 static inline unsigned long vm_start_gap(struct vm_area_struct *vma)
 {
 	unsigned long vm_start = vma->vm_start;