Re: [PATCH v3 07/15] iommufd: PFN handling for iopt_pages

On Tue, Oct 25, 2022 at 03:12:16PM -0300, Jason Gunthorpe wrote:

diff --git a/drivers/iommu/iommufd/pages.c b/drivers/iommu/iommufd/pages.c

+static int pfn_reader_user_pin(struct pfn_reader_user *user,

• 	       struct iopt_pages *pages,
  

• 	       unsigned long start_index,
  

• 	       unsigned long last_index)
  

+{

• bool remote_mm = pages->source_mm != current->mm;
• unsigned long npages;
• uintptr_t uptr;
• long rc;
• if (!user->upages) {

• /* All undone in pfn_reader_destroy() */
  

• user->upages_len =
  

• 	(last_index - start_index + 1) * sizeof(*user->upages);
  

• user->upages = temp_kmalloc(&user->upages_len, NULL, 0);
  

• if (!user->upages)
  

• 	return -ENOMEM;
  

• }
• if (user->locked == -1) {

• /*
  

•  * The majority of usages will run the map task within the mm
  

•  * providing the pages, so we can optimize into
  

•  * get_user_pages_fast()
  

•  */
  

• user->locked = 0;
  

• if (remote_mm) {
  

• 	if (!mmget_not_zero(pages->source_mm)) {
  

• 		kfree(user->upages);
  

• 		user->upages = NULL;
  

Coverity reports BAD_FREE at user->upages here.

In iopt_pages_fill_xarray and iopt_pages_fill_from_mm, user->upages is assigned by shifting the out_pages input of iopt_pages_add_access that could be originated from vfio_pin_pages, I am not sure if the remote_mm and mmget_not_zero value checks can prevent this though.