mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Dave Jones <davej@redhat.com>
Cc: Linux Kernel <linux-kernel@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Davide Libenzi <davidel@xmailserver.org>,
	Eric Wong <normalperson@yhbt.net>,
	Pekka Enberg <penberg@cs.helsinki.fi>,
	Peter Hurley <peter@hurleysoftware.com>
Subject: Re: epoll oops.
Date: Tue, 15 Oct 2013 17:48:38 +0200	[thread overview]
Message-ID: <20131015154838.GA32271@redhat.com> (raw)
In-Reply-To: <CA+55aFxOpT9YLf7WLM+SOsp5U2R1-Px0LiEsdqV0Adr897dOqQ@mail.gmail.com>

> Hmm? There might be other cases..

Yes.

Dave, perhaps you have vmcore? I have no idea if this is possible or
not, but perhaps you can look at eventpoll_release_file's frame and
print file->f_op ?

On 10/14, Linus Torvalds wrote:
>
> [ Adding Pekka to verify the SLAB_DESTROY_BY_RCU semantics

Just in case, we depend on SLAB_DESTROY_BY_RCU anyway, and ->sighand
in particular. lock_task_sighand() equally depends on it.

> Ok, Oleg, going back to that whole thread, I think that old bug went like this:

Yes, yes, thanks, I do remember what this patch does and why. Just
I forgot everything about eventpoll.c, I tried to read it only once
to fix that bug.

>  (b) signalfd is special, and it does a
>
>         poll_wait(file, &current->sighand->signalfd_wqh);
>
>      which means that the wait-queue isn't associated with the file
> lifetime at all. It cleans it up with signalfd_cleanup() if the signal
> handlers are removed. Normal (non-epoll) handling is safe, because
> "current->sighand" obviously cannot go away as long as the current
> thread (doing the polling) is in its poll/select handling.

Yes. and, just in case, the main problem is that sighand has no any
connection with the file. Unlike, say, tty which uses ->private_data.

>  (c) as a result, epoll and exit() can race, since the normal epoll
> cleanup() is serialized by the file being closed, and we're missing
> that for the case of sighand going away.

Yes. Before that 971316f0503a hack epoll can't even know if the task
which did signalfd_poll() exits and frees the active signalfd_wqh.
If for example that task forked a child before exit.

And the whole RCU logic is only needed if exit/ep_remove_wait_queue
actually race with each other.

> Agreed so far? Ugly, ugly, ugly,

Yes, ugly, agreed. d80e731ecab4 even tries to docunent that this all
is the hack.

> And it looks like it should work.

Yes... I tried to read this all again, and so far I do not see
anything wrong... signalfd_cleanup()->waitqueue_active() looks fine
too, afaics. We do not need to clear ->whead unconditionally, the
only caller of ep_ptable_queue_proc() is signalfd_poll(), and we are
the last thread which can use this ->sighand.

> Peter? Does a tty hangup end up actually possibly freeing the tty
> struct? Looking at it, I'm starting to think that it only affects
> f_op, and the "struct tty" stays around, in which case this is all
> fine.

Of course I can't answer, but at first glance file_tty() should not go
away in this case... If nothing else, tty_release() expects tty != NULL,
and it seems that priv->tty is never changed.

Oleg.


  reply	other threads:[~2013-10-15 15:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-14 15:46 Dave Jones
2013-10-14 17:31 ` Linus Torvalds
2013-10-14 19:17   ` Oleg Nesterov
2013-10-14 20:57   ` Linus Torvalds
2013-10-15 15:48     ` Oleg Nesterov [this message]
2013-10-15 19:28       ` Dave Jones
2013-10-16 22:39       ` Eric Wong
2013-10-17 13:53         ` Oleg Nesterov
2013-10-23  9:08     ` Pekka Enberg
2013-10-23 13:43     ` Peter Hurley

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=20131015154838.GA32271@redhat.com \
    --to=oleg@redhat.com \
    --cc=davej@redhat.com \
    --cc=davidel@xmailserver.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=normalperson@yhbt.net \
    --cc=penberg@cs.helsinki.fi \
    --cc=peter@hurleysoftware.com \
    --cc=torvalds@linux-foundation.org \
    --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

Powered by JetHome