BSD tty security, part 3: How to Fix It

[Keith Muller]

In article <3739:May701:40:0591 at kramden.acf.nyu.edu>, brnstnd at kramden.acf.nyu.edu (Dan Bernstein) writes:
> In article <19045 at sdcc6.ucsd.edu> muller at sdcc10.ucsd.edu (Keith Muller) writes:

For the sake of reducing net noise only one part of Dans message bears
comment.....

> Obviously you don't understand what an I/O operation is. If someone is
> performing a read operation or write operation on a tty, he has already
> gotten past the access checks in the open file table, and terminating
> permission there doesn't stop the operations in progress.

Wrong. You failed to read my suggestion. The file pointer has to be carried down
with the io op from all points. This means adding the file pointer as a
single arg. all along the path from the ssyscall to the line disc. (This is
a simple change).

For example here is a partial sample of the code. This will
work with the unmodified 4.3 Tahoe vhangup().

In routine ino_rw() from sys_inode.c This shows the file pointer addition
for read/writes
48c48
< 	error = rwip(ip, uio, rw);
---
> 	error = rwip(ip, uio, rw, fp);
83c83
< rwip(ip, uio, rw)
---
> rwip(ip, uio, rw, fp)
86a87
> 	struct file *fp;
111c112
< 			error = (*cdevsw[major(dev)].d_read)(dev, uio);
---
> 			error = (*cdevsw[major(dev)].d_read)(dev, uio, fp);
114c115
< 			error = (*cdevsw[major(dev)].d_write)(dev, uio);
---
> 			error = (*cdevsw[major(dev)].d_write)(dev, uio, fp);

-----------------------------
Partial sample diffs for tty_tty.c (tahoe)
This shows how the fp is added as an arg to pass to the device ( from
the indirect controlling tty device). (only read is shown).
22a23
> #include "file.h"
36c37
< syread(dev, uio)
---
> syread(dev, uio, fp)
38a40
> 	struct file *fp;
43c45
< 	return ((*cdevsw[major(u.u_ttyd)].d_read)(u.u_ttyd, uio));
---
> 	return ((*cdevsw[major(u.u_ttyd)].d_read)(u.u_ttyd, uio, fp));
--------------------------------
-------------------------
partial diffs for tty_pty.c This also shows the extra arg being passed along by
a tty device to the line disp.
***************
*** 95,103 ****
  	ptcwakeup(tp, FREAD|FWRITE);
  }
  
! ptsread(dev, uio)
  	dev_t dev;
  	struct uio *uio;
  {
  	register struct tty *tp = &pt_tty[minor(dev)];
  	register struct pt_ioctl *pti = &pt_ioctl[minor(dev)];
--- 95,104 ----
  	ptcwakeup(tp, FREAD|FWRITE);
  }
  
! ptsread(dev, uio, fp)
  	dev_t dev;
  	struct uio *uio;
+ 	struct file *fp;
  {
  	register struct tty *tp = &pt_tty[minor(dev)];
  	register struct pt_ioctl *pti = &pt_ioctl[minor(dev)];
***************
*** 130,136 ****
  			return (error);
  	} else
  		if (tp->t_oproc)
! 			error = (*linesw[tp->t_line].l_read)(tp, uio);
  	ptcwakeup(tp, FWRITE);
  	return (error);
  }
--- 131,137 ----
  			return (error);
  	} else
  		if (tp->t_oproc)
! 			error = (*linesw[tp->t_line].l_read)(tp, uio, fp);
  	ptcwakeup(tp, FWRITE);
  	return (error);
  }

-------------
Partial sample diffs for tty.c (tahoe)
*** /tmp/,RCSt1007772	Tue May  7 16:06:21 1991
--- tty.c	Fri Jun 15 14:14:57 1990
***************
*** 1129,1137 ****
   * Called from device's read routine after it has
   * calculated the tty-structure given as argument.
   */
! ttread(tp, uio)
  	register struct tty *tp;
  	struct uio *uio;
  {
  	register struct clist *qp;
  	register c, t_flags;
--- 1129,1138 ----
   * Called from device's read routine after it has
   * calculated the tty-structure given as argument.
   */
! ttread(tp, uio, fp)
  	register struct tty *tp;
  	struct uio *uio;
+ 	struct file *fp;
  {
  	register struct clist *qp;
  	register c, t_flags;
***************
*** 1138,1143 ****
--- 1139,1147 ----
  	int s, first, error = 0;
  
  loop:
+ 	if ((fp != (struct file *)0) && ((fp->f_flag & FREAD) == 0))
+ 		return(EBADF);
+ 
  	/*
  	 * Take any pending input first.
  	 */


-----------------------------------------------------

Now on every ttread the CURRENT file pointer access rights are checked (this
uses the off the shelf vhangup().  The same change appiles ttwrite(), etc.
This catches sleepers as they come back up to the top of the "loop:" on
wakeup. (vhangup() will modify f_flag)

Keith Muller
University of California