| Message ID | 6d973a629a321aa73c286f2d64d5375327d5c02a.1676902832.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [for-next,v2,1/1] io_uring/rsrc: disallow multi-source reg buffers | expand |
Pavel Begunkov <asml.silence@gmail.com> writes: > If two or more mappings go back to back to each other they can be passed > into io_uring to be registered as a single registered buffer. That would > even work if mappings came from different sources, e.g. it's possible to > mix in this way anon pages and pages from shmem or hugetlb. That is not > a problem but it'd rather be less prone if we forbid such mixing. > > Cc: <stable@vger.kernel.org> > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> > --- > io_uring/rsrc.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c > index a59fc02de598..70d7f94670f9 100644 > --- a/io_uring/rsrc.c > +++ b/io_uring/rsrc.c > @@ -1162,18 +1162,19 @@ struct page **io_pin_pages(unsigned long ubuf, unsigned long len, int *npages) > pret = pin_user_pages(ubuf, nr_pages, FOLL_WRITE | FOLL_LONGTERM, > pages, vmas); > if (pret == nr_pages) { > + struct file *file = vmas[0]->vm_file; > + > /* don't support file backed memory */ > for (i = 0; i < nr_pages; i++) { > - struct vm_area_struct *vma = vmas[i]; > - > - if (vma_is_shmem(vma)) > + if (vmas[i]->vm_file != file) > + break; Perhaps, return -EINVAL instead of -EOPNOTSUPP > + if (!file) > continue; > - if (vma->vm_file && > - !is_file_hugepages(vma->vm_file)) { > - ret = -EOPNOTSUPP; > + if (!vma_is_shmem(vmas[i]) && !is_file_hugepages(file)) > break; > - } > } > + if (i != nr_pages) > + ret = -EOPNOTSUPP; > *npages = nr_pages; > } else { > ret = pret < 0 ? pret : -EFAULT;
On 2/20/23 7:53 AM, Gabriel Krisman Bertazi wrote: > Pavel Begunkov <asml.silence@gmail.com> writes: > >> If two or more mappings go back to back to each other they can be passed >> into io_uring to be registered as a single registered buffer. That would >> even work if mappings came from different sources, e.g. it's possible to >> mix in this way anon pages and pages from shmem or hugetlb. That is not >> a problem but it'd rather be less prone if we forbid such mixing. >> >> Cc: <stable@vger.kernel.org> >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> >> --- >> io_uring/rsrc.c | 15 ++++++++------- >> 1 file changed, 8 insertions(+), 7 deletions(-) >> >> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c >> index a59fc02de598..70d7f94670f9 100644 >> --- a/io_uring/rsrc.c >> +++ b/io_uring/rsrc.c >> @@ -1162,18 +1162,19 @@ struct page **io_pin_pages(unsigned long ubuf, unsigned long len, int *npages) >> pret = pin_user_pages(ubuf, nr_pages, FOLL_WRITE | FOLL_LONGTERM, >> pages, vmas); >> if (pret == nr_pages) { >> + struct file *file = vmas[0]->vm_file; >> + >> /* don't support file backed memory */ >> for (i = 0; i < nr_pages; i++) { >> - struct vm_area_struct *vma = vmas[i]; >> - >> - if (vma_is_shmem(vma)) >> + if (vmas[i]->vm_file != file) >> + break; > > Perhaps, return -EINVAL instead of -EOPNOTSUPP Agree, -EOPNOTSUPP is OK for an unsupported case, but I think -EINVAL makes more sense for the weird case of mixed sources as it's unlikely to ever make sense to support that.
On 2/21/23 16:29, Jens Axboe wrote: > On 2/20/23 7:53 AM, Gabriel Krisman Bertazi wrote: >> Pavel Begunkov <asml.silence@gmail.com> writes: >> >>> If two or more mappings go back to back to each other they can be passed >>> into io_uring to be registered as a single registered buffer. That would >>> even work if mappings came from different sources, e.g. it's possible to >>> mix in this way anon pages and pages from shmem or hugetlb. That is not >>> a problem but it'd rather be less prone if we forbid such mixing. >>> >>> Cc: <stable@vger.kernel.org> >>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> >>> --- >>> io_uring/rsrc.c | 15 ++++++++------- >>> 1 file changed, 8 insertions(+), 7 deletions(-) >>> >>> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c >>> index a59fc02de598..70d7f94670f9 100644 >>> --- a/io_uring/rsrc.c >>> +++ b/io_uring/rsrc.c >>> @@ -1162,18 +1162,19 @@ struct page **io_pin_pages(unsigned long ubuf, unsigned long len, int *npages) >>> pret = pin_user_pages(ubuf, nr_pages, FOLL_WRITE | FOLL_LONGTERM, >>> pages, vmas); >>> if (pret == nr_pages) { >>> + struct file *file = vmas[0]->vm_file; >>> + >>> /* don't support file backed memory */ >>> for (i = 0; i < nr_pages; i++) { >>> - struct vm_area_struct *vma = vmas[i]; >>> - >>> - if (vma_is_shmem(vma)) >>> + if (vmas[i]->vm_file != file) >>> + break; >> >> Perhaps, return -EINVAL instead of -EOPNOTSUPP > > Agree, -EOPNOTSUPP is OK for an unsupported case, but I think > -EINVAL makes more sense for the weird case of mixed sources > as it's unlikely to ever make sense to support that. ok
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c index a59fc02de598..70d7f94670f9 100644 --- a/io_uring/rsrc.c +++ b/io_uring/rsrc.c @@ -1162,18 +1162,19 @@ struct page **io_pin_pages(unsigned long ubuf, unsigned long len, int *npages) pret = pin_user_pages(ubuf, nr_pages, FOLL_WRITE | FOLL_LONGTERM, pages, vmas); if (pret == nr_pages) { + struct file *file = vmas[0]->vm_file; + /* don't support file backed memory */ for (i = 0; i < nr_pages; i++) { - struct vm_area_struct *vma = vmas[i]; - - if (vma_is_shmem(vma)) + if (vmas[i]->vm_file != file) + break; + if (!file) continue; - if (vma->vm_file && - !is_file_hugepages(vma->vm_file)) { - ret = -EOPNOTSUPP; + if (!vma_is_shmem(vmas[i]) && !is_file_hugepages(file)) break; - } } + if (i != nr_pages) + ret = -EOPNOTSUPP; *npages = nr_pages; } else { ret = pret < 0 ? pret : -EFAULT;
If two or more mappings go back to back to each other they can be passed into io_uring to be registered as a single registered buffer. That would even work if mappings came from different sources, e.g. it's possible to mix in this way anon pages and pages from shmem or hugetlb. That is not a problem but it'd rather be less prone if we forbid such mixing. Cc: <stable@vger.kernel.org> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- io_uring/rsrc.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)