From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: Kay Sievers <kay.sievers@vrfy.org>
Cc: Greg KH <greg@kroah.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
Lennart Poettering <lennart@poettering.net>,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] tty: add TIOCVHANGUP: time for revoke() in f_ops ?
Date: Fri, 18 Feb 2011 09:50:48 +0000 [thread overview]
Message-ID: <20110218095048.4e9f1e1a@lxorguk.ukuu.org.uk> (raw)
In-Reply-To: <1297964368.2165.1.camel@yio>
> 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
next prev parent reply other threads:[~2011-02-18 9:50 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-02-17 17:39 [PATCH] tty: add TIOCVHANGUP to allow clean tty shutdown of all ttys Kay Sievers
2011-02-18 9:50 ` Alan Cox [this message]
2011-02-22 23:15 ` [PATCH] tty: add TIOCVHANGUP: time for revoke() in f_ops ? Greg KH
2011-02-23 0:09 ` Alan Cox
2011-02-23 0:23 ` Lennart Poettering
2011-02-23 0:30 ` Alan Cox
2011-02-23 0:35 ` Lennart Poettering
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=20110218095048.4e9f1e1a@lxorguk.ukuu.org.uk \
--to=alan@lxorguk.ukuu.org.uk \
--cc=greg@kroah.com \
--cc=kay.sievers@vrfy.org \
--cc=lennart@poettering.net \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®