Messages in this thread Patch in this message | | | Date | Wed, 8 Apr 2020 17:09:40 +0200 | From | Peter Zijlstra <> | Subject | Re: general protection fault in get_futex_key |
| |
On Wed, Apr 08, 2020 at 03:36:45PM +0200, Thomas Gleixner wrote:
> This means that > > get_user_pages_fast(address, 1, FOLL_WRITE, &page) > > returned 0, which is breaking the interface: > > * Returns number of pages pinned. This may be fewer than the number requested. > * If nr_pages is 0 or negative, returns 0. If no pages were pinned, returns > * -errno. > > nr_pages is clearly 1. So if the call fails, the number of pinned pages > is 0 and it should return a proper error number. It did before. > > From a quick look at the commit in question I assume it's the > > + if (fatal_signal_pending(current)) > + break; > > which can cause that to happen.
Yep, that's broken. It wants the below, just like all the other error paths in that loop.
--- Subject: mm/gup: Fix broken fatal_signal_pending() test
Just like all the other error returns in this loop, we need to set pages_done to the error value when it is zero.
Fixes: 4426e945df58 ("mm/gup: allow VM_FAULT_RETRY for multiple times") Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- diff --git a/mm/gup.c b/mm/gup.c index afce0bc47e70..14bce270b249 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -1326,8 +1326,11 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk, * start trying again otherwise it can loop forever. */ - if (fatal_signal_pending(current)) + if (fatal_signal_pending(current)) { + if (!pages_done) + pages_done = -ERESTARTSYS; break; + } ret = down_read_killable(&mm->mmap_sem); if (ret) {
| |