All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Kees Cook <keescook@chromium.org>
Cc: Fengguang Wu <fengguang.wu@intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: Re: yama_ptrace_access_check(): possible recursive locking detected
Date: Wed, 15 Aug 2012 15:01:59 +0200	[thread overview]
Message-ID: <20120815130159.GA3221@redhat.com> (raw)
In-Reply-To: <CAGXu5j+0FUYdBqsotn_vp1EbG=dcURAA1sxv+yFzaJuUkdAh0A@mail.gmail.com>

On 08/14, Kees Cook wrote:
>
> Okay, I've now managed to reproduce this locally. I added a bunch of
> debugging, and I think I understand what's going on. This warning is,
> actually, a false positive.

Sure. I mean that yes, this warning doesn't mean we already hit deadlock.

> get used recursively (the task_struct->alloc_lock), but they are
> separate instantiations ("task" is never "current").

Yes. But suppose that we have 2 tasks T1 and T2,

	- T1 does ptrace(PTRACE_ATTACH, T2);

	- T2 does ptrace(PTRACE_ATTACH, T1);

at the same time. This can lead to the "real" deadlock, no?

> So Oleg's suggestion of removing the locking around the reading of
> ->comm is wrong since it really does need the lock.

Nothing bad can happen without the lock. Yes, printk() can print
some string "in between" if we race with set_task_comm() but this
is all.

BTW, set_task_comm()->wmb() and memset() should die. There are
not needed afaics, and the comment is misleading.

Oleg.


  parent reply	other threads:[~2012-08-15 13:05 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-26 13:47 yama_ptrace_access_check(): possible recursive locking detected Fengguang Wu
2012-07-26 15:41 ` Oleg Nesterov
2012-07-30 17:00   ` Kees Cook
2012-08-10  1:39 ` Kees Cook
2012-08-10  1:52   ` Fengguang Wu
2012-08-14 21:16     ` Kees Cook
2012-08-15  3:01       ` Fengguang Wu
2012-08-15  5:56         ` Kees Cook
2012-08-15  8:05           ` Peter Zijlstra
2012-08-15 13:01           ` Oleg Nesterov [this message]
2012-08-15 14:30             ` Peter Zijlstra
2012-08-15 17:56               ` Oleg Nesterov
2012-08-15 18:09                 ` Kees Cook
2012-08-15 18:17                   ` Oleg Nesterov
2012-08-15 18:30                     ` Kees Cook
2012-08-15 18:44                   ` Alan Cox
2012-08-15 18:43                     ` Kees Cook

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=20120815130159.GA3221@redhat.com \
    --to=oleg@redhat.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=fengguang.wu@intel.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    /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.