* [PATCH v1 ] nitro_enclaves: Add mmap_read_lock() for the get_user_pages() call
@ 2021-12-18 10:35 Andra Paraschiv
2021-12-18 10:43 ` Paraschiv, Andra-Irina
2021-12-20 9:23 ` Vitaly Kuznetsov
0 siblings, 2 replies; 5+ messages in thread
From: Andra Paraschiv @ 2021-12-18 10:35 UTC (permalink / raw)
To: linux-kernel
Cc: Alexandru Ciobotaru, Greg KH, Alexandru Vasile, Marcelo Cerri,
Paolo Bonzini, Stefano Garzarella, Stefan Hajnoczi, Tim Gardner,
Vitaly Kuznetsov, kvm, ne-devel-upstream, stable,
Andra Paraschiv
After commit 5b78ed24e8ec (mm/pagemap: add mmap_assert_locked()
annotations to find_vma*()), the call to get_user_pages() will trigger
the mmap assert.
static inline void mmap_assert_locked(struct mm_struct *mm)
{
lockdep_assert_held(&mm->mmap_lock);
VM_BUG_ON_MM(!rwsem_is_locked(&mm->mmap_lock), mm);
}
[ 62.521410] kernel BUG at include/linux/mmap_lock.h:156!
...........................................................
[ 62.538938] RIP: 0010:find_vma+0x32/0x80
...........................................................
[ 62.605889] Call Trace:
[ 62.608502] <TASK>
[ 62.610956] ? lock_timer_base+0x61/0x80
[ 62.614106] find_extend_vma+0x19/0x80
[ 62.617195] __get_user_pages+0x9b/0x6a0
[ 62.620356] __gup_longterm_locked+0x42d/0x450
[ 62.623721] ? finish_wait+0x41/0x80
[ 62.626748] ? __kmalloc+0x178/0x2f0
[ 62.629768] ne_set_user_memory_region_ioctl.isra.0+0x225/0x6a0 [nitro_enclaves]
[ 62.635776] ne_enclave_ioctl+0x1cf/0x6d7 [nitro_enclaves]
[ 62.639541] __x64_sys_ioctl+0x82/0xb0
[ 62.642620] do_syscall_64+0x3b/0x90
[ 62.645642] entry_SYSCALL_64_after_hwframe+0x44/0xae
Add mmap_read_lock() for the get_user_pages() call when setting the
enclave memory regions.
Signed-off-by: Andra Paraschiv <andraprs@amazon.com>
Cc: stable@vger.kernel.org
---
drivers/virt/nitro_enclaves/ne_misc_dev.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
index 8939612ee0e0..6c51ff024036 100644
--- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
+++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
@@ -886,8 +886,13 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
goto put_pages;
}
+ mmap_read_lock(current->mm);
+
gup_rc = get_user_pages(mem_region.userspace_addr + memory_size, 1, FOLL_GET,
ne_mem_region->pages + i, NULL);
+
+ mmap_read_unlock(current->mm);
+
if (gup_rc < 0) {
rc = gup_rc;
--
2.32.0
Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v1 ] nitro_enclaves: Add mmap_read_lock() for the get_user_pages() call
2021-12-18 10:35 [PATCH v1 ] nitro_enclaves: Add mmap_read_lock() for the get_user_pages() call Andra Paraschiv
@ 2021-12-18 10:43 ` Paraschiv, Andra-Irina
2022-01-03 15:03 ` Paraschiv, Andra-Irina
2021-12-20 9:23 ` Vitaly Kuznetsov
1 sibling, 1 reply; 5+ messages in thread
From: Paraschiv, Andra-Irina @ 2021-12-18 10:43 UTC (permalink / raw)
To: linux-kernel, Greg KH
Cc: Alexandru Ciobotaru, Alexandru Vasile, Marcelo Cerri,
Paolo Bonzini, Stefano Garzarella, Stefan Hajnoczi, Tim Gardner,
Vitaly Kuznetsov, kvm, ne-devel-upstream, stable
On 18.12.2021 12:35, Andra Paraschiv wrote:
> After commit 5b78ed24e8ec (mm/pagemap: add mmap_assert_locked()
> annotations to find_vma*()), the call to get_user_pages() will trigger
> the mmap assert.
>
> static inline void mmap_assert_locked(struct mm_struct *mm)
> {
> lockdep_assert_held(&mm->mmap_lock);
> VM_BUG_ON_MM(!rwsem_is_locked(&mm->mmap_lock), mm);
> }
>
> [ 62.521410] kernel BUG at include/linux/mmap_lock.h:156!
> ...........................................................
> [ 62.538938] RIP: 0010:find_vma+0x32/0x80
> ...........................................................
> [ 62.605889] Call Trace:
> [ 62.608502] <TASK>
> [ 62.610956] ? lock_timer_base+0x61/0x80
> [ 62.614106] find_extend_vma+0x19/0x80
> [ 62.617195] __get_user_pages+0x9b/0x6a0
> [ 62.620356] __gup_longterm_locked+0x42d/0x450
> [ 62.623721] ? finish_wait+0x41/0x80
> [ 62.626748] ? __kmalloc+0x178/0x2f0
> [ 62.629768] ne_set_user_memory_region_ioctl.isra.0+0x225/0x6a0 [nitro_enclaves]
> [ 62.635776] ne_enclave_ioctl+0x1cf/0x6d7 [nitro_enclaves]
> [ 62.639541] __x64_sys_ioctl+0x82/0xb0
> [ 62.642620] do_syscall_64+0x3b/0x90
> [ 62.645642] entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> Add mmap_read_lock() for the get_user_pages() call when setting the
> enclave memory regions.
>
> Signed-off-by: Andra Paraschiv <andraprs@amazon.com>
> Cc: stable@vger.kernel.org
> ---
Greg, can you please include this fix in the queue for v5.16 and then in
the one for the v5.15 stable tree. Let me know if any updates are
necessary for the patch.
Thanks,
Andra
> drivers/virt/nitro_enclaves/ne_misc_dev.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> index 8939612ee0e0..6c51ff024036 100644
> --- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
> +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> @@ -886,8 +886,13 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
> goto put_pages;
> }
>
> + mmap_read_lock(current->mm);
> +
> gup_rc = get_user_pages(mem_region.userspace_addr + memory_size, 1, FOLL_GET,
> ne_mem_region->pages + i, NULL);
> +
> + mmap_read_unlock(current->mm);
> +
> if (gup_rc < 0) {
> rc = gup_rc;
>
Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1 ] nitro_enclaves: Add mmap_read_lock() for the get_user_pages() call
2021-12-18 10:43 ` Paraschiv, Andra-Irina
@ 2022-01-03 15:03 ` Paraschiv, Andra-Irina
0 siblings, 0 replies; 5+ messages in thread
From: Paraschiv, Andra-Irina @ 2022-01-03 15:03 UTC (permalink / raw)
To: linux-kernel, Greg KH
Cc: Alexandru Ciobotaru, Alexandru Vasile, Marcelo Cerri,
Paolo Bonzini, Stefano Garzarella, Stefan Hajnoczi, Tim Gardner,
Vitaly Kuznetsov, kvm, ne-devel-upstream, stable
On 18.12.2021 12:43, Paraschiv, Andra-Irina wrote:
>
>
> On 18.12.2021 12:35, Andra Paraschiv wrote:
>> After commit 5b78ed24e8ec (mm/pagemap: add mmap_assert_locked()
>> annotations to find_vma*()), the call to get_user_pages() will trigger
>> the mmap assert.
>>
>> static inline void mmap_assert_locked(struct mm_struct *mm)
>> {
>> lockdep_assert_held(&mm->mmap_lock);
>> VM_BUG_ON_MM(!rwsem_is_locked(&mm->mmap_lock), mm);
>> }
>>
>> [ 62.521410] kernel BUG at include/linux/mmap_lock.h:156!
>> ...........................................................
>> [ 62.538938] RIP: 0010:find_vma+0x32/0x80
>> ...........................................................
>> [ 62.605889] Call Trace:
>> [ 62.608502] <TASK>
>> [ 62.610956] ? lock_timer_base+0x61/0x80
>> [ 62.614106] find_extend_vma+0x19/0x80
>> [ 62.617195] __get_user_pages+0x9b/0x6a0
>> [ 62.620356] __gup_longterm_locked+0x42d/0x450
>> [ 62.623721] ? finish_wait+0x41/0x80
>> [ 62.626748] ? __kmalloc+0x178/0x2f0
>> [ 62.629768] ne_set_user_memory_region_ioctl.isra.0+0x225/0x6a0
>> [nitro_enclaves]
>> [ 62.635776] ne_enclave_ioctl+0x1cf/0x6d7 [nitro_enclaves]
>> [ 62.639541] __x64_sys_ioctl+0x82/0xb0
>> [ 62.642620] do_syscall_64+0x3b/0x90
>> [ 62.645642] entry_SYSCALL_64_after_hwframe+0x44/0xae
>>
>> Add mmap_read_lock() for the get_user_pages() call when setting the
>> enclave memory regions.
>>
>> Signed-off-by: Andra Paraschiv <andraprs@amazon.com>
>> Cc: stable@vger.kernel.org
>> ---
>
> Greg, can you please include this fix in the queue for v5.16 and then in
> the one for the v5.15 stable tree. Let me know if any updates are
> necessary for the patch.
>
Thank you, Greg, for helping with the merge of v2 for this patch.
Andra
>
>> drivers/virt/nitro_enclaves/ne_misc_dev.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c
>> b/drivers/virt/nitro_enclaves/ne_misc_dev.c
>> index 8939612ee0e0..6c51ff024036 100644
>> --- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
>> +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
>> @@ -886,8 +886,13 @@ static int ne_set_user_memory_region_ioctl(struct
>> ne_enclave *ne_enclave,
>> goto put_pages;
>> }
>> + mmap_read_lock(current->mm);
>> +
>> gup_rc = get_user_pages(mem_region.userspace_addr +
>> memory_size, 1, FOLL_GET,
>> ne_mem_region->pages + i, NULL);
>> +
>> + mmap_read_unlock(current->mm);
>> +
>> if (gup_rc < 0) {
>> rc = gup_rc;
Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 ] nitro_enclaves: Add mmap_read_lock() for the get_user_pages() call
2021-12-18 10:35 [PATCH v1 ] nitro_enclaves: Add mmap_read_lock() for the get_user_pages() call Andra Paraschiv
2021-12-18 10:43 ` Paraschiv, Andra-Irina
@ 2021-12-20 9:23 ` Vitaly Kuznetsov
2021-12-20 20:04 ` Paraschiv, Andra-Irina
1 sibling, 1 reply; 5+ messages in thread
From: Vitaly Kuznetsov @ 2021-12-20 9:23 UTC (permalink / raw)
To: Andra Paraschiv, linux-kernel
Cc: Alexandru Ciobotaru, Greg KH, Alexandru Vasile, Marcelo Cerri,
Paolo Bonzini, Stefano Garzarella, Stefan Hajnoczi, Tim Gardner,
kvm, ne-devel-upstream, stable, Andra Paraschiv
Andra Paraschiv <andraprs@amazon.com> writes:
> After commit 5b78ed24e8ec (mm/pagemap: add mmap_assert_locked()
> annotations to find_vma*()), the call to get_user_pages() will trigger
> the mmap assert.
>
> static inline void mmap_assert_locked(struct mm_struct *mm)
> {
> lockdep_assert_held(&mm->mmap_lock);
> VM_BUG_ON_MM(!rwsem_is_locked(&mm->mmap_lock), mm);
> }
>
> [ 62.521410] kernel BUG at include/linux/mmap_lock.h:156!
> ...........................................................
> [ 62.538938] RIP: 0010:find_vma+0x32/0x80
> ...........................................................
> [ 62.605889] Call Trace:
> [ 62.608502] <TASK>
> [ 62.610956] ? lock_timer_base+0x61/0x80
> [ 62.614106] find_extend_vma+0x19/0x80
> [ 62.617195] __get_user_pages+0x9b/0x6a0
> [ 62.620356] __gup_longterm_locked+0x42d/0x450
> [ 62.623721] ? finish_wait+0x41/0x80
> [ 62.626748] ? __kmalloc+0x178/0x2f0
> [ 62.629768] ne_set_user_memory_region_ioctl.isra.0+0x225/0x6a0 [nitro_enclaves]
> [ 62.635776] ne_enclave_ioctl+0x1cf/0x6d7 [nitro_enclaves]
> [ 62.639541] __x64_sys_ioctl+0x82/0xb0
> [ 62.642620] do_syscall_64+0x3b/0x90
> [ 62.645642] entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> Add mmap_read_lock() for the get_user_pages() call when setting the
> enclave memory regions.
>
> Signed-off-by: Andra Paraschiv <andraprs@amazon.com>
> Cc: stable@vger.kernel.org
In case commit 5b78ed24e8ec broke Nitro Enclaves driver, we need to
explicitly state this:
Fixes: 5b78ed24e8ec ("mm/pagemap: add mmap_assert_locked() annotations to find_vma*()")
> ---
> drivers/virt/nitro_enclaves/ne_misc_dev.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> index 8939612ee0e0..6c51ff024036 100644
> --- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
> +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> @@ -886,8 +886,13 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
> goto put_pages;
> }
>
> + mmap_read_lock(current->mm);
> +
> gup_rc = get_user_pages(mem_region.userspace_addr + memory_size, 1, FOLL_GET,
> ne_mem_region->pages + i, NULL);
> +
> + mmap_read_unlock(current->mm);
> +
This looks very much like get_user_pages_unlocked(), I think we can use
it instead of open-coding it.
> if (gup_rc < 0) {
> rc = gup_rc;
--
Vitaly
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1 ] nitro_enclaves: Add mmap_read_lock() for the get_user_pages() call
2021-12-20 9:23 ` Vitaly Kuznetsov
@ 2021-12-20 20:04 ` Paraschiv, Andra-Irina
0 siblings, 0 replies; 5+ messages in thread
From: Paraschiv, Andra-Irina @ 2021-12-20 20:04 UTC (permalink / raw)
To: Vitaly Kuznetsov, linux-kernel, Greg KH
Cc: Alexandru Ciobotaru, Alexandru Vasile, Marcelo Cerri,
Paolo Bonzini, Stefano Garzarella, Stefan Hajnoczi, Tim Gardner,
kvm, ne-devel-upstream, stable
On 20.12.2021 11:23, Vitaly Kuznetsov wrote:
>
> Andra Paraschiv <andraprs@amazon.com> writes:
>
>> After commit 5b78ed24e8ec (mm/pagemap: add mmap_assert_locked()
>> annotations to find_vma*()), the call to get_user_pages() will trigger
>> the mmap assert.
>>
>> static inline void mmap_assert_locked(struct mm_struct *mm)
>> {
>> lockdep_assert_held(&mm->mmap_lock);
>> VM_BUG_ON_MM(!rwsem_is_locked(&mm->mmap_lock), mm);
>> }
>>
>> [ 62.521410] kernel BUG at include/linux/mmap_lock.h:156!
>> ...........................................................
>> [ 62.538938] RIP: 0010:find_vma+0x32/0x80
>> ...........................................................
>> [ 62.605889] Call Trace:
>> [ 62.608502] <TASK>
>> [ 62.610956] ? lock_timer_base+0x61/0x80
>> [ 62.614106] find_extend_vma+0x19/0x80
>> [ 62.617195] __get_user_pages+0x9b/0x6a0
>> [ 62.620356] __gup_longterm_locked+0x42d/0x450
>> [ 62.623721] ? finish_wait+0x41/0x80
>> [ 62.626748] ? __kmalloc+0x178/0x2f0
>> [ 62.629768] ne_set_user_memory_region_ioctl.isra.0+0x225/0x6a0 [nitro_enclaves]
>> [ 62.635776] ne_enclave_ioctl+0x1cf/0x6d7 [nitro_enclaves]
>> [ 62.639541] __x64_sys_ioctl+0x82/0xb0
>> [ 62.642620] do_syscall_64+0x3b/0x90
>> [ 62.645642] entry_SYSCALL_64_after_hwframe+0x44/0xae
>>
>> Add mmap_read_lock() for the get_user_pages() call when setting the
>> enclave memory regions.
>>
>> Signed-off-by: Andra Paraschiv <andraprs@amazon.com>
>> Cc: stable@vger.kernel.org
>
> In case commit 5b78ed24e8ec broke Nitro Enclaves driver, we need to
> explicitly state this:
>
> Fixes: 5b78ed24e8ec ("mm/pagemap: add mmap_assert_locked() annotations to find_vma*()")
Added to the commit message.
>
>> ---
>> drivers/virt/nitro_enclaves/ne_misc_dev.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
>> index 8939612ee0e0..6c51ff024036 100644
>> --- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
>> +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
>> @@ -886,8 +886,13 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
>> goto put_pages;
>> }
>>
>> + mmap_read_lock(current->mm);
>> +
>> gup_rc = get_user_pages(mem_region.userspace_addr + memory_size, 1, FOLL_GET,
>> ne_mem_region->pages + i, NULL);
>> +
>> + mmap_read_unlock(current->mm);
>> +
>
> This looks very much like get_user_pages_unlocked(), I think we can use
> it instead of open-coding it.
Indeed, it's mentioned as well in the comment for
get_user_pages_unlocked() this pattern of mmap_read_lock() /
get_user_pages() / mmap_read_unlock().
Thank you for the feedback, Vitaly. I sent v2 that includes the
mentioned updates.
Thanks,
Andra
>
>> if (gup_rc < 0) {
>> rc = gup_rc;
>
> --
> Vitaly
>
Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-03 15:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-18 10:35 [PATCH v1 ] nitro_enclaves: Add mmap_read_lock() for the get_user_pages() call Andra Paraschiv
2021-12-18 10:43 ` Paraschiv, Andra-Irina
2022-01-03 15:03 ` Paraschiv, Andra-Irina
2021-12-20 9:23 ` Vitaly Kuznetsov
2021-12-20 20:04 ` Paraschiv, Andra-Irina
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.