From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
To: "Souptick Joarder" <jrdr.linux@gmail.com>,
"Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Matthew Wilcox <willy@infradead.org>,
Michal Hocko <mhocko@suse.com>, Juergen Gross <jgross@suse.com>,
Russell King - ARM Linux <linux@armlinux.org.uk>,
robin.murphy@arm.com, xen-devel@lists.xenproject.org,
linux-kernel@vger.kernel.org, Linux-MM <linux-mm@kvack.org>
Subject: Re: [Xen-devel] [PATCH v4 8/9] xen/gntdev.c: Convert to use vm_map_pages()
Date: Tue, 30 Jul 2019 10:05:42 -0400 [thread overview]
Message-ID: <bf02becc-9db0-bb78-8efc-9e25cc115237@oracle.com> (raw)
In-Reply-To: <CAFqt6zZN+6r6wYJY+f15JAjj8dY+o30w_+EWH9Vy2kUXCKSBog@mail.gmail.com>
On 7/30/19 2:03 AM, Souptick Joarder wrote:
> On Mon, Jul 29, 2019 at 7:06 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
>> On Mon, Jul 29, 2019 at 02:02:54PM +0530, Souptick Joarder wrote:
>>> On Mon, Jul 29, 2019 at 1:35 PM Souptick Joarder <jrdr.linux@gmail.com> wrote:
>>>> On Sun, Jul 28, 2019 at 11:36 PM Marek Marczykowski-Górecki
>>>> <marmarek@invisiblethingslab.com> wrote:
>>>>> On Fri, Feb 15, 2019 at 08:18:31AM +0530, Souptick Joarder wrote:
>>>>>> Convert to use vm_map_pages() to map range of kernel
>>>>>> memory to user vma.
>>>>>>
>>>>>> map->count is passed to vm_map_pages() and internal API
>>>>>> verify map->count against count ( count = vma_pages(vma))
>>>>>> for page array boundary overrun condition.
>>>>> This commit breaks gntdev driver. If vma->vm_pgoff > 0, vm_map_pages
>>>>> will:
>>>>> - use map->pages starting at vma->vm_pgoff instead of 0
>>>> The actual code ignores vma->vm_pgoff > 0 scenario and mapped
>>>> the entire map->pages[i]. Why the entire map->pages[i] needs to be mapped
>>>> if vma->vm_pgoff > 0 (in original code) ?
>> vma->vm_pgoff is used as index passed to gntdev_find_map_index. It's
>> basically (ab)using this parameter for "which grant reference to map".
>>
>>>> are you referring to set vma->vm_pgoff = 0 irrespective of value passed
>>>> from user space ? If yes, using vm_map_pages_zero() is an alternate
>>>> option.
>> Yes, that should work.
> I prefer to use vm_map_pages_zero() to resolve both the issues. Alternatively
> the patch can be reverted as you suggested. Let me know you opinion and wait
> for feedback from others.
>
> Boris, would you like to give any feedback ?
vm_map_pages_zero() looks good to me. Marek, does it work for you?
-boris
WARNING: multiple messages have this Message-ID (diff)
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
To: "Souptick Joarder" <jrdr.linux@gmail.com>,
"Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
Cc: Juergen Gross <jgross@suse.com>, Michal Hocko <mhocko@suse.com>,
Russell King - ARM Linux <linux@armlinux.org.uk>,
Matthew Wilcox <willy@infradead.org>,
linux-kernel@vger.kernel.org, Linux-MM <linux-mm@kvack.org>,
xen-devel@lists.xenproject.org,
Andrew Morton <akpm@linux-foundation.org>,
robin.murphy@arm.com
Subject: Re: [Xen-devel] [PATCH v4 8/9] xen/gntdev.c: Convert to use vm_map_pages()
Date: Tue, 30 Jul 2019 10:05:42 -0400 [thread overview]
Message-ID: <bf02becc-9db0-bb78-8efc-9e25cc115237@oracle.com> (raw)
In-Reply-To: <CAFqt6zZN+6r6wYJY+f15JAjj8dY+o30w_+EWH9Vy2kUXCKSBog@mail.gmail.com>
On 7/30/19 2:03 AM, Souptick Joarder wrote:
> On Mon, Jul 29, 2019 at 7:06 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
>> On Mon, Jul 29, 2019 at 02:02:54PM +0530, Souptick Joarder wrote:
>>> On Mon, Jul 29, 2019 at 1:35 PM Souptick Joarder <jrdr.linux@gmail.com> wrote:
>>>> On Sun, Jul 28, 2019 at 11:36 PM Marek Marczykowski-Górecki
>>>> <marmarek@invisiblethingslab.com> wrote:
>>>>> On Fri, Feb 15, 2019 at 08:18:31AM +0530, Souptick Joarder wrote:
>>>>>> Convert to use vm_map_pages() to map range of kernel
>>>>>> memory to user vma.
>>>>>>
>>>>>> map->count is passed to vm_map_pages() and internal API
>>>>>> verify map->count against count ( count = vma_pages(vma))
>>>>>> for page array boundary overrun condition.
>>>>> This commit breaks gntdev driver. If vma->vm_pgoff > 0, vm_map_pages
>>>>> will:
>>>>> - use map->pages starting at vma->vm_pgoff instead of 0
>>>> The actual code ignores vma->vm_pgoff > 0 scenario and mapped
>>>> the entire map->pages[i]. Why the entire map->pages[i] needs to be mapped
>>>> if vma->vm_pgoff > 0 (in original code) ?
>> vma->vm_pgoff is used as index passed to gntdev_find_map_index. It's
>> basically (ab)using this parameter for "which grant reference to map".
>>
>>>> are you referring to set vma->vm_pgoff = 0 irrespective of value passed
>>>> from user space ? If yes, using vm_map_pages_zero() is an alternate
>>>> option.
>> Yes, that should work.
> I prefer to use vm_map_pages_zero() to resolve both the issues. Alternatively
> the patch can be reverted as you suggested. Let me know you opinion and wait
> for feedback from others.
>
> Boris, would you like to give any feedback ?
vm_map_pages_zero() looks good to me. Marek, does it work for you?
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-07-30 14:07 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-15 2:48 [PATCH v4 8/9] xen/gntdev.c: Convert to use vm_map_pages() Souptick Joarder
2019-07-28 18:06 ` [Xen-devel] " Marek Marczykowski-Górecki
2019-07-28 18:06 ` Marek Marczykowski-Górecki
2019-07-29 8:05 ` Souptick Joarder
2019-07-29 8:05 ` Souptick Joarder
2019-07-29 8:05 ` Souptick Joarder
2019-07-29 8:32 ` Souptick Joarder
2019-07-29 8:32 ` Souptick Joarder
2019-07-29 8:32 ` Souptick Joarder
2019-07-29 13:36 ` Marek Marczykowski-Górecki
2019-07-29 13:36 ` Marek Marczykowski-Górecki
2019-07-30 6:03 ` Souptick Joarder
2019-07-30 6:03 ` Souptick Joarder
2019-07-30 6:03 ` Souptick Joarder
2019-07-30 14:05 ` Boris Ostrovsky [this message]
2019-07-30 14:05 ` Boris Ostrovsky
2019-07-30 14:22 ` Marek Marczykowski-Górecki
2019-07-30 14:22 ` Marek Marczykowski-Górecki
2019-07-30 14:52 ` Souptick Joarder
2019-07-30 14:52 ` Souptick Joarder
2019-07-30 14:52 ` Souptick Joarder
2019-07-30 15:01 ` Marek Marczykowski-Górecki
2019-07-30 15:01 ` Marek Marczykowski-Górecki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bf02becc-9db0-bb78-8efc-9e25cc115237@oracle.com \
--to=boris.ostrovsky@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=jgross@suse.com \
--cc=jrdr.linux@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux@armlinux.org.uk \
--cc=marmarek@invisiblethingslab.com \
--cc=mhocko@suse.com \
--cc=robin.murphy@arm.com \
--cc=willy@infradead.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.