[Devel] [RFC rh7 v4] ve/tty: vt -- Implement per VE support for console and terminals

[Cyrill Gorcunov]

On Fri, Aug 21, 2015 at 05:15:52PM +0300, Vladimir Davydov wrote:
> > +
> > +static void vtty_close(struct tty_struct *tty, struct file *filp)
> > +{
> > +
> > +	if (tty->count > 2)
> > +		return;
> > +
> > +	if (test_bit(TTY_IO_ERROR, &tty->flags))
> > +		return;
> 
> Why?

Actually this is redundant (I thought it would eliminate
the race on close I've had in my previous versions of
the patch but it doesn't bring any benefit by now). Thanks!

> 
> > +
> > +	if (tty->driver == vttys_driver) {
> > +		if (peer->count == 1) {
> > +			clear_bit(TTY_EXTRA_REF, &peer->flags);
> > +			peer->count--;
> 
> Here you read and modify peer->count. Why is it safe? You hold neither
> peer->legacy_mutex nor tty_mutex here.
> 
> Another question regarding synchronization is what prevents the
> following race, which results in leaking a vtty pair?
> 
> CPU0				CPU1
> ----				----
> opened slave			opened slave
> 
> [ master->count = 1, slave->count = 3 ]
> 
> closing slave			closing slave
>  tty_release
>   ->close (vtty_close)
>    tty->count > 2 => return
>    				 tty_release
> 				  ->close (vtty_close)
> 				   tty->count > 2 => return
>   lock tty_mutex
>   tty->count--;
>   unlock tty_mutex
> 				  lock tty_mutex
> 				  tty->count--;
> 				  unlock tty_mutex
> 
> [ master->count = 1, slave->count = 1 ]
> 
> If this race can happen, is it relevant to pty too?

I think so. In vanilla kernel pty lock is kept during the whole
@count manipulations. Moreover, when closing slave the lock
is taken on master peer as well.

I think we need to gather this changes into rh7.

	Cyrill