Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <1550f363-a1bd-bbf9-a7d5-47520137c9f4@nvidia.com>
Date:   Thu, 27 Jan 2022 22:26:17 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.1
Subject: Re: [PATCH v2] mm: Fix invalid page pointer returned with FOLL_PIN
 gups
Content-Language: en-US
To:     Peter Xu <peterx@redhat.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Cc:     Jason Gunthorpe <jgg@ziepe.ca>,
        Andrew Morton <akpm@linux-foundation.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Jan Kara <jack@suse.cz>,
        =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= <jglisse@redhat.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
        Alex Williamson <alex.williamson@redhat.com>
References: <20220128061918.20121-1-peterx@redhat.com>
From:   John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <20220128061918.20121-1-peterx@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bVZCOEF6RCtoNmg5VWthYW9YclhUQTlNWFNBNG5qWkxPWi9yNUpiQmhYRERO?=
 =?utf-8?B?dlFGSnAzaWtaTVRxZldxcTJzLzhCRzQ2U3U5YjV0QjkvY2FHamtkK2V2Vjhr?=
 =?utf-8?B?NDVsOFpiR2tUL1pDZ1Z0MndrS2FmK2JoZGdyOG1wS2g4bm9wazNsVWFUSEdv?=
 =?utf-8?B?MlNnWk85WmNxU0IydWZlWlFBSnNxaUllTUlXTXhqdzQ4c2l2eVBva1duMTlw?=
 =?utf-8?B?TXo5Y2hrN3Y0ZWlwaU5IUDJ3SGY1ck1KMHF2ZTNGMENScGdBSmppbDhxNW1o?=
 =?utf-8?B?RWtEZUVQc1FHblUyWmRGMzBOSy9jNldTdFdtM0pwUlpCNDBCZDU0OTVRNkNV?=
 =?utf-8?B?Z1J6cXk5Y1dhQ1NsbXZlUnJvTGE5bFpOVEYvbU5Pa3h2YzMwMFphcHJNZmRC?=
 =?utf-8?B?aHk4eU0vNW9Jc1htcVpobmVPTFB1UExMdDlUbi9MOE94TkZpUERCNUNmSnlx?=
 =?utf-8?B?MVErNG1nRHhIRGVIcFgvYW5URGxIMWtndjQvSGQ3dkorOGhPb3UyL2k2c05Z?=
 =?utf-8?B?ZWxGRHhINzYxWGhYQ25JeVZsNDZHK2xSazNlU1dJc2UwUDBia0dqQkV3RUdl?=
 =?utf-8?B?TVg0OXZ1Z2NraVEzMm84TTBBNEhBRS9Ndm9hRjlvcFYycitCQjRZdEc1WmtT?=
 =?utf-8?B?WldQREtFdmNkNFMveGIyL3R6SWo5NVAyWkpHdUtxQUd6dDNRZVdHeXRRclAz?=
 =?utf-8?B?dXEreFB0d25JZ0Z0RjY5eHFJL1NMOFlvaENwNFNZSWRMbEpOQU4vTkFrcjdG?=
 =?utf-8?B?YURreGg1RUpvMGZYcnpGWEFjYTlvU2V4MHoreTFidXYrNjQyWWpxRW53SVk5?=
 =?utf-8?B?QUpIOUJwZGhVRWlsR09qSzl0U0RGeE5UUjN4K2crUVNOMitISlo5SXdxWWFk?=
 =?utf-8?B?WEtuK0FBMDRTK1pRSDlmSGpvdnphWkJhSXVzUTJkMkhFN2wxcGZtdENNbUVj?=
 =?utf-8?B?aENCdmloeE9tVWxqZHp6cVU2TUl3a3grazFsQ2pmblZ0U211OE1xR0NZTTAx?=
 =?utf-8?B?TXV3TEEwRWZ3K1BSRHU2VUhzZmcxUE96RFoydWUzUFBxUFRmYjcvSkt5UHBo?=
 =?utf-8?B?bTZSSVdPWk5ENkhHSi9qZjJEM1pHdUVuUlNVSEwvM3RKSnV0M0hLcFlJTkpX?=
 =?utf-8?B?SlFJV2FESjZNcGtXRENoY2xMcnNVL01YbEI0bDlla0tNaXJpeC9zcXFsQ29U?=
 =?utf-8?B?NjI5N3BBQmtLM1E4WktadzloRXhUQm8xL0NYUElHaGhGNzc5d1pjVXlMMzNW?=
 =?utf-8?B?cUNQTUJYaFNHV1JzV3JqR2ZkaG9CbXlQOWdoZk1LdzBwNS9nMlhGcFZwYTN2?=
 =?utf-8?B?MjgwSzB1Um5GZ2U1NG5URE41WnNraE9rcEptQ0JSQlhBRlFpbnlXVVRRWlNp?=
 =?utf-8?B?ZnNqa2U0SGlYbWd6MDY1ZE55cjdldUhyRFJHeDBoWld1TmxOd0VHSDRuYkFn?=
 =?utf-8?B?MVNxVS9RWmdlVXVqNmhuUXh6WHRlZk5pVGdmTUU4a20yTGdaMENPSXhRL2ln?=
 =?utf-8?B?Q0NUVWxRSmx6dFRnVTlSNjhZYzZwaWhPM1BOSVM2K3JCZDhPU3pZaEtGWGx2?=
 =?utf-8?B?Z2VZVUY0M05acVUzVFVjTDFNR3VLc3V6WmNhLy80UFJLOTdSZUMwYU9OTCtQ?=
 =?utf-8?B?TEoyQjI1MlUvTVRLdkNnNjlERkhhY1gyK3JVTWxYQlRpYzFIcFBZT0pDTjdn?=
 =?utf-8?B?VmhDalFBeksyWmRNWFhyWWFRNGFhSzJhVXZUK3pidVU5bGFmRys5YlB1YlhW?=
 =?utf-8?B?ZkJiT285WnZsc3ZiZElWYkJnREFtNVhyUlhNMHRRSDh0a3NyNWRGd1QzQzJq?=
 =?utf-8?B?dyttOSt3VWFOU3UvRjdZNEFZV2w5RjFKSC9xdnlnSnJTVnNYSEhQVGhXYUQw?=
 =?utf-8?B?Z1c0RTdLZjdBWHBBazZ4R09FK1NVT3ZWUlNKTjlYVS85Z2VzZ01aNWx4MlU1?=
 =?utf-8?B?Y1NoNTd2bkp1N1BoR3IxbzN1Z0hOQVhGZkNUWlRxalRUc1laTngxb1RodVJG?=
 =?utf-8?B?MENkT3NGcmZBZmR1UVFlemhmdEpNaWdFbnRHOCsveVFPQnJMZmFIeDdManZT?=
 =?utf-8?B?WjZZL1NCbEY5b3hveUhjWDNFaDRVekE4cnNST3hlWHRVN1BROSszUVBJQ0hI?=
 =?utf-8?B?WVRGMDFDbFBlcDJBZUlCdEVQYWVuczRkY296MDVFYW0xc09uYUtGcThuVExa?=
 =?utf-8?Q?W3u8TVxVvIAwV7xj89nc/3M=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 76eac4d2-00f3-46da-db2b-08d9e2271831
X-MS-Exchange-CrossTenant-AuthSource: BY5PR12MB4130.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jan 2022 06:26:18.8251
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: O4fz+A1kZ9AJqYYCRXxmgR3M+NMkJmzgcjkRUaCQu7Qy8Tyh5RxLlE7pSejul5lm4Ma19sPG1MUPkH21YVuGOw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB3787
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/27/22 22:19, Peter Xu wrote:
> Alex reported invalid page pointer returned with pin_user_pages_remote() from
> vfio after upstream commit 4b6c33b32296 ("vfio/type1: Prepare for batched
> pinning with struct vfio_batch").  This problem breaks NVIDIA vfio mdev.
> 
> It turns out that it's not the fault of the vfio commit; however after vfio
> switches to a full page buffer to store the page pointers it starts to expose
> the problem easier.
> 
> The problem is for VM_PFNMAP vmas we should normally fail with an -EFAULT then
> vfio will carry on to handle the MMIO regions.  However when the bug triggered,
> follow_page_mask() returned -EEXIST for such a page, which will jump over the
> current page, leaving that entry in **pages untouched.  However the caller is
> not aware of it, hence the caller will reference the page as usual even if the
> pointer data can be anything.
> 
> We had that -EEXIST logic since commit 1027e4436b6a ("mm: make GUP handle pfn
> mapping unless FOLL_GET is requested") which seems very reasonable.  It could
> be that when we reworked GUP with FOLL_PIN we could have overlooked that
> special path in commit 3faa52c03f44 ("mm/gup: track FOLL_PIN pages"), even if
> that commit rightfully touched up follow_devmap_pud() on checking FOLL_PIN when
> it needs to return an -EEXIST.
> 
> Attaching the Fixes to the FOLL_PIN rework commit, as it happened later than
> 1027e4436b6a.
> 

Reviewed-by: John Hubbard <jhubbard@nvidia.com>


thanks,
-- 
John Hubbard
NVIDIA

> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jérôme Glisse <jglisse@redhat.com>
> Cc: John Hubbard <jhubbard@nvidia.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Fixes: 3faa52c03f44 ("mm/gup: track FOLL_PIN pages")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Debugged-by: Alex Williamson <alex.williamson@redhat.com>
> Tested-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> v2:
> - Drop the WARN_ON_ONCE() [Jason, John]
> - Add Alex's Tested-by too, as after dropping the WARN_ON_ONCE() then the patch
>    is exactly the one that Alex helped on bug verification, hence very safe to
>    grant the credit alongside.
> ---
>   mm/gup.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/gup.c b/mm/gup.c
> index f0af462ac1e2..65575ae3602f 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -440,7 +440,7 @@ static int follow_pfn_pte(struct vm_area_struct *vma, unsigned long address,
>   		pte_t *pte, unsigned int flags)
>   {
>   	/* No page to get reference */
> -	if (flags & FOLL_GET)
> +	if (flags & (FOLL_GET | FOLL_PIN))
>   		return -EFAULT;
>   
>   	if (flags & FOLL_TOUCH) {