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

["M. Vefa Bicakci" <m.v.b@runbox.com>]

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