Subject: Re: accept(2) behaviour.
To: None <tech-net@netbsd.org>
From: Darren Reed <darrenr@reed.wattle.id.au>
List: tech-net
Date: 06/27/1999 23:40:28
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.

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);