Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Huang, Yonghua" <yonghua.huang@intel.com>
To:     Daniel Vetter <daniel@ffwll.ch>
CC:     "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>,
        "Chatre, Reinette" <reinette.chatre@intel.com>,
        "Wang, Zhi A" <zhi.a.wang@intel.com>,
        "Wang, Yu1" <yu1.wang@intel.com>, "Li, Fei1" <fei1.li@intel.com>,
        Linux MM <linux-mm@kvack.org>,
        DRI Development <dri-devel@lists.freedesktop.org>
Subject: RE: [PATCH] virt: acrn: obtain pa from VMA with PFNMAP flag
Thread-Topic: [PATCH] virt: acrn: obtain pa from VMA with PFNMAP flag
Thread-Index: AQHYLEfg4LGJm4nTXkWIyXm40QwGzK2pDmuAgEICZEA=
Date:   Wed, 21 Sep 2022 12:23:47 +0000
Message-ID: <BYAPR11MB354411F3B1C3AE2423627147F74F9@BYAPR11MB3544.namprd11.prod.outlook.com>
References: <20220228022212.419406-1-yonghua.huang@intel.com>
 <YvOiZ2jp2Fv0Ex0J@phenom.ffwll.local>
In-Reply-To: <YvOiZ2jp2Fv0Ex0J@phenom.ffwll.local>
Accept-Language: en-US
Content-Language: en-US
dlp-reaction: no-action
dlp-version: 11.6.500.17
dlp-product: dlpe-windows
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Wx9MPx6v0Op6f7Zt3TnCKn5UFRobkIaIM07WGGFSjzFkDMjb2oSHG+or3cC6?=
 =?us-ascii?Q?ctLhYOs/fl9KjsqK45DhuPdgiE8kFGvanFxjGeylsiE4hmzpGJPFijdyjp9U?=
 =?us-ascii?Q?ZK6GhWZQQJ/7TCyt6N7tPRJIdvFTOIq4a6afOL6eaU8Xmg1uRdHSlDYY9b8s?=
 =?us-ascii?Q?rr03Cyv7zaZOOguvn15g5pUOF8krqfPvL2tjGZ+wXEkG37nbqr8hdPKSW+lT?=
 =?us-ascii?Q?VSmpqCn72gEsRkqapQ/yx9Q+kqLIopw/X5PpJmflCLieF56ZtnKxt9QTPk2b?=
 =?us-ascii?Q?RvQnapwxdI58ybOy4q3ZW5xwQCsYLHzvrgY3ICFn1I63q71SgWNMn4v5+yoL?=
 =?us-ascii?Q?rxJBeVVAxL2SBPRqbVHaVOpgK20ur+q34cjCDapgveqHB8ahKis4uxpuTbic?=
 =?us-ascii?Q?1ic0RqoZv+lLr28t1LD7dZ3N/zV9yvN6XU9WeI9oRdImejhvYIbMpefMohMy?=
 =?us-ascii?Q?BBtZFELPM4aZwOcDnIelH5uDmockOEQE10EKW8vSrvofXGUNgDdhy3qJrz13?=
 =?us-ascii?Q?HlEdmqi1iqOGm7pk4QTj0nrYGj1+h5+uov42kwrL+sCOAIbngOFYWHOFeWuT?=
 =?us-ascii?Q?TvEhFX7DQ15ngxdP6bHdH50UPkfFjKCYWf6Gsbfej+F/F0eDt3cqsK3H8E7t?=
 =?us-ascii?Q?k1q7qX3QLaZhlLqQovzfBl4AJS4j3+2Nff4JVISxCZr+1unng2jSO9WCP7tK?=
 =?us-ascii?Q?wFYt6d7xKDTzweUf3to+AjAak3TN3zMwJeBnmWwZmWuLu3Nw9mO9J6E/GWnv?=
 =?us-ascii?Q?8RlIiv+93Cs1JnaLTTwsUX8p3yQkgGlhgKAedR08K0g6helCUg6/RpYFBI3G?=
 =?us-ascii?Q?/wuEL5Sr37O+/3hE5Ui0Q8Tfu/I6zvM0RESx6QbJpZAwMx/c8DWRj41BKLi8?=
 =?us-ascii?Q?q34vXqYcYOejHwuzIuI3O8R+U8EX/2v1FGQfmkzo6CfuAApPeW8ANb17XNBP?=
 =?us-ascii?Q?E8SDwSbkNcYY34lLl+uX+Y/ZgPrGZxymX629pdq9uAOr6+JbvaUzRKHf6coQ?=
 =?us-ascii?Q?xBZdGQ2eOrf1dnfqLjdWZhNlStDH/Opu9vh6fSaqkDXbO/Ixf54E11XPxJnz?=
 =?us-ascii?Q?G3wZuK/GDkdK78Wuf5m9UQKkeKtC/kVajjpo3LR+jvl0w2GW0DUXnIis8rWW?=
 =?us-ascii?Q?GQ0LALjr5zqwoW09bCmOs7JyTooOtWwwmNFuWCSgd3sU6ZCu3yzysyvFV64o?=
 =?us-ascii?Q?wYRtpcnQPIWTrytXf9dNfUhyvj4LlcIm8QkAJRsdeioHRC0/euTU+uUpjjjX?=
 =?us-ascii?Q?WcrFX/QhoOYJm781p9DqjKt03Kk3Evu46BtvZGtWmRj7DM6y8XVBLsyc0BPx?=
 =?us-ascii?Q?Hek9n8q/jnIBo9RnIcPHlzOQSdQXTTyQlxqUpAwMkFV93+Pba7cedz4hw30y?=
 =?us-ascii?Q?KaoAqEis0V3DSitV5v31SuSMqbXpJk6fgMZflIJuKcf3fPYhqj09267Eas6h?=
 =?us-ascii?Q?iLzKvIpuAKF2SfRoJcuerGTW1CSp34guLF0ZrBA/u2MqKHEoflAJqAetZrSs?=
 =?us-ascii?Q?zqFxqafPBvLdYO/QItmAdUp7N/e0AmPeRhHrO+Ebvo4LGlo7gv8fJv5+pZFR?=
 =?us-ascii?Q?YArK4R09baQSwcrz5Eb7k1gC7gPMrIxVqGQNE5Z5?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3544.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e0c0aab0-a540-4d90-25b9-08da9bcc2202
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Sep 2022 12:23:47.2390
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: XR68joh7DU6kn85Q5PFGt0KaRiAL7M4l8wseXwTiYM8Q0BtSxSSwFWPG47bTvehjoMkcxEEnkbh4wCoA+EL/dQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7074
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Daniel,

 Thank you for this info, we will fix this issue.

Almost miss this mail, sorry!

-Yonghua

> -----Original Message-----
> From: Daniel Vetter <daniel@ffwll.ch>
> Sent: Wednesday, August 10, 2022 20:20
> To: Huang, Yonghua <yonghua.huang@intel.com>
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> stable@vger.kernel.org; Chatre, Reinette <reinette.chatre@intel.com>; Wan=
g,
> Zhi A <zhi.a.wang@intel.com>; Wang, Yu1 <yu1.wang@intel.com>; Li, Fei1
> <fei1.li@intel.com>; Linux MM <linux-mm@kvack.org>; DRI Development <dri-
> devel@lists.freedesktop.org>
> Subject: Re: [PATCH] virt: acrn: obtain pa from VMA with PFNMAP flag
>=20
> On Mon, Feb 28, 2022 at 05:22:12AM +0300, Yonghua Huang wrote:
> >  acrn_vm_ram_map can't pin the user pages with VM_PFNMAP flag  by
> > calling get_user_pages_fast(), the PA(physical pages)  may be mapped
> > by kernel driver and set PFNMAP flag.
> >
> >  This patch fixes logic to setup EPT mapping for PFN mapped RAM region
> > by checking the memory attribute before adding EPT mapping for them.
> >
> > Fixes: 88f537d5e8dd ("virt: acrn: Introduce EPT mapping management")
> > Signed-off-by: Yonghua Huang <yonghua.huang@intel.com>
> > Signed-off-by: Fei Li <fei1.li@intel.com>
> > ---
> >  drivers/virt/acrn/mm.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/virt/acrn/mm.c b/drivers/virt/acrn/mm.c index
> > c4f2e15c8a2b..3b1b1e7a844b 100644
> > --- a/drivers/virt/acrn/mm.c
> > +++ b/drivers/virt/acrn/mm.c
> > @@ -162,10 +162,34 @@ int acrn_vm_ram_map(struct acrn_vm *vm, struct
> acrn_vm_memmap *memmap)
> >  	void *remap_vaddr;
> >  	int ret, pinned;
> >  	u64 user_vm_pa;
> > +	unsigned long pfn;
> > +	struct vm_area_struct *vma;
> >
> >  	if (!vm || !memmap)
> >  		return -EINVAL;
> >
> > +	mmap_read_lock(current->mm);
> > +	vma =3D vma_lookup(current->mm, memmap->vma_base);
> > +	if (vma && ((vma->vm_flags & VM_PFNMAP) !=3D 0)) {
> > +		if ((memmap->vma_base + memmap->len) > vma->vm_end) {
> > +			mmap_read_unlock(current->mm);
> > +			return -EINVAL;
> > +		}
> > +
> > +		ret =3D follow_pfn(vma, memmap->vma_base, &pfn);
>=20
> This races, don't use follow_pfn() and most definitely don't add new user=
s. In
> some cases follow_pte, but the pte/pfn is still only valid for as long as=
 you hold
> the pte spinlock.
>=20
> > +		mmap_read_unlock(current->mm);
>=20
> Definitely after here there's zero guarantees about this pfn and it could=
 point at
> anything.
>=20
> Please fix, I tried pretty hard to get rid of follow_pfn(), but some of t=
hem are
> just too hard to fix (e.g. kvm needs a pretty hug rewrite to get it all s=
orted).
>=20
> Cheers, Daniel
>=20
> > +		if (ret < 0) {
> > +			dev_dbg(acrn_dev.this_device,
> > +				"Failed to lookup PFN at VMA:%pK.\n", (void
> *)memmap->vma_base);
> > +			return ret;
> > +		}
> > +
> > +		return acrn_mm_region_add(vm, memmap->user_vm_pa,
> > +			 PFN_PHYS(pfn), memmap->len,
> > +			 ACRN_MEM_TYPE_WB, memmap->attr);
> > +	}
> > +	mmap_read_unlock(current->mm);
> > +
> >  	/* Get the page number of the map region */
> >  	nr_pages =3D memmap->len >> PAGE_SHIFT;
> >  	pages =3D vzalloc(nr_pages * sizeof(struct page *));
> >
> > base-commit: 73878e5eb1bd3c9656685ca60bc3a49d17311e0c
> > --
> > 2.25.1
> >
>=20
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch