Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sun, 28 Jul 2019 20:06:11 +0200
From:   Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= 
        <marmarek@invisiblethingslab.com>
To:     Souptick Joarder <jrdr.linux@gmail.com>
Cc:     akpm@linux-foundation.org, willy@infradead.org, mhocko@suse.com,
        boris.ostrovsky@oracle.com, jgross@suse.com, linux@armlinux.org.uk,
        robin.murphy@arm.com, xen-devel@lists.xenproject.org,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [Xen-devel] [PATCH v4 8/9] xen/gntdev.c: Convert to use
 vm_map_pages()
Message-ID: <20190728180611.GA20589@mail-itl>
References: <20190215024830.GA26477@jordon-HP-15-Notebook-PC>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="MGYHOYXEY6WxJCY8"
Content-Disposition: inline
In-Reply-To: <20190215024830.GA26477@jordon-HP-15-Notebook-PC>
User-Agent: Mutt/1.12+29 (a621eaed) (2019-06-14)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--MGYHOYXEY6WxJCY8
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Feb 15, 2019 at 08:18:31AM +0530, Souptick Joarder wrote:
> Convert to use vm_map_pages() to map range of kernel
> memory to user vma.
>=20
> map->count is passed to vm_map_pages() and internal API
> verify map->count against count ( count =3D vma_pages(vma))
> for page array boundary overrun condition.

This commit breaks gntdev driver. If vma->vm_pgoff > 0, vm_map_pages
will:
 - use map->pages starting at vma->vm_pgoff instead of 0
 - verify map->count against vma_pages()+vma->vm_pgoff instead of just
   vma_pages().

In practice, this breaks using a single gntdev FD for mapping multiple
grants.

It looks like vm_map_pages() is not a good fit for this code and IMO it
should be reverted.

> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
>  drivers/xen/gntdev.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>=20
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 5efc5ee..5d64262 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -1084,7 +1084,7 @@ static int gntdev_mmap(struct file *flip, struct vm=
_area_struct *vma)
>  	int index =3D vma->vm_pgoff;
>  	int count =3D vma_pages(vma);
>  	struct gntdev_grant_map *map;
> -	int i, err =3D -EINVAL;
> +	int err =3D -EINVAL;
> =20
>  	if ((vma->vm_flags & VM_WRITE) && !(vma->vm_flags & VM_SHARED))
>  		return -EINVAL;
> @@ -1145,12 +1145,9 @@ static int gntdev_mmap(struct file *flip, struct v=
m_area_struct *vma)
>  		goto out_put_map;
> =20
>  	if (!use_ptemod) {
> -		for (i =3D 0; i < count; i++) {
> -			err =3D vm_insert_page(vma, vma->vm_start + i*PAGE_SIZE,
> -				map->pages[i]);
> -			if (err)
> -				goto out_put_map;
> -		}
> +		err =3D vm_map_pages(vma, map->pages, map->count);
> +		if (err)
> +			goto out_put_map;
>  	} else {
>  #ifdef CONFIG_X86
>  		/*

--=20
Best Regards,
Marek Marczykowski-G=C3=B3recki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

--MGYHOYXEY6WxJCY8
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAl095BIACgkQ24/THMrX
1yziIAf/exzcYKVSO+KS0CX9O2QdFSocXv52LbbEaeXP7AvIDXtfXcdbvxrkBwyA
dM4LYJgnMPbjYusQKNqWNDwi16zSJJgNfM0F4g+B4Ch2wkPXqCsobfHILsV8/x96
uYVr05q30FJ5goCzeMvQMNdPwDHv6+xGalM5Zhl56Xj+BGUQNmKo5sw2dAvarOM2
vdJUiQvbaZSBYSLZnufgbaEoZsXKQpDJftX7uM2gt6qmW3OwcEyhhGVI9loMCyJ5
jCWaVsXNj3EW/pZpwSX2nJgygQEp0C0x6xIZrG9rPNt/mZClap556QsmZzUkZDN7
92r6MMVJJLYyM0f880I5KEOKsIGNaQ==
=PmYG
-----END PGP SIGNATURE-----

--MGYHOYXEY6WxJCY8--