> On 2015/10/07 12:00, Theo de Raadt wrote:
> > CVSROOT:    /cvs
> > Module name:        src
> > Changes by: [email protected] 2015/10/07 12:00:06
> > 
> > Modified files:
> >     usr.bin/tty    : tty.c 
> > 
> > Log message:
> > use tame "stdio rpath tty", for ttyname().  from Rob Pierce, who chose to
> > do this using ktrace step by step.  not the method i recommend, because
> > it requires 100% coverage via feature tests.  better to read the code and
> > understand everything being called, then make decisions.
> > 
> 
> I ran into a problem building ports/korean/hanterm-xf in DPB with this.
> 
> checking for tty group name... Killed 
> wheel
> checking if we may use wheel group... Killed 
> no
> 
> Followed later by
> 
> ./main.c: In function 'get_pty':
> ./main.c:2352: warning: assignment makes pointer from integer without a cast
> ./main.c:2353: error: dereferencing pointer to incomplete type
> 
> because USE_TTY_GROUP wasn't defined, which is a prerequisite to pull in 
> grp.h:
> 
> #ifdef USE_TTY_GROUP
> #include <grp.h>
> #endif
> 
> So during the autoconf checks, we get this:
> 
> Oct  8 17:44:01 i386 /bsd: tty(18956): syscall 54
> Oct  8 17:44:01 i386 /bsd: tty(18956): syscall 54
> Oct  8 17:44:01 i386 /bsd: tty(968): syscall 54
> Oct  8 17:44:01 i386 /bsd: tty(968): syscall 54
> 
> This comes from tty(1) doing this
> 
>         t = ttyname(STDIN_FILENO);
> 
> which calls tcgetattr, which calls an ioctl (54=ioctl)
> 
>         return (ioctl(fd, TIOCGETA, t));
> 
> But it only happens when run through ssh.
> 
> $ tty
> /dev/ttyp2
> $ ssh i386 tty
> ^[[A Killed 
> 
> So in order to tame tty(1), the ioctl check needs to be more lax than
> the current one,
> 
>                         if (fp->f_type == DTYPE_VNODE && (vp->v_flag & 
> VISTTY))
>                                 return (0);
> 

TIOCGETA is being run on a non-tty?  Hmm... yes, in that case it should
fail (meaning return with errno) like the lower layer would, rather than
kill the process.

Maybe something like the following.  Note that this diff is AFTER the
tame->pledge rename, however it should apply easily once the filename
is handled.

Index: kern_pledge.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.1
diff -u -p -u -r1.1 kern_pledge.c
--- kern_pledge.c       9 Oct 2015 01:17:21 -0000       1.1
+++ kern_pledge.c       9 Oct 2015 01:21:23 -0000
@@ -1016,6 +1016,9 @@ pledge_ioctl_check(struct proc *p, long 
                case FIOGETOWN:
                        return (0);
                case TIOCGETA:
+                       if (fp->f_type == DTYPE_VNODE && (vp->v_flag & VISTTY))
+                               return (0);
+                       return (ENOTTY);
                case TIOCGPGRP:
                case TIOCGWINSZ:        /* various programs */
                        if (fp->f_type == DTYPE_VNODE && (vp->v_flag & VISTTY))
@@ -1059,6 +1062,9 @@ pledge_ioctl_check(struct proc *p, long 
                                break;
                        /* FALTHROUGH */
                case TIOCGETA:
+                       if (fp->f_type == DTYPE_VNODE && (vp->v_flag & VISTTY))
+                               return (0);
+                       return (ENOTTY);
                case TIOCGPGRP:
                case TIOCGWINSZ:        /* various programs */
 #if notyet

Reply via email to