[PATCH 4/7] mm/gup: introduce the FOLL_SAME_FILE GUP flag

Lorenzo Stoakes posted 7 patches 2 years, 8 months ago
There is a newer version of this series
[PATCH 4/7] mm/gup: introduce the FOLL_SAME_FILE GUP flag
Posted by Lorenzo Stoakes 2 years, 8 months ago
This flag causes GUP to assert that all VMAs within the input range possess
the same vma->vm_file. If not, the operation fails.

This is part of a patch series which eliminates the vmas parameter from the
GUP API, implementing the one remaining assertion within the entire kernel
that requires access to the VMAs associated with a GUP range.

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
 include/linux/mm_types.h |  2 ++
 mm/gup.c                 | 16 ++++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 3fc9e680f174..84d1aec9dbab 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -1185,6 +1185,8 @@ enum {
 	FOLL_PCI_P2PDMA = 1 << 10,
 	/* allow interrupts from generic signals */
 	FOLL_INTERRUPTIBLE = 1 << 11,
+	/* assert that the range spans VMAs with the same vma->vm_file */
+	FOLL_SAME_FILE = 1 << 12,
 
 	/* See also internal only FOLL flags in mm/internal.h */
 };
diff --git a/mm/gup.c b/mm/gup.c
index 9440aa54c741..3954ce499a4a 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -959,7 +959,8 @@ static int faultin_page(struct vm_area_struct *vma,
 	return 0;
 }
 
-static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)
+static int check_vma_flags(struct vm_area_struct *vma, struct file *file,
+			   unsigned long gup_flags)
 {
 	vm_flags_t vm_flags = vma->vm_flags;
 	int write = (gup_flags & FOLL_WRITE);
@@ -968,7 +969,7 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)
 	if (vm_flags & (VM_IO | VM_PFNMAP))
 		return -EFAULT;
 
-	if (gup_flags & FOLL_ANON && !vma_is_anonymous(vma))
+	if ((gup_flags & FOLL_ANON) && !vma_is_anonymous(vma))
 		return -EFAULT;
 
 	if ((gup_flags & FOLL_LONGTERM) && vma_is_fsdax(vma))
@@ -977,6 +978,9 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)
 	if (vma_is_secretmem(vma))
 		return -EFAULT;
 
+	if ((gup_flags & FOLL_SAME_FILE) && vma->vm_file != file)
+		return -EFAULT;
+
 	if (write) {
 		if (!(vm_flags & VM_WRITE)) {
 			if (!(gup_flags & FOLL_FORCE))
@@ -1081,6 +1085,7 @@ static long __get_user_pages(struct mm_struct *mm,
 	long ret = 0, i = 0;
 	struct vm_area_struct *vma = NULL;
 	struct follow_page_context ctx = { NULL };
+	struct file *file = NULL;
 
 	if (!nr_pages)
 		return 0;
@@ -1111,10 +1116,13 @@ static long __get_user_pages(struct mm_struct *mm,
 				ret = -EFAULT;
 				goto out;
 			}
-			ret = check_vma_flags(vma, gup_flags);
+			ret = check_vma_flags(vma, i == 0 ? vma->vm_file : file,
+					      gup_flags);
 			if (ret)
 				goto out;
 
+			file = vma->vm_file;
+
 			if (is_vm_hugetlb_page(vma)) {
 				i = follow_hugetlb_page(mm, vma, pages, vmas,
 						&start, &nr_pages, i,
@@ -1595,7 +1603,7 @@ long faultin_vma_page_range(struct vm_area_struct *vma, unsigned long start,
 	 * We want to report -EINVAL instead of -EFAULT for any permission
 	 * problems or incompatible mappings.
 	 */
-	if (check_vma_flags(vma, gup_flags))
+	if (check_vma_flags(vma, vma->vm_file, gup_flags))
 		return -EINVAL;
 
 	ret = __get_user_pages(mm, start, nr_pages, gup_flags,
-- 
2.40.0
Re: [PATCH 4/7] mm/gup: introduce the FOLL_SAME_FILE GUP flag
Posted by Jason Gunthorpe 2 years, 8 months ago
On Sat, Apr 15, 2023 at 12:27:40AM +0100, Lorenzo Stoakes wrote:
> This flag causes GUP to assert that all VMAs within the input range possess
> the same vma->vm_file. If not, the operation fails.
> 
> This is part of a patch series which eliminates the vmas parameter from the
> GUP API, implementing the one remaining assertion within the entire kernel
> that requires access to the VMAs associated with a GUP range.
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
> ---
>  include/linux/mm_types.h |  2 ++
>  mm/gup.c                 | 16 ++++++++++++----
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 3fc9e680f174..84d1aec9dbab 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -1185,6 +1185,8 @@ enum {
>  	FOLL_PCI_P2PDMA = 1 << 10,
>  	/* allow interrupts from generic signals */
>  	FOLL_INTERRUPTIBLE = 1 << 11,
> +	/* assert that the range spans VMAs with the same vma->vm_file */
> +	FOLL_SAME_FILE = 1 << 12,

I hope we don't add this flag, but it needs to be rejected in
internal_get_user_pages_fast()

Jason
Re: [PATCH 4/7] mm/gup: introduce the FOLL_SAME_FILE GUP flag
Posted by Lorenzo Stoakes 2 years, 8 months ago
On Mon, Apr 17, 2023 at 10:14:02AM -0300, Jason Gunthorpe wrote:
> On Sat, Apr 15, 2023 at 12:27:40AM +0100, Lorenzo Stoakes wrote:
> > This flag causes GUP to assert that all VMAs within the input range possess
> > the same vma->vm_file. If not, the operation fails.
> >
> > This is part of a patch series which eliminates the vmas parameter from the
> > GUP API, implementing the one remaining assertion within the entire kernel
> > that requires access to the VMAs associated with a GUP range.
> >
> > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
> > ---
> >  include/linux/mm_types.h |  2 ++
> >  mm/gup.c                 | 16 ++++++++++++----
> >  2 files changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 3fc9e680f174..84d1aec9dbab 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -1185,6 +1185,8 @@ enum {
> >  	FOLL_PCI_P2PDMA = 1 << 10,
> >  	/* allow interrupts from generic signals */
> >  	FOLL_INTERRUPTIBLE = 1 << 11,
> > +	/* assert that the range spans VMAs with the same vma->vm_file */
> > +	FOLL_SAME_FILE = 1 << 12,
>
> I hope we don't add this flag, but it needs to be rejected in
> internal_get_user_pages_fast()
>

intenal_get_user_pages_fast() checks against the complement of accepted
masks, therefore it will reject this as-is unless I'm missing something.

As for not adding the flag (an entirely understandable sentiment), it would
be good to get an insight into the semantics of what you feel would be more
suitable!

> Jason
Re: [PATCH 4/7] mm/gup: introduce the FOLL_SAME_FILE GUP flag
Posted by Jason Gunthorpe 2 years, 8 months ago
On Mon, Apr 17, 2023 at 02:25:54PM +0100, Lorenzo Stoakes wrote:
> On Mon, Apr 17, 2023 at 10:14:02AM -0300, Jason Gunthorpe wrote:
> > On Sat, Apr 15, 2023 at 12:27:40AM +0100, Lorenzo Stoakes wrote:
> > > This flag causes GUP to assert that all VMAs within the input range possess
> > > the same vma->vm_file. If not, the operation fails.
> > >
> > > This is part of a patch series which eliminates the vmas parameter from the
> > > GUP API, implementing the one remaining assertion within the entire kernel
> > > that requires access to the VMAs associated with a GUP range.
> > >
> > > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
> > > ---
> > >  include/linux/mm_types.h |  2 ++
> > >  mm/gup.c                 | 16 ++++++++++++----
> > >  2 files changed, 14 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > > index 3fc9e680f174..84d1aec9dbab 100644
> > > --- a/include/linux/mm_types.h
> > > +++ b/include/linux/mm_types.h
> > > @@ -1185,6 +1185,8 @@ enum {
> > >  	FOLL_PCI_P2PDMA = 1 << 10,
> > >  	/* allow interrupts from generic signals */
> > >  	FOLL_INTERRUPTIBLE = 1 << 11,
> > > +	/* assert that the range spans VMAs with the same vma->vm_file */
> > > +	FOLL_SAME_FILE = 1 << 12,
> >
> > I hope we don't add this flag, but it needs to be rejected in
> > internal_get_user_pages_fast()
> >
> 
> intenal_get_user_pages_fast() checks against the complement of accepted
> masks, therefore it will reject this as-is unless I'm missing
> something.

Hmm, yes, that looks OK

> As for not adding the flag (an entirely understandable sentiment), it would
> be good to get an insight into the semantics of what you feel would be more
> suitable!

I'm hoping there is not a solid justification for why io_uring is
doing this check so we just delete it.

Jason