From: John Hubbard <jhubbard@nvidia.com>
To: Souptick Joarder <jrdr.linux@gmail.com>, Jason Gunthorpe <jgg@ziepe.ca>
Cc: Matthew Wilcox <willy@infradead.org>,
Andrew Morton <akpm@linux-foundation.org>,
Linux-MM <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm/gup.c: Handling ERR within unpin_user_pages()
Date: Mon, 14 Sep 2020 13:56:19 -0700 [thread overview]
Message-ID: <e56f3bd8-c025-cd68-a7b2-cb1e8eafae72@nvidia.com> (raw)
In-Reply-To: <CAFqt6zasxyAc9heAFuQ0xXuwpk8s7RThordModvLVDNDfFYvkA@mail.gmail.com>
On 9/14/20 1:52 PM, Souptick Joarder wrote:
> On Mon, Sep 14, 2020 at 7:38 PM Jason Gunthorpe <jgg@ziepe.ca> 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@infradead.org> 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
next prev parent reply other threads:[~2020-09-14 20:56 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-13 14:32 [PATCH] mm/gup.c: Handling ERR within unpin_user_pages() Souptick Joarder
2020-09-13 14:55 ` Matthew Wilcox
2020-09-14 1:50 ` Souptick Joarder
2020-09-14 1:50 ` Souptick Joarder
2020-09-14 14:08 ` Jason Gunthorpe
2020-09-14 20:52 ` Souptick Joarder
2020-09-14 20:52 ` Souptick Joarder
2020-09-14 20:56 ` John Hubbard [this message]
2020-09-14 21:01 ` Ira Weiny
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e56f3bd8-c025-cd68-a7b2-cb1e8eafae72@nvidia.com \
--to=jhubbard@nvidia.com \
--cc=akpm@linux-foundation.org \
--cc=jgg@ziepe.ca \
--cc=jrdr.linux@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.