On 21/04/15(Tue) 17:15, Vitaliy Makkoveev wrote:
> Now fd_getfile() function returns unacquired "struct file" instance
> [...]
> It is unacceptable on multiprocessor machine because the instance referenced
> by fp can be destroyed between fd_getfile() and FREF() calls. So I want
> fd_getfile() returns acquired fp. [...]
Your diff makes sense but sadly it is broken, could you send a diff that
can be applied?
> Index: share/man/man9/file.9
> ===================================================================
> RCS file: /cvs/src/share/man/man9/file.9,v
> retrieving revision 1.12
> diff -u -p -r1.12 file.9
> --- share/man/man9/file.9 4 Jun 2013 19:27:06 -0000 1.12
> +++ share/man/man9/file.9 21 Apr 2015 13:27:08 -0000
> @@ -42,7 +42,7 @@
> .Ft struct file *
> .Fn fd_getfile "struct filedesc *fdp" "int fd"
> .Ft int
> -.Fn getsock "struct filedesc *fdp" "int fd" "struct file **fpp"
> +.Fn getsock "struct proc *p" "int fd" "struct file **fpp"
> .In sys/file.h
> .In sys/filedesc.h
> .In sys/vnode.h
> @@ -74,21 +74,28 @@ recommended to make complicated kernel A
> .Pp
> The files are extracted from the file descriptor table using the
> functions
> -.Fn fd_getfile ,
> -.Fn getvnode
> +.Fn fd_getfile
> and
> -.Fn getsock .
> +.Fn getvnode .
> .Fn fd_getfile
> performs all necessary checks to see if the file descriptor number is
> within the range of file descriptor table, and if the descriptor is
> valid.
> -.Fn getsock
> -and
> .Fn getvnode
> -are special cases that besides doing
> +is special case that besides doing
> +.Fn fd_getfile
> +also checks if the descriptor is a vnode, returns the proper
> +errno on error and increases the use count with
> +.Fn FREF .
> +.Pp
> +The files are extracted from the process context using the
> +function
> +.Fn getsock .
> +.Fn getsock
> +is special case that besides doing
> .Fn fd_getfile
> -also check if the descriptor is a vnode or socket, return the proper
> -errno on error and increase the use count with
> +also checks if the descriptor is a socket, returns the proper
> +errno on error and increases the use count with
> .Fn FREF .
> .Sh CONCURRENT ACCESS
> Since multiple processes can share the same file descriptor table,
> Index: sys/compat/linux/linux_socket.c
> ===================================================================
> RCS file: /cvs/src/sys/compat/linux/linux_socket.c,v
> retrieving revision 1.60
> diff -u -p -r1.60 linux_socket.c
> --- sys/compat/linux/linux_socket.c 30 Jan 2015 23:38:49 -0000 1.60
> +++ sys/compat/linux/linux_socket.c 21 Apr 2015 13:28:23 -0000
> @@ -937,7 +937,7 @@ linux_setsockopt(p, v, retval)
> if ((error = copyin((caddr_t) uap, (caddr_t) &lsa, sizeof lsa)))
> return error;
>
> - if ((error = getsock(p->p_fd, lsa.s, &fp)) != 0)
> + if ((error = getsock(p, lsa.s, &fp)) != 0)
> return error;
>
> level = linux_to_bsd_sopt_level(lsa.level);
> Index: sys/kern/subr_log.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/subr_log.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 subr_log.c
> --- sys/kern/subr_log.c 14 Mar 2015 03:38:50 -0000 1.29
> +++ sys/kern/subr_log.c 21 Apr 2015 13:28:42 -0000
> @@ -334,7 +334,7 @@ logioctl(dev_t dev, u_long com, caddr_t
> case LIOCSFD:
> if ((error = suser(p, 0)) != 0)
> return (error);
> - if ((error = getsock(p->p_fd, *(int *)data, &fp)) != 0)
> + if ((error = getsock(p, *(int *)data, &fp)) != 0)
> return (error);
> if (syslogf)
> FRELE(syslogf, p);
> Index: sys/kern/uipc_socket.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.137
> diff -u -p -r1.137 uipc_socket.c
> --- sys/kern/uipc_socket.c 14 Mar 2015 03:38:51 -0000 1.137
> +++ sys/kern/uipc_socket.c 21 Apr 2015 13:28:42 -0000
> @@ -1076,7 +1076,7 @@ sosplice(struct socket *so, int fd, off_
> return (EINVAL);
>
> /* Find sosp, the drain socket where data will be spliced into. */
> - if ((error = getsock(curproc->p_fd, fd, &fp)) != 0)
> + if ((error = getsock(curproc, fd, &fp)) != 0)
> return (error);
> sosp = fp->f_data;
> if (sosp->so_sp == NULL)
> Index: sys/kern/uipc_syscalls.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
> retrieving revision 1.100
> diff -u -p -r1.100 uipc_syscalls.c
> --- sys/kern/uipc_syscalls.c 14 Mar 2015 03:38:51 -0000 1.100
> +++ sys/kern/uipc_syscalls.c 21 Apr 2015 13:28:42 -0000
> @@ -120,7 +120,7 @@ sys_bind(struct proc *p, void *v, regist
> struct mbuf *nam;
> int error;
>
> - if ((error = getsock(p->p_fd, SCARG(uap, s), &fp)) != 0)
> + if ((error = getsock(p, SCARG(uap, s), &fp)) != 0)
> return (error);
> error = sockargs(&nam, SCARG(uap, name), SCARG(uap, namelen),
> MT_SONAME);
> @@ -147,7 +147,7 @@ sys_listen(struct proc *p, void *v, regi
> struct file *fp;
> int error;
>
> - if ((error = getsock(p->p_fd, SCARG(uap, s), &fp)) != 0)
> + if ((error = getsock(p, SCARG(uap, s), &fp)) != 0)
> return (error);
> error = solisten(fp->f_data, SCARG(uap, backlog));
> FRELE(fp, p);
> @@ -198,7 +198,7 @@ doaccept(struct proc *p, int sock, struc
>
> if (name && (error = copyin(anamelen, &namelen, sizeof (namelen))))
> return (error);
> - if ((error = getsock(fdp, sock, &fp)) != 0)
> + if ((error = getsock(p, sock, &fp)) != 0)
> return (error);
> headfp = fp;
> s = splsoftnet();
> @@ -315,7 +315,7 @@ sys_connect(struct proc *p, void *v, reg
> struct mbuf *nam = NULL;
> int error, s;
>
> - if ((error = getsock(p->p_fd, SCARG(uap, s), &fp)) != 0)
> + if ((error = getsock(p, SCARG(uap, s), &fp)) != 0)
> return (error);
> so = fp->f_data;
> if ((so->so_state & SS_NBIO) && (so->so_state & SS_ISCONNECTING)) {
> @@ -518,7 +518,7 @@ sendit(struct proc *p, int s, struct msg
>
> to = NULL;
>
> - if ((error = getsock(p->p_fd, s, &fp)) != 0)
> + if ((error = getsock(p, s, &fp)) != 0)
> return (error);
> auio.uio_iov = mp->msg_iov;
> auio.uio_iovcnt = mp->msg_iovlen;
> @@ -684,7 +684,7 @@ recvit(struct proc *p, int s, struct msg
> int iovlen = 0;
> #endif
>
> - if ((error = getsock(p->p_fd, s, &fp)) != 0)
> + if ((error = getsock(p, s, &fp)) != 0)
> return (error);
> auio.uio_iov = mp->msg_iov;
> auio.uio_iovcnt = mp->msg_iovlen;
> @@ -803,7 +803,7 @@ sys_shutdown(struct proc *p, void *v, re
> struct file *fp;
> int error;
>
> - if ((error = getsock(p->p_fd, SCARG(uap, s), &fp)) != 0)
> + if ((error = getsock(p, SCARG(uap, s), &fp)) != 0)
> return (error);
> error = soshutdown(fp->f_data, SCARG(uap, how));
> FRELE(fp, p);
> @@ -825,7 +825,7 @@ sys_setsockopt(struct proc *p, void *v,
> struct mbuf *m = NULL;
> int error;
>
> - if ((error = getsock(p->p_fd, SCARG(uap, s), &fp)) != 0)
> + if ((error = getsock(p, SCARG(uap, s), &fp)) != 0)
> return (error);
> if (SCARG(uap, valsize) > MCLBYTES) {
> error = EINVAL;
> @@ -876,7 +876,7 @@ sys_getsockopt(struct proc *p, void *v,
> socklen_t valsize;
> int error;
>
> - if ((error = getsock(p->p_fd, SCARG(uap, s), &fp)) != 0)
> + if ((error = getsock(p, SCARG(uap, s), &fp)) != 0)
> return (error);
> if (SCARG(uap, val)) {
> error = copyin(SCARG(uap, avalsize),
> @@ -920,7 +920,7 @@ sys_getsockname(struct proc *p, void *v,
> socklen_t len;
> int error;
>
> - if ((error = getsock(p->p_fd, SCARG(uap, fdes), &fp)) != 0)
> + if ((error = getsock(p, SCARG(uap, fdes), &fp)) != 0)
> return (error);
> error = copyin(SCARG(uap, alen), &len, sizeof (len));
> if (error)
> @@ -956,7 +956,7 @@ sys_getpeername(struct proc *p, void *v,
> socklen_t len;
> int error;
>
> - if ((error = getsock(p->p_fd, SCARG(uap, fdes), &fp)) != 0)
> + if ((error = getsock(p, SCARG(uap, fdes), &fp)) != 0)
> return (error);
> so = fp->f_data;
> if ((so->so_state & SS_ISCONNECTED) == 0) {
> @@ -1016,11 +1016,11 @@ sockargs(struct mbuf **mp, const void *b
> }
>
> int
> -getsock(struct filedesc *fdp, int fdes, struct file **fpp)
> +getsock(struct proc *p, int fdes, struct file **fpp)
> {
> struct file *fp;
>
> - if ((fp = fd_getfile(fdp, fdes)) == NULL)
> + if ((fp = fd_getfile(p->p_fd, fdes)) == NULL)
> return (EBADF);
> if (fp->f_type != DTYPE_SOCKET)
> return (ENOTSOCK);
> Index: sys/nfs/nfs_syscalls.c
> ===================================================================
> RCS file: /cvs/src/sys/nfs/nfs_syscalls.c,v
> retrieving revision 1.99
> diff -u -p -r1.99 nfs_syscalls.c
> --- sys/nfs/nfs_syscalls.c 14 Mar 2015 03:38:52 -0000 1.99
> +++ sys/nfs/nfs_syscalls.c 21 Apr 2015 13:28:51 -0000
> @@ -166,7 +166,7 @@ sys_nfssvc(struct proc *p, void *v, regi
> if (error)
> return (error);
>
> - error = getsock(p->p_fd, nfsdarg.sock, &fp);
> + error = getsock(p, nfsdarg.sock, &fp);
> if (error)
> return (error);
>
> Index: sys/sys/filedesc.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/filedesc.h,v
> retrieving revision 1.28
> diff -u -p -r1.28 filedesc.h
> --- sys/sys/filedesc.h 15 May 2014 03:52:25 -0000 1.28
> +++ sys/sys/filedesc.h 21 Apr 2015 13:28:53 -0000
> @@ -135,7 +135,7 @@ void fdcloseexec(struct proc *);
> struct file *fd_getfile(struct filedesc *, int fd);
>
> int closef(struct file *, struct proc *);
> -int getsock(struct filedesc *, int, struct file **);
> +int getsock(struct proc *, int, struct file **);
>
> #define fdplock(fdp) rw_enter_write(&(fdp)->fd_lock)
> #define fdpunlock(fdp) rw_exit_write(&(fdp)->fd_lock)
>
