Subject: re: Problems with xsrc X server on -current
To: None <bad@ora.de, greywolf@starwolf.com, mrg@eterna.com.au>
From: Chris Torek <torek@BSDI.COM>
List: port-sparc
Date: 02/27/1999 17:58:57
>* Greywolf puzzles over this.
>You're passing an fd to TIOCCONS in the first place.

Yes -- but we really need to get *two* fd's!

The (current, 4BSD-origin) TIOCCONS ioctl is handled in kern_tty.c:

	case TIOCCONS:			/* become virtual console */
		if (*(int *)data) {
			... release old constty ...
			constty = tp;
		} else if (tp == constty)
			constty = NULL;
		break;

where "tp" is the tty corresponding to the fd that did the ioctl.
Hence, to "become" the console, xterm opens a pty as usual, and then
applies the TIOCCONS ioctl to that pty.

Nowhere in all of this process does anyone actually open /dev/console,
hence all the usual permissions tests never apply.

My suggestion is basically to do "corrected TIOCCONS" in the console
driver, rather than in tty.c, so that you need to be able to open
/dev/console in order to do the ioctl.  This puts ordinary file
permissions back into the picture.  The target for the redirected
console (or, more generally -- all this currently applies to the
magic variable "constty", but it really should just be stream-of-data
stuff) has to come from somewhere though, hence I suggest that the
"data" part of the new ioctl gives the fd for the target:

int
cnioctl(...)
{
	int targfd;
	struct file *fp;
	struct tty *targtp;
	...
	case TIOCCONSREDIR:	/* XXX should be generic TIOCREDIR? */
		/*
		 * At this point, "tp" is the tty struct for /dev/console,
		 * and "p" and "data" are as usual.
		 */
		targfd = *(int *)data;

		/* This part is optional, depending on how you handle
		   REDIRs from /dev/console to /dev/console, below. */
		if (targfd < 0) {
			constty = NULL;
			break;
		}
		if ((error = getvnode(p->p_fd, targfd, &fp)) != 0)
			return (error);

		/* Make sure it's a tty, etc, and find its "ttyp" --
		   not sure how you can do this in NetBSD.  Here is
		   one way, that requires adding an "internal only"
		   tty ioctl, in ttioctl().  (Users can call this,
		   but not really do anything interesting with it.) */
		if ((error = vn_ioctl(fp, TIOCTTYP, &targtp, p)) != 0)
			return (error);

		/* Optional, but if we do not have this, REDIR of
		   /dev/console to itself will cause future REDIRs
		   to fail.  (This could be considered a feature!)  */
		if (targtp == tp) {
			constty = NULL;
			break;
		}

		/* this next part should look familiar */
		if (constty && constty != targtp &&
		    ((constty->t_state & (TS_CARR_ON | TS_ISOPEN)) ==
		     (TS_CARR_ON | TS_ISOPEN))
			return (EBUSY);
		constty = targtp;
		break;

This (new) TIOCTTYP is no uglier than the existing DIOCGPART. :-)
(Maybe it should be spelled TIOCGTTYP?)

There *is* another way to do this (using the existing TIOCCONS), by
having the ttioctl() code vn_open "/dev/console" to see if the
current process is allowed to do that.  That might look something
like:

		struct nameidata nd;
		struct vnode *vp;

		NDINIT(&nd, LOOKUP, FOLLOW, UIO_SYSSPACE, "/dev/console", p);
		if ((error = namei(&nd)) != 0)
			return (error);
		vp = nd.ni_vp;
		if (vp->v_type != VCHR)
			panic("TIOCCONS");
		error = VOP_ACCESS(vp, VWRITE, p->p_ucred, p);
		vput(vp);
		if (error)
			return (error);
		... proceed with TIOCCONS as usual ...

This has the disadvantage of hardcoding in "/dev/console", of
course.  It also seems to me to be entirely the wrong way around
-- the operation of cross-connecting fd's should supply both fd's,
even if it is currently a special case just for the console.

Chris