linux-kernel - Re: [RFC PATCH v2] xen/privcmd: Convert get_user_pages*() to 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]

Message-ID: <c68a3805-080f-22c3-a4d3-f03be6b32176@oracle.com>
Date:   Tue, 23 Jun 2020 13:41:33 -0400
From:   Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:     Souptick Joarder <jrdr.linux@...il.com>, jgross@...e.com,
        sstabellini@...nel.org
Cc:     xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org,
        John Hubbard <jhubbard@...dia.com>
Subject: Re: [RFC PATCH v2] xen/privcmd: Convert get_user_pages*() to
 pin_user_pages*()

On 6/23/20 7:58 AM, Souptick Joarder wrote:
> In 2019, we introduced pin_user_pages*() and now we are converting
> get_user_pages*() to the new API as appropriate. [1] & [2] could
> be referred for more information. This is case 5 as per document [1].
>
> As discussed, pages need to be marked as dirty before unpinned it.
>
> Previously, if lock_pages() end up partially mapping pages, it used
> to return -ERRNO due to which unlock_pages() have to go through
> each pages[i] till *nr_pages* to validate them. This can be avoided
> by passing correct number partially mapped pages & -ERRNO separately
> while returning from lock_pages() due to error.
> With this fix unlock_pages() doesn't need to validate pages[i] till
> *nr_pages* for error scenario.


This should be split into two patches please. The first one will fix the
return value bug (and will need to go to stable branches) and the second
will use new routine to pin pages.


> @@ -580,25 +580,30 @@ static long privcmd_ioctl_mmap_batch(
>  
>  static int lock_pages(
>  	struct privcmd_dm_op_buf kbufs[], unsigned int num,
> -	struct page *pages[], unsigned int nr_pages)
> +	struct page *pages[], unsigned int nr_pages, int *errno)


I'd prefer if you used more traditional way of returning error code by
the function, and pass the number of pinned pages as an argument. This
will also make call site simpler.


-boris