mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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

  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®