[PATCH v2 0/2] xen/gntdev: Fixes for leaks and VMA splitting

M. Vefa Bicakci posted 2 patches 1 year, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
drivers/xen/gntdev-common.h |  3 +-
drivers/xen/gntdev.c        | 80 +++++++++++++++++++------------------
2 files changed, 44 insertions(+), 39 deletions(-)
[PATCH v2 0/2] xen/gntdev: Fixes for leaks and VMA splitting
Posted by M. Vefa Bicakci 1 year, 7 months ago
Hi all,

First of all, sorry for the delay!

These patches continue the code review for the following patches:
  https://lore.kernel.org/xen-devel/20220912040002.198191-1-m.v.b@runbox.com/t/#u

The original description of the patch set is as follows:

  "The changes in this patch series intend to fix the Xen grant device
  driver, so that grant mapping leaks caused by partially failed grant
  mapping operations are avoided with the first patch, and so that the
  splitting of VMAs does not result in incorrectly unmapped grant pages
  with the second patch. The second patch also prevents a similar issue
  in a double-mapping scenario, where mmap() is used with MAP_FIXED to
  map grants over an existing mapping created with the same grants, and
  where grant pages are unmapped incorrectly as well."

A summary of the changes from v1 is as follows:
- Addressed Juergen's code review comment regarding the first patch.
- Amended the description of the second patch to note that the described
  issues are encountered with PV domains.

Verification notes:

- I have tested these commits on top of Linux v5.15.70 and v5.15.71, and
  I verified that they compile successfully on top of the tag
  "next-20220930", which corresponds to the base commit ID included at
  the bottom of this e-mail.

- My tests consist of using a kernel with Qubes OS v4.1's patches and
  these patches on my main computer for day-to-day tasks, in conjunction
  with Qubes OS's version of the Xen hypervisor v4.14.5, with the latter
  custom-compiled with CONFIG_DEBUG.

- I used a test program that verifies the following scenarios with an
  unprivileged paravirtualized (PV) Xen domain:

  - A program mmap()s two pages from another Xen domain and munmap()s
    the pages one by one. This used to result in implicit unmap errors
    to be reported by Xen and a general protection fault to be triggered
    by Xen in the affected domain, but now works as expected.
  - A program mmap()s two pages from another Xen domain and then
    attempts to remap (via MAP_FIXED) the same mapping again over the
    same virtual address. This used to result in similar issues
    (implicit unmap errors and general protection fault), but now is
    rejected by the kernel.
  - A program mmap()s two pages from another Xen domain and then
    attempts to mmap() the same mapping again to a different virtual
    address, by passing NULL as mmap()'s first argument. This used to be
    rejected by the kernel, and it continues to be rejected by the
    kernel.

- Unprivileged PVH Xen domains were also sanity tested with the same
  test program. I should note that PVH domains worked as expected
  without these patches too.

- Finally, I have verified that the original "g.e. 0x1234 still pending"
  issue does not appear after rapidly resizing GUI windows in Qubes OS
  v4.1.

Thank you,

Vefa

M. Vefa Bicakci (2):
  xen/gntdev: Prevent leaking grants
  xen/gntdev: Accommodate VMA splitting

 drivers/xen/gntdev-common.h |  3 +-
 drivers/xen/gntdev.c        | 80 +++++++++++++++++++------------------
 2 files changed, 44 insertions(+), 39 deletions(-)


base-commit: 274d7803837da78dfc911bcda0d593412676fc20
-- 
2.37.3
Re: [PATCH v2 0/2] xen/gntdev: Fixes for leaks and VMA splitting
Posted by Juergen Gross 1 year, 6 months ago
On 03.10.22 00:20, M. Vefa Bicakci wrote:
> Hi all,
> 
> First of all, sorry for the delay!
> 
> These patches continue the code review for the following patches:
>    https://lore.kernel.org/xen-devel/20220912040002.198191-1-m.v.b@runbox.com/t/#u
> 
> The original description of the patch set is as follows:
> 
>    "The changes in this patch series intend to fix the Xen grant device
>    driver, so that grant mapping leaks caused by partially failed grant
>    mapping operations are avoided with the first patch, and so that the
>    splitting of VMAs does not result in incorrectly unmapped grant pages
>    with the second patch. The second patch also prevents a similar issue
>    in a double-mapping scenario, where mmap() is used with MAP_FIXED to
>    map grants over an existing mapping created with the same grants, and
>    where grant pages are unmapped incorrectly as well."
> 
> A summary of the changes from v1 is as follows:
> - Addressed Juergen's code review comment regarding the first patch.
> - Amended the description of the second patch to note that the described
>    issues are encountered with PV domains.
> 
> Verification notes:
> 
> - I have tested these commits on top of Linux v5.15.70 and v5.15.71, and
>    I verified that they compile successfully on top of the tag
>    "next-20220930", which corresponds to the base commit ID included at
>    the bottom of this e-mail.
> 
> - My tests consist of using a kernel with Qubes OS v4.1's patches and
>    these patches on my main computer for day-to-day tasks, in conjunction
>    with Qubes OS's version of the Xen hypervisor v4.14.5, with the latter
>    custom-compiled with CONFIG_DEBUG.
> 
> - I used a test program that verifies the following scenarios with an
>    unprivileged paravirtualized (PV) Xen domain:
> 
>    - A program mmap()s two pages from another Xen domain and munmap()s
>      the pages one by one. This used to result in implicit unmap errors
>      to be reported by Xen and a general protection fault to be triggered
>      by Xen in the affected domain, but now works as expected.
>    - A program mmap()s two pages from another Xen domain and then
>      attempts to remap (via MAP_FIXED) the same mapping again over the
>      same virtual address. This used to result in similar issues
>      (implicit unmap errors and general protection fault), but now is
>      rejected by the kernel.
>    - A program mmap()s two pages from another Xen domain and then
>      attempts to mmap() the same mapping again to a different virtual
>      address, by passing NULL as mmap()'s first argument. This used to be
>      rejected by the kernel, and it continues to be rejected by the
>      kernel.
> 
> - Unprivileged PVH Xen domains were also sanity tested with the same
>    test program. I should note that PVH domains worked as expected
>    without these patches too.
> 
> - Finally, I have verified that the original "g.e. 0x1234 still pending"
>    issue does not appear after rapidly resizing GUI windows in Qubes OS
>    v4.1.

Series pushed to xen/tip.git for-linus-6.1


Juergen

Re: [PATCH v2 0/2] xen/gntdev: Fixes for leaks and VMA splitting
Posted by M. Vefa Bicakci 1 year, 6 months ago
On 2022-10-07 01:17, Juergen Gross wrote:
> On 03.10.22 00:20, M. Vefa Bicakci wrote:
>> Hi all,
>>
>> First of all, sorry for the delay!
>>
>> These patches continue the code review for the following patches:
>>    https://lore.kernel.org/xen-devel/20220912040002.198191-1-m.v.b@runbox.com/t/#u
>>
>> The original description of the patch set is as follows:
>>
>>    "The changes in this patch series intend to fix the Xen grant device
>>    driver, so that grant mapping leaks caused by partially failed grant
>>    mapping operations are avoided with the first patch, and so that the
>>    splitting of VMAs does not result in incorrectly unmapped grant pages
>>    with the second patch. The second patch also prevents a similar issue
>>    in a double-mapping scenario, where mmap() is used with MAP_FIXED to
>>    map grants over an existing mapping created with the same grants, and
>>    where grant pages are unmapped incorrectly as well."
>>
>> A summary of the changes from v1 is as follows:
>> - Addressed Juergen's code review comment regarding the first patch.
>> - Amended the description of the second patch to note that the described
>>    issues are encountered with PV domains.
>>
>> Verification notes:
>>
>> - I have tested these commits on top of Linux v5.15.70 and v5.15.71, and
>>    I verified that they compile successfully on top of the tag
>>    "next-20220930", which corresponds to the base commit ID included at
>>    the bottom of this e-mail.
>>
>> - My tests consist of using a kernel with Qubes OS v4.1's patches and
>>    these patches on my main computer for day-to-day tasks, in conjunction
>>    with Qubes OS's version of the Xen hypervisor v4.14.5, with the latter
>>    custom-compiled with CONFIG_DEBUG.
>>
>> - I used a test program that verifies the following scenarios with an
>>    unprivileged paravirtualized (PV) Xen domain:
>>
>>    - A program mmap()s two pages from another Xen domain and munmap()s
>>      the pages one by one. This used to result in implicit unmap errors
>>      to be reported by Xen and a general protection fault to be triggered
>>      by Xen in the affected domain, but now works as expected.
>>    - A program mmap()s two pages from another Xen domain and then
>>      attempts to remap (via MAP_FIXED) the same mapping again over the
>>      same virtual address. This used to result in similar issues
>>      (implicit unmap errors and general protection fault), but now is
>>      rejected by the kernel.
>>    - A program mmap()s two pages from another Xen domain and then
>>      attempts to mmap() the same mapping again to a different virtual
>>      address, by passing NULL as mmap()'s first argument. This used to be
>>      rejected by the kernel, and it continues to be rejected by the
>>      kernel.
>>
>> - Unprivileged PVH Xen domains were also sanity tested with the same
>>    test program. I should note that PVH domains worked as expected
>>    without these patches too.
>>
>> - Finally, I have verified that the original "g.e. 0x1234 still pending"
>>    issue does not appear after rapidly resizing GUI windows in Qubes OS
>>    v4.1.
> 
> Series pushed to xen/tip.git for-linus-6.1
> 
> 
> Juergen

I am a bit late, but thank you for reviewing the changes and merging them!

Vefa

Re: [PATCH v2 0/2] xen/gntdev: Fixes for leaks and VMA splitting
Posted by Demi Marie Obenour 1 year, 6 months ago
On Fri, Oct 07, 2022 at 07:17:41AM +0200, Juergen Gross wrote:
> On 03.10.22 00:20, M. Vefa Bicakci wrote:
> > Hi all,
> > 
> > First of all, sorry for the delay!
> > 
> > These patches continue the code review for the following patches:
> >    https://lore.kernel.org/xen-devel/20220912040002.198191-1-m.v.b@runbox.com/t/#u
> > 
> > The original description of the patch set is as follows:
> > 
> >    "The changes in this patch series intend to fix the Xen grant device
> >    driver, so that grant mapping leaks caused by partially failed grant
> >    mapping operations are avoided with the first patch, and so that the
> >    splitting of VMAs does not result in incorrectly unmapped grant pages
> >    with the second patch. The second patch also prevents a similar issue
> >    in a double-mapping scenario, where mmap() is used with MAP_FIXED to
> >    map grants over an existing mapping created with the same grants, and
> >    where grant pages are unmapped incorrectly as well."
> > 
> > A summary of the changes from v1 is as follows:
> > - Addressed Juergen's code review comment regarding the first patch.
> > - Amended the description of the second patch to note that the described
> >    issues are encountered with PV domains.
> > 
> > Verification notes:
> > 
> > - I have tested these commits on top of Linux v5.15.70 and v5.15.71, and
> >    I verified that they compile successfully on top of the tag
> >    "next-20220930", which corresponds to the base commit ID included at
> >    the bottom of this e-mail.
> > 
> > - My tests consist of using a kernel with Qubes OS v4.1's patches and
> >    these patches on my main computer for day-to-day tasks, in conjunction
> >    with Qubes OS's version of the Xen hypervisor v4.14.5, with the latter
> >    custom-compiled with CONFIG_DEBUG.
> > 
> > - I used a test program that verifies the following scenarios with an
> >    unprivileged paravirtualized (PV) Xen domain:
> > 
> >    - A program mmap()s two pages from another Xen domain and munmap()s
> >      the pages one by one. This used to result in implicit unmap errors
> >      to be reported by Xen and a general protection fault to be triggered
> >      by Xen in the affected domain, but now works as expected.
> >    - A program mmap()s two pages from another Xen domain and then
> >      attempts to remap (via MAP_FIXED) the same mapping again over the
> >      same virtual address. This used to result in similar issues
> >      (implicit unmap errors and general protection fault), but now is
> >      rejected by the kernel.
> >    - A program mmap()s two pages from another Xen domain and then
> >      attempts to mmap() the same mapping again to a different virtual
> >      address, by passing NULL as mmap()'s first argument. This used to be
> >      rejected by the kernel, and it continues to be rejected by the
> >      kernel.
> > 
> > - Unprivileged PVH Xen domains were also sanity tested with the same
> >    test program. I should note that PVH domains worked as expected
> >    without these patches too.
> > 
> > - Finally, I have verified that the original "g.e. 0x1234 still pending"
> >    issue does not appear after rapidly resizing GUI windows in Qubes OS
> >    v4.1.
> 
> Series pushed to xen/tip.git for-linus-6.1

Thanks!
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab