Re: [PATCH] tty: add TIOCVHANGUP: time for revoke() in f_ops ?

From: Alan Cox
Date: Fri Feb 18 2011 - 04:50:16 EST


> Without this ioctl it would have to temporarily become the owner of
> the tty, then call vhangup() and then give it up again.

This is a hack - it's also unfortunately not actually sufficient or
complete which is why we didn't do it years ago. Sorry but if it was easy
it would have been in a long time back !


> + case TIOCVHANGUP:
> + if (!capable(CAP_SYS_ADMIN))

Is there any reason for not allowing revocation of a tty that you are
the owner of (ie one you could anyway take ownership of and hangup ?)


> + return -EPERM;
> + tty_vhangup(tty);

That raises a few locking questions. From a brief review of the code I
think its directly 1:1 equivalent to allowing a parallel vhangup and
carrier drop which we already do. The tty locks appear to cover this
correctly for the basic stuff although I further review of the ldisc
hangup area from someone with a strong stomache would be good.

You would still need to be very careful how you used it from the admin
side because the parallel

CPU1 CPU2
vhangup() chmod()
process vhangup
return
chown to user1
chmod to 777
syscall ends (fd
revocation takes effect)

Oops, 0wned

case is not handled by the paths you are using. So to actually do this
you need rather more infrastructure work to ensure the existing calls
have completed before returning.

At that point you've essentially provided revoke() for the tty layer so
it might well be implemented to be called as revoke() as well.

So I don't actually think the patch should be applied in this form,
because it simply adds an interface that will cause problems. Fixed to
return after the revocation is truely complete would be good though.

I'd guess you need to add a counter to the tty f_ops entry/exit points to
know when that occurs, and wake_up the revoke path when ready
(remembering two revokes in parallel shouldn't deadlock! so need
counting too)

In its current form the proposal isn't secure, so NAK, but I'd love to
see it resubmitted with the the other bits done to return at the right
moment.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/