Subject: Re: accept(2) behaviour.
To: Darren Reed <darrenr@reed.wattle.id.au>
From: Matt Thomas <matt@3am-software.com>
List: tech-net
Date: 06/27/1999 10:13:39
At 06:40 AM 6/27/99 , Darren Reed wrote:
>
>Further to this, one patch to solve the previous problem is to detect
>the invalid pointer early and bail out of sys_accept() then.  It doesn't,
>however, solve the real problem which could still be triggered by bad
>programming - that is making the address space into which the address
>is being written after the system call has been started (next to
>impossible to trigger, I'd say, without threads).  I'm not sure what
>the right thing to do here is, with two options: (1) ignore the error
>returned by copyout at the end of sys_accept(), pretend there was no
>error and return the fd or (2) close the fd if there's an error.  My
>instincts say (2) - see patch below.
>
>Another interesting `feature' of accept(2) is that in doing a sanity
>check to ensure that it has been passed an fd which belongs to a
>connection marked for listen'ing, it makes it impossible for it to
>return EOPNOTSUPP.  In order to correct this, I'd like to add a check
>for so_type == SOCK_STREAM which would return EOPNOTSUPP as clearly the
>check described on the man page for accept(2) is not being made.

SOCK_SEQPACKET is also able to do accepts.  Anyways, it should be
up to the xxx_usrreq to determine whether a PRU_LISTEN is allowed
or not.  If you want to catch this before the usrreq routine, the
correct way to solve this is to add a protosw flag which indicates
whether listen in allowed.

>Comments ?
>
>Cheers,
>Darren
>
>
>*** /sys/kern/uipc_syscalls.c.dist	Sat Jun 26 16:01:29 1999
>--- /sys/kern/uipc_syscalls.c	Sun Jun 27 23:36:20 1999
>***************
>*** 67,72 ****
>--- 67,75 ----
>  #include <sys/mount.h>
>  #include <sys/syscallargs.h>
>  
>+ #include <vm/vm.h>
>+ #include <uvm/uvm_extern.h>
>+ 
>  /*
>   * System call interface to the socket abstraction.
>   */
>***************
>*** 171,181 ****
>--- 174,192 ----
>  	if (SCARG(uap, name) && (error = copyin((caddr_t)SCARG(uap, anamelen),
>  	    (caddr_t)&namelen, sizeof(namelen))))
>  		return (error);
>+ 	if (SCARG(uap, name) != NULL &&
>+ 	    uvm_useracc((caddr_t)SCARG(uap, name), sizeof(struct sockaddr),
>+ 	     B_WRITE) == FALSE)
>+ 		return (EFAULT);
>  
>  	if ((error = getsock(p->p_fd, SCARG(uap, s), &fp)) != 0)
>  		return (error);
>  	s = splsoftnet();
>  	so = (struct socket *)fp->f_data;
>+ 	if (so->so_type != SOCK_STREAM) {
>+ 		splx(s);
>+ 		return (EOPNOTSUPP);
>+ 	}
>  	if ((so->so_options & SO_ACCEPTCONN) == 0) {
>  		splx(s);
>  		return (EINVAL);
>***************
>*** 227,232 ****
>--- 238,245 ----
>  			error = copyout((caddr_t)&namelen,
>  			    (caddr_t)SCARG(uap, anamelen),
>  			    sizeof(*SCARG(uap, anamelen)));
>+ 		if (error != 0)
>+ 			(void0 closef(fp, p);
>  	}
>  	m_freem(nam);
>  	splx(s);

-- 
Matt Thomas               Internet:   matt@3am-software.com
3am Software Foundry      WWW URL:    http://www.3am-software.com/bio/matt/
Sunnyvale, CA             Disclaimer: I avow all knowledge of this message