Re: [PATCH v2 0/8] Fix several device private page reference counting issues

2022-10-25 Thread Vlastimil Babka (SUSE) 
On 9/28/22 14:01, Alistair Popple wrote:
> This series aims to fix a number of page reference counting issues in
> drivers dealing with device private ZONE_DEVICE pages. These result in
> use-after-free type bugs, either from accessing a struct page which no
> longer exists because it has been removed or accessing fields within the
> struct page which are no longer valid because the page has been freed.
> 
> During normal usage it is unlikely these will cause any problems. However
> without these fixes it is possible to crash the kernel from userspace.
> These crashes can be triggered either by unloading the kernel module or
> unbinding the device from the driver prior to a userspace task exiting. In
> modules such as Nouveau it is also possible to trigger some of these issues
> by explicitly closing the device file-descriptor prior to the task exiting
> and then accessing device private memory.

Hi, as this series was noticed to create a CVE [1], do you think a stable
backport is warranted? I think the "It is possible to launch the attack
remotely." in [1] is incorrect though, right?

It looks to me that patch 1 would be needed since the CONFIG_DEVICE_PRIVATE
introduction, while the following few only to kernels with 27674ef6c73f
(probably not so critical as that includes no LTS)?

Thanks,
Vlastimil

[1] https://nvd.nist.gov/vuln/detail/CVE-2022-3523

> This involves some minor changes to both PowerPC and AMD GPU code.
> Unfortunately I lack hardware to test either of those so any help there
> would be appreciated. The changes mimic what is done in for both Nouveau
> and hmm-tests though so I doubt they will cause problems.
> 
> To: Andrew Morton 
> To: linux...@kvack.org
> Cc: linux-ker...@vger.kernel.org
> Cc: amd-...@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> 
> Alistair Popple (8):
>   mm/memory.c: Fix race when faulting a device private page
>   mm: Free device private pages have zero refcount
>   mm/memremap.c: Take a pgmap reference on page allocation
>   mm/migrate_device.c: Refactor migrate_vma and migrate_deivce_coherent_page()
>   mm/migrate_device.c: Add migrate_device_range()
>   nouveau/dmem: Refactor nouveau_dmem_fault_copy_one()
>   nouveau/dmem: Evict device private memory during release
>   hmm-tests: Add test for migrate_device_range()
> 
>  arch/powerpc/kvm/book3s_hv_uvmem.c   |  17 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c |  19 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.h |   2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c |  11 +-
>  drivers/gpu/drm/nouveau/nouveau_dmem.c   | 108 +++
>  include/linux/memremap.h |   1 +-
>  include/linux/migrate.h  |  15 ++-
>  lib/test_hmm.c   | 129 ++---
>  lib/test_hmm_uapi.h  |   1 +-
>  mm/memory.c  |  16 +-
>  mm/memremap.c|  30 ++-
>  mm/migrate.c |  34 +--
>  mm/migrate_device.c  | 239 +---
>  mm/page_alloc.c  |   8 +-
>  tools/testing/selftests/vm/hmm-tests.c   |  49 +-
>  15 files changed, 516 insertions(+), 163 deletions(-)
> 
> base-commit: 088b8aa537c2c767765f1c19b555f21ffe555786

Re: [PATCH 3/3] mm/mempool: use might_alloc()

2022-06-14 Thread Vlastimil Babka (SUSE) 
On 6/5/22 17:25, Daniel Vetter wrote:
> mempool are generally used for GFP_NOIO, so this wont benefit all that
> much because might_alloc currently only checks GFP_NOFS. But it does
> validate against mmu notifier pte zapping, some might catch some
> drivers doing really silly things, plus it's a bit more meaningful in
> what we're checking for here.
> 
> Signed-off-by: Daniel Vetter 
> Cc: Andrew Morton 
> Cc: linux...@kvack.org

Reviewed-by: Vlastimil Babka 

> ---
>  mm/mempool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/mempool.c b/mm/mempool.c
> index b933d0fc21b8..96488b13a1ef 100644
> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -379,7 +379,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>   gfp_t gfp_temp;
>  
>   VM_WARN_ON_ONCE(gfp_mask & __GFP_ZERO);
> - might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM);
> + might_alloc(gfp_mask);
>  
>   gfp_mask |= __GFP_NOMEMALLOC;   /* don't allocate emergency reserves */
>   gfp_mask |= __GFP_NORETRY;  /* don't loop in __alloc_pages */

Re: [PATCH 1/3] mm/page_alloc: use might_alloc()

2022-06-14 Thread Vlastimil Babka (SUSE) 
On 6/5/22 17:25, Daniel Vetter wrote:
> ... instead of open codding it. Completely equivalent code, just
> a notch more meaningful when reading.
> 
> Signed-off-by: Daniel Vetter 
> Cc: Andrew Morton 
> Cc: linux...@kvack.org

Reviewed-by: Vlastimil Babka 

> ---
>  mm/page_alloc.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 2db95780e003..24d170cb 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5177,10 +5177,7 @@ static inline bool prepare_alloc_pages(gfp_t gfp_mask, 
> unsigned int order,
>   *alloc_flags |= ALLOC_CPUSET;
>   }
>  
> - fs_reclaim_acquire(gfp_mask);
> - fs_reclaim_release(gfp_mask);
> -
> - might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM);
> + might_alloc(gfp_mask);
>  
>   if (should_fail_alloc_page(gfp_mask, order))
>   return false;