linux-kernel - Re: [PATCH 1/1] vfio/spapr_tce: convert get_user_pages() --> pin_user

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 27 May 2020 00:58:05 +0530
From:   Souptick Joarder <jrdr.linux@...il.com>
To:     John Hubbard <jhubbard@...dia.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Alex Williamson <alex.williamson@...hat.com>,
        Cornelia Huck <cohuck@...hat.com>, kvm@...r.kernel.org
Subject: Re: [PATCH 1/1] vfio/spapr_tce: convert get_user_pages() --> pin_user_pages()

Hi John,

On Sat, May 23, 2020 at 7:13 AM John Hubbard <jhubbard@...dia.com> wrote:
>
> This code was using get_user_pages*(), in a "Case 2" scenario
> (DMA/RDMA), using the categorization from [1]. That means that it's
> time to convert the get_user_pages*() + put_page() calls to
> pin_user_pages*() + unpin_user_pages() calls.
>
> There is some helpful background in [2]: basically, this is a small
> part of fixing a long-standing disconnect between pinning pages, and
> file systems' use of those pages.
>
> [1] Documentation/core-api/pin_user_pages.rst
>
> [2] "Explicit pinning of user-space pages":
>     https://lwn.net/Articles/807108/
>
> Cc: Alex Williamson <alex.williamson@...hat.com>
> Cc: Cornelia Huck <cohuck@...hat.com>
> Cc: kvm@...r.kernel.org
> Signed-off-by: John Hubbard <jhubbard@...dia.com>
> ---
>
> Hi,
>
> I'm compile-tested this, but am not able to run-time test, so any
> testing help is much appreciated!
>
> thanks,
> John Hubbard
> NVIDIA
>
>  drivers/vfio/vfio_iommu_spapr_tce.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> index 16b3adc508db..fe888b5dcc00 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -383,7 +383,7 @@ static void tce_iommu_unuse_page(struct tce_container *container,
>         struct page *page;
>
>         page = pfn_to_page(hpa >> PAGE_SHIFT);
> -       put_page(page);
> +       unpin_user_page(page);
>  }
>
>  static int tce_iommu_prereg_ua_to_hpa(struct tce_container *container,
> @@ -486,7 +486,7 @@ static int tce_iommu_use_page(unsigned long tce, unsigned long *hpa)
>         struct page *page = NULL;
>         enum dma_data_direction direction = iommu_tce_direction(tce);
>
> -       if (get_user_pages_fast(tce & PAGE_MASK, 1,
> +       if (pin_user_pages_fast(tce & PAGE_MASK, 1,
>                         direction != DMA_TO_DEVICE ? FOLL_WRITE : 0,
>                         &page) != 1)
>                 return -EFAULT;

There are few places where nr_pages is passed as 1 to get_user_pages_fast().
With similar conversion those will be changed to pin_user_pages_fast().

Does it make sense to add an inline like - pin_user_page_fast(), similar to
get_user_page_fast_only() ( now merged in linux-next) ?