linux-kernel - Re: general protection fault in get_futex

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200408150940.GM20730@hirez.programming.kicks-ass.net>
Date:   Wed, 8 Apr 2020 17:09:40 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     syzbot <syzbot+a95c7753605c9d219466@...kaller.appspotmail.com>,
        akpm@...ux-foundation.org, bgeffon@...gle.com,
        dvhart@...radead.org, linux-kernel@...r.kernel.org,
        mingo@...hat.com, peterx@...hat.com,
        syzkaller-bugs@...glegroups.com, torvalds@...ux-foundation.org
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@...radead.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) {