All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Amit Pundir <amit.pundir@linaro.org>
Cc: Stable <stable@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Jiri Olsa <jolsa@redhat.com>, Kees Cook <keescook@chromium.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Min Chong <mchong@google.com>,
	Stephane Eranian <eranian@google.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vince Weaver <vincent.weaver@maine.edu>,
	Ingo Molnar <mingo@kernel.org>,
	Ben Hutchings <ben@decadent.org.uk>,
	Suren Baghdasaryan <surenb@google.com>
Subject: Re: [PATCH for-3.18.y] perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race
Date: Wed, 10 Jan 2018 13:15:28 +0100	[thread overview]
Message-ID: <20180110121528.GB12483@kroah.com> (raw)
In-Reply-To: <1515505362-26760-1-git-send-email-amit.pundir@linaro.org>

On Tue, Jan 09, 2018 at 07:12:42PM +0530, Amit Pundir wrote:
> From: Peter Zijlstra <peterz@infradead.org>
> 
> commit 321027c1fe77f892f4ea07846aeae08cefbbb290 upstream.
> 
> Di Shen reported a race between two concurrent sys_perf_event_open()
> calls where both try and move the same pre-existing software group
> into a hardware context.
> 
> The problem is exactly that described in commit:
> 
>   f63a8daa5812 ("perf: Fix event->ctx locking")
> 
> ... where, while we wait for a ctx->mutex acquisition, the event->ctx
> relation can have changed under us.
> 
> That very same commit failed to recognise sys_perf_event_context() as an
> external access vector to the events and thereby didn't apply the
> established locking rules correctly.
> 
> So while one sys_perf_event_open() call is stuck waiting on
> mutex_lock_double(), the other (which owns said locks) moves the group
> about. So by the time the former sys_perf_event_open() acquires the
> locks, the context we've acquired is stale (and possibly dead).
> 
> Apply the established locking rules as per perf_event_ctx_lock_nested()
> to the mutex_lock_double() for the 'move_group' case. This obviously means
> we need to validate state after we acquire the locks.
> 
> Reported-by: Di Shen (Keen Lab)
> Tested-by: John Dias <joaodias@google.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Min Chong <mchong@google.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Vince Weaver <vincent.weaver@maine.edu>
> Fixes: f63a8daa5812 ("perf: Fix event->ctx locking")
> Link: http://lkml.kernel.org/r/20170106131444.GZ3174@twins.programming.kicks-ass.net
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> [bwh: Backported to 3.16:
>  - Use ACCESS_ONCE() instead of READ_ONCE()
>  - Test perf_event::group_flags instead of group_caps
>  - Add the err_locked cleanup block, which we didn't need before
>  - Adjust context]
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> ---
> This upstream patch is featured in recent Android Security bulletin.
> Picked up this backported patch from android-3.18. Build tested on 3.18.91

Thanks for this, now queued up.

greg k-h

      reply	other threads:[~2018-01-10 12:15 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-09 13:42 [PATCH for-3.18.y] perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race Amit Pundir
2018-01-10 12:15 ` Greg KH [this message]

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=20180110121528.GB12483@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=amit.pundir@linaro.org \
    --cc=ben@decadent.org.uk \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=keescook@chromium.org \
    --cc=mchong@google.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=vincent.weaver@maine.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.