* [PATCH for-next v2 1/1] io_uring/rsrc: disallow multi-source reg buffers
@ 2023-02-20 14:20 Pavel Begunkov
2023-02-20 14:53 ` Gabriel Krisman Bertazi
0 siblings, 1 reply; 4+ messages in thread
From: Pavel Begunkov @ 2023-02-20 14:20 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
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: <[email protected]>
Signed-off-by: Pavel Begunkov <[email protected]>
---
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;
+ 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;
--
2.39.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH for-next v2 1/1] io_uring/rsrc: disallow multi-source reg buffers
2023-02-20 14:20 [PATCH for-next v2 1/1] io_uring/rsrc: disallow multi-source reg buffers Pavel Begunkov
@ 2023-02-20 14:53 ` Gabriel Krisman Bertazi
2023-02-21 16:29 ` Jens Axboe
0 siblings, 1 reply; 4+ messages in thread
From: Gabriel Krisman Bertazi @ 2023-02-20 14:53 UTC (permalink / raw)
To: Pavel Begunkov; +Cc: io-uring, Jens Axboe
Pavel Begunkov <[email protected]> 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: <[email protected]>
> Signed-off-by: Pavel Begunkov <[email protected]>
> ---
> 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;
--
Gabriel Krisman Bertazi
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-next v2 1/1] io_uring/rsrc: disallow multi-source reg buffers
2023-02-20 14:53 ` Gabriel Krisman Bertazi
@ 2023-02-21 16:29 ` Jens Axboe
2023-02-21 18:40 ` Pavel Begunkov
0 siblings, 1 reply; 4+ messages in thread
From: Jens Axboe @ 2023-02-21 16:29 UTC (permalink / raw)
To: Gabriel Krisman Bertazi, Pavel Begunkov; +Cc: io-uring
On 2/20/23 7:53 AM, Gabriel Krisman Bertazi wrote:
> Pavel Begunkov <[email protected]> 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: <[email protected]>
>> Signed-off-by: Pavel Begunkov <[email protected]>
>> ---
>> 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.
--
Jens Axboe
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-next v2 1/1] io_uring/rsrc: disallow multi-source reg buffers
2023-02-21 16:29 ` Jens Axboe
@ 2023-02-21 18:40 ` Pavel Begunkov
0 siblings, 0 replies; 4+ messages in thread
From: Pavel Begunkov @ 2023-02-21 18:40 UTC (permalink / raw)
To: Jens Axboe, Gabriel Krisman Bertazi; +Cc: io-uring
On 2/21/23 16:29, Jens Axboe wrote:
> On 2/20/23 7:53 AM, Gabriel Krisman Bertazi wrote:
>> Pavel Begunkov <[email protected]> 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: <[email protected]>
>>> Signed-off-by: Pavel Begunkov <[email protected]>
>>> ---
>>> 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
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-02-21 18:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-20 14:20 [PATCH for-next v2 1/1] io_uring/rsrc: disallow multi-source reg buffers Pavel Begunkov
2023-02-20 14:53 ` Gabriel Krisman Bertazi
2023-02-21 16:29 ` Jens Axboe
2023-02-21 18:40 ` Pavel Begunkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox