| Message ID | 20210510165839.2692974-2-Liam.Howlett@Oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: Add vma_lookup() | expand |
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>
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);
* 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);
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;
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(+)