linux-kernel - Re: [PATCH 1/2] rapidio: fix an error in get_user_pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20200518143723.2c3159ddd53345b9cde5d869@linux-foundation.org>
Date:   Mon, 18 May 2020 14:37:23 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     John Hubbard <jhubbard@...dia.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Matt Porter <mporter@...nel.crashing.org>,
        Alexandre Bounine <alex.bou9@...il.com>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        <linux-media@...r.kernel.org>, <stable@...r.kernel.org>
Subject: Re: [PATCH 1/2] rapidio: fix an error in get_user_pages_fast()
 error handling

On Sun, 17 May 2020 16:56:19 -0700 John Hubbard <jhubbard@...dia.com> 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.