All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: Greg KH <greg@kroah.com>
Cc: Kay Sievers <kay.sievers@vrfy.org>,
	Valdis.Kletnieks@vt.edu,
	Lennart Poettering <mzxreary@0pointer.de>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Werner Fink <werner@suse.de>, Jiri Slaby <jslaby@suse.cz>
Subject: Re: tty: add 'active' sysfs attribute to tty0 and console device
Date: Thu, 18 Nov 2010 10:15:19 +0000	[thread overview]
Message-ID: <20101118101519.11729a74@lxorguk.ukuu.org.uk> (raw)
In-Reply-To: <20101118012734.GA8558@kroah.com>

> 	- the existing ioctl is broken and no userspace program can use
> 	  it properly, so it might as well be removed.

You can use it happily for various things providing you hold the vt
switch lock. It's not a very good API and wants something doing about it.
However we can't deprecate it using either of the proposed patches
because neither of them cover the needed functionality.

> 	- Kay's patch is one proposed solution for what userspace is
> 	  wanting to learn about ttys.  Werner's is another one.

Kay's patch is useless. As we've demonstrated by as you call it "going
off topic" it can't actually provide a credible security model. Its
providing a variable without holding the locks that make the value
meaningful. Thats as basic an error I can think of.

We wouldn't accept kernel code which did

	mutex_lock(&foo->lock)
	temp = foo->only_valid_under_lock;
	mutex_unlock(&foo->lock);
	return temp;	/* ma invalid by now */

especially when it was going to be used for permission management. We'd
call it a bug. So why are we proposing to add an API that does exactly
that ?

> I can do any one, or multiple things from the following options:
> 
> 	- disable the existing ioctl to return an error so that no new
> 	  userspace program starts to use it thinking it is valid
> 	- accept Werner's patch for those who like proc files
> 	- accept Kay's patch
> 
> Any suggestions?

None of the above. In fact the current situation is better than either of
the patches because it's equally broken and involves no more broken APIs !

Doing it right means:
- deprecating the existing ioctl (because it's a dumb interface and Kay
  is right about that)
- adding a proper event device which is pollable and returns the same
  events (and more) using a small kfifo queue. Trivial to code and will
  not add another API we'll need to deprecate again the moment anyone
  wants to use it.
- support synchronous switching by that interface or verify the existing
  vt locking interfaces will do the job in conjunction with it.

Kay's approach doesn't solve three things

- You can't use the data to implement proper secure desktop switching
- It doesn't support moving to multiple active vts at a time
- You can't deprecate the wait event interface unless you replace all the
  features of it that people use (eg resize events)

So we will be back here again doing the same thing deprecating Kay's
interface if we go that way. Having an event device actually lets us
solve the problem properly, and if we are careful about the message
formats cover the fact the KMS folks and others want multiple "live"
virtual consoles at a time.

If you have a /dev/vtmanager or similar and opening it allocates a kfifo
of a page into which we stuff the events we currently post for waitevent
then the code is trivial and it does what Kay needs as well as being able
to cover the rest of the WAITEVENT interface and future multi-desktop
stuff.

So its trivial to do the job properly, which makes the existing patches
all the wrong thing to do.

Alan

  parent reply	other threads:[~2010-11-18 10:16 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-16 15:46 tty: add 'active' sysfs attribute to tty0 and console device Kay Sievers
2010-11-16 15:57 ` Alan Cox
2010-11-16 16:13   ` Kay Sievers
2010-11-16 17:14     ` Alan Cox
2010-11-16 18:51       ` Kay Sievers
2010-11-16 19:55         ` Alan Cox
2010-11-16 20:15           ` Kay Sievers
2010-11-16 20:49             ` Alan Cox
2010-11-16 21:29               ` Kay Sievers
2010-11-16 21:42               ` Lennart Poettering
2010-11-16 22:51                 ` Alan Cox
2010-11-16 22:58                   ` Lennart Poettering
2010-11-16 23:04                     ` Alan Cox
2010-11-16 23:18                       ` Lennart Poettering
2010-11-16 23:49                         ` Etched Pixels
2010-11-16 21:36           ` Lennart Poettering
2010-11-16 22:56             ` Alan Cox
2010-11-16 23:10               ` Lennart Poettering
2010-11-16 23:45                 ` Alan Cox
2010-11-17 16:31                 ` John Stoffel
2010-11-17 22:01                 ` Valdis.Kletnieks
2010-11-17 23:40                   ` Kay Sievers
2010-11-17 23:56                     ` Alan Cox
2010-11-18  1:27                       ` Greg KH
2010-11-18  1:48                         ` Lennart Poettering
2010-11-18  1:53                           ` Greg KH
2010-11-18  2:29                             ` Lennart Poettering
2010-11-18 11:00                             ` Dr. Werner Fink
2010-11-18 11:23                               ` Alan Cox
2010-11-18 12:12                                 ` Dr. Werner Fink
2010-11-18 12:58                                   ` Alan Cox
2010-11-18 13:14                                     ` Dr. Werner Fink
2010-11-18 14:41                                       ` Alan Cox
2010-11-19 13:21                                         ` Dr. Werner Fink
2010-11-19 15:47                                           ` Alan Cox
2010-11-19 17:07                                             ` Dr. Werner Fink
2010-11-19 18:02                                             ` Greg KH
2010-11-19 18:41                                               ` Dr. Werner Fink
2010-11-20 12:40                                                 ` Alan Cox
2010-12-01 11:15                                                   ` Dr. Werner Fink
2010-11-18 12:04                               ` Kay Sievers
2010-11-18 10:15                         ` Alan Cox [this message]
2010-11-18 11:55                           ` Kay Sievers
2010-11-18 13:01                             ` Alan Cox
     [not found] <20101201112004.12d78cd7@lxorguk.ukuu.org.uk>
2010-12-01 12:32 ` Dr. Werner Fink
     [not found]   ` <tiocgdev1@mdm.bga.com>
2010-12-03 11:48     ` Dr. Werner Fink

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=20101118101519.11729a74@lxorguk.ukuu.org.uk \
    --to=alan@lxorguk.ukuu.org.uk \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=greg@kroah.com \
    --cc=jslaby@suse.cz \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mzxreary@0pointer.de \
    --cc=werner@suse.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.