Re: [PATCH 1/2] rapidio: fix an error in get_user_pages_fast() error handling

[Andrew Morton]

On Sun, 17 May 2020 16:56:19 -0700 John Hubbard <jhubbard@xxxxxxxxxx> wrote:

> In the case of get_user_pages_fast() returning fewer pages than
> requested, rio_dma_transfer() does not quite do the right thing.
> It attempts to release all the pages that were requested, rather
> than just the pages that were pinned.
> 
> Fix the error handling so that only the pages that were successfully
> pinned are released.
> 
> ...
>
> --- a/drivers/rapidio/devices/rio_mport_cdev.c
> +++ b/drivers/rapidio/devices/rio_mport_cdev.c
> @@ -877,6 +877,11 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
>  				rmcd_error("pinned %ld out of %ld pages",
>  					   pinned, nr_pages);
>  			ret = -EFAULT;
> +			/*
> +			 * Set nr_pages up to mean "how many pages to unpin, in
> +			 * the error handler:
> +			 */
> +			nr_pages = pinned;
>  			goto err_pg;
>  		}

The code is a bit odd.  If (xfer->loc_addr == 0) then we do the `else'
stuff then fall through to

err_pg:
	if (!req->page_list) {
		for (i = 0; i < nr_pages; i++)
			put_page(page_list[i]);
		kfree(page_list);
	}

all of which is a big no-op because nr_pages==0 and page_list==NULL,
but it could all be easily avoided.

Oh well.  Reviewed-by:me.