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

From: Andrew Morton
Date: Mon May 18 2020 - 17:37:31 EST


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.