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