linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrea Arcangeli <aarcange@redhat.com>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: syzbot+cbc64b24b2b2d54c07a9@syzkaller.appspotmail.com,
	Sasha Levin <Alexander.Levin@microsoft.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
	Al Viro <viro@zeniv.linux.org.uk>, Peter Xu <peterx@redhat.com>,
	Mike Rapoport <rppt@linux.vnet.ibm.com>,
	Mike Kravetz <mike.kravetz@oracle.com>
Subject: Re: KASAN: use-after-free Read in handle_userfault (2)
Date: Fri, 4 Jan 2019 18:10:13 -0500	[thread overview]
Message-ID: <20190104231013.GB4924@redhat.com> (raw)
In-Reply-To: <CACT4Y+Y+ZdBPXGhytZycqcLc_7GOR-VZB8KKuX_qUgxmiVT60w@mail.gmail.com>

On Wed, Jan 02, 2019 at 02:37:58PM +0100, Dmitry Vyukov wrote:
> If we are proceeding with "mm: some enhancements to the page fault
> mechanism", that's good as it will eliminate at least part of this
> output.

Agreed.

> There are 2 types of debug configs: ones add additional checks for
> machines and another add verbose output for humans. CONFIG_DEBUG_VM
> seems to be more of the first type of debug config -- additional
> checks for machines. I've seen some of the second type debug configs
> are prefixed with DEBUG_VERBOSE or something along these lines. Maybe
> it makes sense to split this out of CONFIG_DEBUG_VM. Since it's a
> "gray" check (rather then white/black check), it can't be used in
> CI/fuzzing setups anyways -- not possible to analyse thousands of
> cases manually (though maybe we actually hitting some that can be
> classified as kernel bugs).

The problem with the DEBUG_VERBOSE is that if the kernel needs a
rebuild it's only marginally better than having to ask the developer
to add a one liner dump_stack before rebuilding the kernel.

Next time we need it, it may be simpler to figure a dynamic tracing
trick than to ask a rebuild.

In addition of pursuing the VM_FAULT_RETRY improvements from Peter,
it'd be fine to drop it for now to avoid confusing the machines.

> Yes, the problem is way more general. As you noted it applies to 100K
> of EINVAL|EFAULT|ENOMEM, it's super hard to figure out what/where
> exactly goes wrong in the kernel getting only -22. But at the same

What stands out for this location is that it's the bailout point
of all non uffd compatible syscalls.

The chances such annotation turns out to be useful is much higher than
a random -EINVAL location, but in principle it's the same kind of
issue and I agree it'd be unpractical to annotate them manually.

> time we can't have all of these 100K places dump stacks. I don't know
> what's a good solution for this. Manually annotating 100K places does
> not look like the right way to go. Maybe kprobes can do this? In some
> cases I used CONFIG_KCOV  and kcovtrace
> (https://github.com/google/syzkaller/blob/master/tools/kcovtrace/kcovtrace.c)
> to collect kernel trace from a failing syscall.

If there is a de facto standard to search for a "syscall bailout call
trace" that can provide us that very same dump_stack, that should work
for this uffd issue too indeed.

KCOV may or may not be enabled in enterprise -debug kernels, but the
main limitation of the kcovtrace.c seems to be the lack of threading
support and the privilege inheritance through fork. While it's not
mandatory, the uffd manager is practically always implemented as a
thread of some process so whatever alternative to dump_stack() should
work with threads.

I think kprobes/ebpf should work provided debug info is available
(which is not always guaranteed), otherwise to obtain it without debug
info, it'd require a ftrace static trace point to use with the
function graph tracer I guess.

Thanks,
Andrea

  reply	other threads:[~2019-01-04 23:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-12  9:45 KASAN: use-after-free Read in handle_userfault (2) syzbot
2018-12-12  9:58 ` Dmitry Vyukov
2018-12-30  7:48   ` Dmitry Vyukov
2018-12-30 15:46     ` Andrea Arcangeli
2019-01-02 13:37       ` Dmitry Vyukov
2019-01-04 23:10         ` Andrea Arcangeli [this message]
2019-01-07  9:44           ` Dmitry Vyukov

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=20190104231013.GB4924@redhat.com \
    --to=aarcange@redhat.com \
    --cc=Alexander.Levin@microsoft.com \
    --cc=dvyukov@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mike.kravetz@oracle.com \
    --cc=peterx@redhat.com \
    --cc=rppt@linux.vnet.ibm.com \
    --cc=syzbot+cbc64b24b2b2d54c07a9@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=viro@zeniv.linux.org.uk \
    /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 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).