linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Andrea Arcangeli <aarcange@redhat.com>
Cc: Vlastimil Babka <vbabka@suse.cz>,
	Dmitry Vyukov <dvyukov@google.com>,
	syzbot 
	<bot+6a5269ce759a7bb12754ed9622076dc93f65a1f6@syzkaller.appspotmail.com>,
	Jan Beulich <JBeulich@suse.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Laurent Dufour <ldufour@linux.vnet.ibm.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Andy Lutomirski <luto@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	syzkaller-bugs@googlegroups.com,
	Thomas Gleixner <tglx@linutronix.de>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.com>, Hugh Dickins <hughd@google.com>,
	David Rientjes <rientjes@google.com>,
	linux-mm <linux-mm@kvack.org>,
	Thorsten Leemhuis <regressions@leemhuis.info>
Subject: Re: KASAN: use-after-free Read in __do_page_fault
Date: Wed, 1 Nov 2017 08:26:24 -0700	[thread overview]
Message-ID: <CA+55aFyxA2mdSEKaP7v2GTWY2qC971unym8grwe1VnxQRctaUA@mail.gmail.com> (raw)
In-Reply-To: <20171031191336.GA2799@redhat.com>

On Tue, Oct 31, 2017 at 12:13 PM, Andrea Arcangeli <aarcange@redhat.com> wrote:
>
> The problematic path for the return to userland (get_user_pages
> returns to kernel) is this one:
>
>         if (return_to_userland) {
>                 if (signal_pending(current) &&
>                     !fatal_signal_pending(current)) {
>                         /*
>                          * If we got a SIGSTOP or SIGCONT and this is
>                          * a normal userland page fault, just let
>                          * userland return so the signal will be
>                          * handled and gdb debugging works.  The page
>                          * fault code immediately after we return from
>                          * this function is going to release the
>                          * mmap_sem and it's not depending on it
>                          * (unlike gup would if we were not to return
>                          * VM_FAULT_RETRY).
>                          *
>                          * If a fatal signal is pending we still take
>                          * the streamlined VM_FAULT_RETRY failure path
>                          * and there's no need to retake the mmap_sem
>                          * in such case.
>                          */
>                         down_read(&mm->mmap_sem);
>                         ret = VM_FAULT_NOPAGE;
>                 }
>         }
>
> We could remove the above branch all together and then
> handle_userfault() would always return VM_FAULT_RETRY whenever it
> decides to release the mmap_sem.

Honestly, I would *much* prefer that.

>    The above makes debugging with gdb
> more user friendly and it potentially lowers the latency of signals as
> signals can unblock handle_userfault.

I don't disagree about that, but why don't you use VM_FAULT_RETRY and
not re-take the mmap_sem? Then we wouldn't have a special case for
userfaultfd at all.

I see the gdb issue, but I wonder if we shouldn't fix that differently
by changing the retry logic in the fault handler.

In particular, right now we do

 -  Retry at most once

 - handle fatal signals specially

and I think the gdb case actually shows that both of those decisions
may have been wrong, or at least something we could improve on?

Maybe we should return to user space on _any_ pending signal? That
might help latency for other things than gdb (think ^Z etc, but also
things that catch SIGSEGV and abort).

And maybe we should allow FAULT_FLAG_ALLOW_RETRY to just go on forever
- although that will want to verify that every case that returns
VM_FAULT_RETRY does wait for the condition it dropped the mmap
semaphore and is retrying for.

There aren't that many places that return VM_FAULT_RETRY. The
important one is lock_page_or_retry(), and that one does wait for the
page.

(There's one in mm/shmem.c too, and obviously the userfaultfd case,
but those seem to do waiting too).

So maybe we could just fix the gdb case without that userfaultfd hack?

             Linus

  reply	other threads:[~2017-11-01 15:26 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-30 19:12 syzbot
2017-10-30 19:15 ` Dmitry Vyukov
2017-10-31 12:00   ` Vlastimil Babka
2017-10-31 12:42     ` Dmitry Vyukov
2017-10-31 13:20       ` Vlastimil Babka
2017-10-31 13:57         ` Vlastimil Babka
2017-10-31 14:11           ` Kirill A. Shutemov
2017-10-31 14:28             ` Vlastimil Babka
2017-10-31 19:15               ` Andrea Arcangeli
2017-11-01  7:42                 ` Vlastimil Babka
2017-11-01 10:17                   ` Andrea Arcangeli
2017-11-01 12:14                     ` Vlastimil Babka
2017-10-31 15:37           ` Linus Torvalds
2017-10-31 19:13             ` Andrea Arcangeli
2017-11-01 15:26               ` Linus Torvalds [this message]
2017-11-02 19:36                 ` Andrea Arcangeli
2017-11-02 10:00           ` Laurent Dufour

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=CA+55aFyxA2mdSEKaP7v2GTWY2qC971unym8grwe1VnxQRctaUA@mail.gmail.com \
    --to=torvalds@linux-foundation.org \
    --cc=JBeulich@suse.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=bot+6a5269ce759a7bb12754ed9622076dc93f65a1f6@syzkaller.appspotmail.com \
    --cc=dvyukov@google.com \
    --cc=hpa@zytor.com \
    --cc=hughd@google.com \
    --cc=jpoimboe@redhat.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=ldufour@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@kernel.org \
    --cc=mhocko@suse.com \
    --cc=mingo@redhat.com \
    --cc=regressions@leemhuis.info \
    --cc=rientjes@google.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=tglx@linutronix.de \
    --cc=vbabka@suse.cz \
    --cc=x86@kernel.org \
    --subject='Re: KASAN: use-after-free Read in __do_page_fault' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).