On Tue, Jul 30, 2019 at 7:52 PM Marek Marczykowski-Górecki marmarek@invisiblethingslab.com wrote:
On Tue, Jul 30, 2019 at 10:05:42AM -0400, Boris Ostrovsky wrote:
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?
Yes, replacing vm_map_pages() with vm_map_pages_zero() fixes the problem for me.
Marek, I can send a patch for the same if you are ok. We need to cc stable as this changes are available in 5.2.4.
-- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing?
On Tue, Jul 30, 2019 at 08:22:02PM +0530, Souptick Joarder wrote:
On Tue, Jul 30, 2019 at 7:52 PM Marek Marczykowski-Górecki marmarek@invisiblethingslab.com wrote:
On Tue, Jul 30, 2019 at 10:05:42AM -0400, Boris Ostrovsky wrote:
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?
Yes, replacing vm_map_pages() with vm_map_pages_zero() fixes the problem for me.
Marek, I can send a patch for the same if you are ok. We need to cc stable as this changes are available in 5.2.4.
Sounds good, thanks!
linux-stable-mirror@lists.linaro.org
-
Marek Marczykowski-Górecki -
Souptick Joarder