public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Erica Bugden <ebugden@efficios.com>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: uprobes misses breakpoint insertion into VM_WRITE mappings
Date: Fri, 16 Mar 2018 17:52:39 +0100	[thread overview]
Message-ID: <20180316165239.GA28462@redhat.com> (raw)
In-Reply-To: <670124481.11073.1521146881571.JavaMail.zimbra@efficios.com>

On 03/15, Mathieu Desnoyers wrote:
>
> Hi,
>
> Erica has been working on extending test-cases for uprobes, and found
> something unexpected:
>
> Since commit e40cfce626a5 "uprobes: Restrict valid_vma(false) to skip VM_SHARED vmas"
> uprobes does not insert breakpoints into mappings mprotect'd as writeable.

Not really, VM_WRITE was illegal from the very beginning, this commit only
affects the "is_register == false" case.

> This issue can be reproduced by compiling a library without PIC (not using GOT),
> and then concurrently:
>
> A) Load the library (dynamic loader mprotect the code as writeable to do
>    the relocations, and then mprotect as executable),
>
> B) Enable a uprobe through perf.
>
> (it is a race window between the two mprotect syscalls)
>
> It appears that the following restriction in valid_vma() is responsible
> for this behavior:
>
>         if (is_register)
>                 flags |= VM_WRITE;
>
> I don't figure a clear explanation for this flag based on the function
> comment nor the commit changelog. Any idea on whether this is really
> needed ?

Because we do not want to modify the writable area. If nothing else, this
can break the application which writes to the page we are going to replace.

> Note that on uprobes unregister, it allows removing a breakpoint event
> on a writeable mapping,

Yes. Because a probed apllication can do mprotect() after the kernel installs
the breakpoint. And we have to remove this breakpoint in any case, even if
this is unsafe too.

Oleg.

  reply	other threads:[~2018-03-16 16:52 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-15 20:48 uprobes misses breakpoint insertion into VM_WRITE mappings Mathieu Desnoyers
2018-03-16 16:52 ` Oleg Nesterov [this message]
2018-03-22 21:48   ` Mathieu Desnoyers

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=20180316165239.GA28462@redhat.com \
    --to=oleg@redhat.com \
    --cc=ebugden@efficios.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    /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