public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Alan Cox <alan@redhat.com>
To: Oleg Nesterov <oleg@tv-sign.ru>
Cc: akpm@osdl.org, linux-kernel@vger.kernel.org,
	alan@lxorguk.ukuu.org.uk, alan@redhat.com, arjan@linux.intel.com,
	viro@zeniv.linux.org.uk
Subject: Re: + audit-accounting-tty-locking.patch added to -mm tree
Date: Tue, 5 Sep 2006 19:19:42 -0400	[thread overview]
Message-ID: <20060905231942.GA10997@devserv.devel.redhat.com> (raw)
In-Reply-To: <20060905230753.GA613@oleg>

On Wed, Sep 06, 2006 at 03:07:53AM +0400, Oleg Nesterov wrote:
> Nowadays ->signal/->sighand are _also_ protected by ->sighand->siglock.
> Unless you are current, you can't lock ->siglock directly (without holding
> tasklist_lock), you should use lock_task_sighand().

Gack, that makes current controlling tty locking horrible (and wrong almost
everywhere still across a clone)

> tty_io.c:
> 	->tty is set under task_lock()
> 
> 	->tty is cleared under lock_kernel() + tasklist_lock
> 
> 	except TIOCNOTTY, cleared under task_lock()
> 
> Note that include/linux/sched.h doesn't document that ->alloc_lock
> protects ->tty, it is only used in tty_io.c for that purpose, why?

Work in progress

> Btw, I think tiocsctty()/tty_open() is racy wrt to sys_setsid().
> tiocsctty() can see the result of '->signal->leader = 1' before
> sys_setsid() changed ->session/->pgrp and passed '->tty = NULL'.

Correct. I'm doing them bit by bit as I unpick them and check they
don't deadlock. If we need to take task_lock as well then its time
for set_controlling_tty() to get added.

Thanks for the signal lock explanation though. Now I've more idea wtf
is going on below the tty layer



      reply	other threads:[~2006-09-05 23:19 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200609052013.k85KDRWx010062@shell0.pdx.osdl.net>
2006-09-05 23:07 ` + audit-accounting-tty-locking.patch added to -mm tree Oleg Nesterov
2006-09-05 23:19   ` Alan Cox [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=20060905231942.GA10997@devserv.devel.redhat.com \
    --to=alan@redhat.com \
    --cc=akpm@osdl.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=arjan@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@tv-sign.ru \
    --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