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
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
