On 9/14/20 1:52 PM, Souptick Joarder wrote:
On Mon, Sep 14, 2020 at 7:38 PM Jason Gunthorpe <jgg@xxxxxxxx> wrote:
On Mon, Sep 14, 2020 at 07:20:34AM +0530, Souptick Joarder wrote:
On Sun, Sep 13, 2020 at 8:25 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
On Sun, Sep 13, 2020 at 08:02:35PM +0530, Souptick Joarder wrote:
It is possible that a buggy caller of unpin_user_pages()
(specially in error handling path) may end up calling it with
npages < 0 which is unnecessary.
@@ -328,6 +328,9 @@ void unpin_user_pages(struct page **pages, unsigned long npages)
{
unsigned long index;
+ if (WARN_ON_ONCE(npages < 0))
+ return;
But npages is unsigned long. So it can't be less than zero.
Sorry, I missed it.
Then, it means if npages is assigned with -ERRNO by caller, unpin_user_pages()
may end up calling a big loop, which is unnecessary.
How will a caller allocate memory of the right size and still manage
to call with the wrong npages? Do you have an example of a broken caller?
These are two broken callers which might end up calling unpin_user_pages()
with -ERRNO.
drivers/rapidio/devices/rio_mport_cdev.c#L952
drivers/misc/mic/scif/scif_rma.c#L1399
They both are in error handling paths, so might not have any serious impact.
But theoretically possible.
Eventually, I settled on fixing up the callers so that they match the gup/pup
API better. In other words, gup/pup has signed int for both input and return
value, and the callers need to handle that perfectly.
So let's fix up the callers.
thanks,
--
John Hubbard
NVIDIA
