All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Oleg Nesterov <oleg@tv-sign.ru>
Cc: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>,
	Roland McGrath <roland@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] tty: fix leakage of -ERESTARTSYS to userland
Date: Wed, 30 May 2007 11:05:13 -0700	[thread overview]
Message-ID: <20070530110513.b2ae50ad.akpm@linux-foundation.org> (raw)
In-Reply-To: <20070529184435.GA174@tv-sign.ru>

On Tue, 29 May 2007 22:44:35 +0400
Oleg Nesterov <oleg@tv-sign.ru> wrote:

> Spotted by Satoru Takeuchi.
> 
> kill_pgrp(task_pgrp(current)) sends the signal to the current's thread group,
> but can choose any sub-thread as a target for signal_wake_up(). This means
> that job_control() and tty_check_change() may return -ERESTARTSYS without
> signal_pending().
> 
> Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>
> 
> --- t/drivers/char/n_tty.c~	2007-04-05 12:18:26.000000000 +0400
> +++ t/drivers/char/n_tty.c	2007-05-28 10:57:58.000000000 +0400
> @@ -1191,6 +1191,7 @@ static int job_control(struct tty_struct
>  			    is_current_pgrp_orphaned())
>  				return -EIO;
>  			kill_pgrp(task_pgrp(current), SIGTTIN, 1);
> +			set_thread_flag(TIF_SIGPENDING);
>  			return -ERESTARTSYS;
>  		}
>  	}
> --- t/drivers/char/tty_io.c~	2007-04-05 12:18:26.000000000 +0400
> +++ t/drivers/char/tty_io.c	2007-05-29 22:15:52.000000000 +0400
> @@ -1121,7 +1121,8 @@ int tty_check_change(struct tty_struct *
>  		return 0;
>  	if (is_current_pgrp_orphaned())
>  		return -EIO;
> -	(void) kill_pgrp(task_pgrp(current), SIGTTOU, 1);
> +	kill_pgrp(task_pgrp(current), SIGTTOU, 1);
> +	set_thread_flag(TIF_SIGPENDING);
>  	return -ERESTARTSYS;
>  }
>  

Are there other callers of kill_pgrp() which have the same problem?

Perhaps we should have a kill_pgrp_self() which takes care of doing
this, rather than open-coding it.  Something with a comment which
explains what's going on ;)

  reply	other threads:[~2007-05-30 18:06 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-29 18:44 [PATCH] tty: fix leakage of -ERESTARTSYS to userland Oleg Nesterov
2007-05-30 18:05 ` Andrew Morton [this message]
2007-05-30 19:18   ` Oleg Nesterov
2007-05-31  0:48     ` Satoru Takeuchi
2007-05-31  1:08       ` Roland McGrath
2007-05-31  1:55         ` Satoru Takeuchi
2007-05-31  7:40           ` Roland McGrath

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=20070530110513.b2ae50ad.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@tv-sign.ru \
    --cc=roland@redhat.com \
    --cc=takeuchi_satoru@jp.fujitsu.com \
    /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.