On Fri, Feb 18, 2011 at 09:50:48AM +0000, Alan Cox wrote: > > 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 ?) You could do that already today with the vhangup() syscall, right? > > + 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. But wouldn't this race still happen no matter if vhangup() is in the mix or not? I don't see how adding this ioctl changes anything here, what am I missing? > 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. It's not a "real" revoke, more like vhangup(file_descriptor) only. revoke() involves a lot more than just this. It would be great to have a real revoke() but that seems beyond this need at the moment. > 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) Again, I'm confused, how does the locking differ from vhangup() today? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
