Revert readv/writev changes, as they trigger an apparent file descriptor

deadlock for ckuethe@

"if you have to revert, you have to revert" deraadt@

2 files changed, 190 insertions, 92 deletions

diff --git a/sys/kern/sys_generic.c b/sys/kern/sys_generic.c
index 9011e3be6e1..dc77c5810d8 100644
--- a/sys/kern/sys_generic.c
+++ b/sys/kern/sys_generic.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sys_generic.c,v 1.63 2009/06/02 05:20:41 thib Exp $	*/
+/*	$OpenBSD: sys_generic.c,v 1.64 2009/06/03 03:57:20 blambert Exp $	*/
 /*	$NetBSD: sys_generic.c,v 1.24 1996/03/29 00:25:32 cgd Exp $	*/
 
 /*
@@ -79,7 +79,6 @@ sys_read(struct proc *p, void *v, register_t *retval)
 		syscallarg(void *) buf;
 		syscallarg(size_t) nbyte;
 	} */ *uap = v;
-	struct iovec iov;
 	int fd = SCARG(uap, fd);
 	struct file *fp;
 	struct filedesc *fdp = p->p_fd;
@@ -89,13 +88,68 @@ sys_read(struct proc *p, void *v, register_t *retval)
 	if ((fp->f_flag & FREAD) == 0)
 		return (EBADF);
 
-	iov.iov_base = (void *)SCARG(uap, buf);
-	iov.iov_len = SCARG(uap, nbyte);
-
 	FREF(fp);
 
-	/* dofilereadv() will FRELE the descriptor for us */
-	return (dofilereadv(p, fd, fp, &iov, 1, &fp->f_offset, retval));
+	/* dofileread() will FRELE the descriptor for us */
+	return (dofileread(p, fd, fp, SCARG(uap, buf), SCARG(uap, nbyte),
+	    &fp->f_offset, retval));
+}
+
+int
+dofileread(struct proc *p, int fd, struct file *fp, void *buf, size_t nbyte,
+    off_t *offset, register_t *retval)
+{
+	struct uio auio;
+	struct iovec aiov;
+	long cnt, error = 0;
+#ifdef KTRACE
+	struct iovec ktriov;
+#endif
+
+	aiov.iov_base = buf;
+	aiov.iov_len = nbyte;
+	auio.uio_iov = &aiov;
+	auio.uio_iovcnt = 1;
+	auio.uio_resid = nbyte;
+	auio.uio_rw = UIO_READ;
+	auio.uio_segflg = UIO_USERSPACE;
+	auio.uio_procp = p;
+
+	/*
+	 * Reads return ssize_t because -1 is returned on error.  Therefore
+	 * we must restrict the length to SSIZE_MAX to avoid garbage return
+	 * values.
+	 */
+	if (auio.uio_resid > SSIZE_MAX) {
+		error = EINVAL;
+		goto out;
+	}
+
+#ifdef KTRACE
+	/*
+	 * if tracing, save a copy of iovec
+	 */
+	if (KTRPOINT(p, KTR_GENIO))
+		ktriov = aiov;
+#endif
+	cnt = auio.uio_resid;
+	error = (*fp->f_ops->fo_read)(fp, offset, &auio, fp->f_cred);
+	if (error)
+		if (auio.uio_resid != cnt && (error == ERESTART ||
+		    error == EINTR || error == EWOULDBLOCK))
+			error = 0;
+	cnt -= auio.uio_resid;
+
+	fp->f_rxfer++;
+	fp->f_rbytes += cnt;
+#ifdef KTRACE
+	if (KTRPOINT(p, KTR_GENIO) && error == 0)
+		ktrgenio(p, fd, UIO_READ, &ktriov, cnt, error);
+#endif
+	*retval = cnt;
+ out:
+	FRELE(fp);
+	return (error);
 }
 
 /*
@@ -104,7 +158,6 @@ sys_read(struct proc *p, void *v, register_t *retval)
 int
 sys_readv(struct proc *p, void *v, register_t *retval)
 {
-	struct iovec aiov[UIO_SMALLIOV];
 	struct sys_readv_args /* {
 		syscallarg(int) fd;
 		syscallarg(const struct iovec *) iovp;
@@ -113,17 +166,33 @@ sys_readv(struct proc *p, void *v, register_t *retval)
 	int fd = SCARG(uap, fd);
 	struct file *fp;
 	struct filedesc *fdp = p->p_fd;
-	struct iovec *iov, *needfree;
-	const struct iovec *iovp = SCARG(uap, iovp);
-	int iovcnt = SCARG(uap, iovcnt);
-	int iovlen;
-	int error;
 
 	if ((fp = fd_getfile(fdp, fd)) == NULL)
 		return (EBADF);
 	if ((fp->f_flag & FREAD) == 0)
 		return (EBADF);
 
+	FREF(fp);
+
+	/* dofilereadv() will FRELE the descriptor for us */
+	return (dofilereadv(p, fd, fp, SCARG(uap, iovp), SCARG(uap, iovcnt),
+	    &fp->f_offset, retval));
+}
+
+int
+dofilereadv(struct proc *p, int fd, struct file *fp, const struct iovec *iovp,
+    int iovcnt, off_t *offset, register_t *retval)
+{
+	struct uio auio;
+	struct iovec *iov;
+	struct iovec *needfree;
+	struct iovec aiov[UIO_SMALLIOV];
+	long i, cnt, error = 0;
+	u_int iovlen;
+#ifdef KTRACE
+	struct iovec *ktriov = NULL;
+#endif
+
 	/* note: can't use iovlen until iovcnt is validated */
 	iovlen = iovcnt * sizeof(struct iovec);
 	if ((u_int)iovcnt > UIO_SMALLIOV) {
@@ -140,37 +209,14 @@ sys_readv(struct proc *p, void *v, register_t *retval)
 		goto out;
 	}
 
-	error = copyin(iovp, iov, iovlen);
-	if (error)
-		goto out;
-
-	FREF(fp);
-
-	/* dofilereadv() will FRELE the descriptor for us */
-	error = dofilereadv(p, fd, fp, iov, iovcnt, &fp->f_offset, retval);
-out:
-	if (needfree)
-		free(needfree, M_IOV);
-
-	return (error);
-}
-
-int
-dofilereadv(struct proc *p, int fd, struct file *fp, const struct iovec *iov,
-    int iovcnt, off_t *offset, register_t *retval)
-{
-	struct uio auio;
-	long i, cnt, error = 0;
-#ifdef KTRACE
-	u_int iovlen = iovcnt * sizeof(struct iovec);
-	struct iovec *ktriov = NULL;
-#endif
-
-	auio.uio_iov = (struct iovec *)iov;
+	auio.uio_iov = iov;
 	auio.uio_iovcnt = iovcnt;
 	auio.uio_rw = UIO_READ;
 	auio.uio_segflg = UIO_USERSPACE;
 	auio.uio_procp = p;
+	error = copyin(iovp, iov, iovlen);
+	if (error)
+		goto done;
 	auio.uio_resid = 0;
 	for (i = 0; i < iovcnt; i++) {
 		auio.uio_resid += iov->iov_len;
@@ -181,7 +227,7 @@ dofilereadv(struct proc *p, int fd, struct file *fp, const struct iovec *iov,
 		 */
 		if (iov->iov_len > SSIZE_MAX || auio.uio_resid > SSIZE_MAX) {
 			error = EINVAL;
-			goto out;
+			goto done;
 		}
 		iov++;
 	}
@@ -213,6 +259,9 @@ dofilereadv(struct proc *p, int fd, struct file *fp, const struct iovec *iov,
 	}
 #endif
 	*retval = cnt;
+ done:
+	if (needfree)
+		free(needfree, M_IOV);
  out:
 	FRELE(fp);
 	return (error);
@@ -229,7 +278,6 @@ sys_write(struct proc *p, void *v, register_t *retval)
 		syscallarg(const void *) buf;
 		syscallarg(size_t) nbyte;
 	} */ *uap = v;
-	struct iovec iov;
 	int fd = SCARG(uap, fd);
 	struct file *fp;
 	struct filedesc *fdp = p->p_fd;
@@ -241,11 +289,69 @@ sys_write(struct proc *p, void *v, register_t *retval)
 
 	FREF(fp);
 
-	iov.iov_base = (void *)SCARG(uap, buf);
-	iov.iov_len = SCARG(uap, nbyte);
+	/* dofilewrite() will FRELE the descriptor for us */
+	return (dofilewrite(p, fd, fp, SCARG(uap, buf), SCARG(uap, nbyte),
+	    &fp->f_offset, retval));
+}
 
-	/* dofilewritev() will FRELE the descriptor for us */
-	return (dofilewritev(p, fd, fp, &iov, 1, &fp->f_offset, retval));
+int
+dofilewrite(struct proc *p, int fd, struct file *fp, const void *buf,
+    size_t nbyte, off_t *offset, register_t *retval)
+{
+	struct uio auio;
+	struct iovec aiov;
+	long cnt, error = 0;
+#ifdef KTRACE
+	struct iovec ktriov;
+#endif
+
+	aiov.iov_base = (void *)buf;		/* XXX kills const */
+	aiov.iov_len = nbyte;
+	auio.uio_iov = &aiov;
+	auio.uio_iovcnt = 1;
+	auio.uio_resid = nbyte;
+	auio.uio_rw = UIO_WRITE;
+	auio.uio_segflg = UIO_USERSPACE;
+	auio.uio_procp = p;
+
+	/*
+	 * Writes return ssize_t because -1 is returned on error.  Therefore
+	 * we must restrict the length to SSIZE_MAX to avoid garbage return
+	 * values.
+	 */
+	if (auio.uio_resid > SSIZE_MAX) {
+		error = EINVAL;
+		goto out;
+	}
+
+#ifdef KTRACE
+	/*
+	 * if tracing, save a copy of iovec
+	 */
+	if (KTRPOINT(p, KTR_GENIO))
+		ktriov = aiov;
+#endif
+	cnt = auio.uio_resid;
+	error = (*fp->f_ops->fo_write)(fp, offset, &auio, fp->f_cred);
+	if (error) {
+		if (auio.uio_resid != cnt && (error == ERESTART ||
+		    error == EINTR || error == EWOULDBLOCK))
+			error = 0;
+		if (error == EPIPE)
+			ptsignal(p, SIGPIPE, STHREAD);
+	}
+	cnt -= auio.uio_resid;
+
+	fp->f_wxfer++;
+	fp->f_wbytes += cnt;
+#ifdef KTRACE
+	if (KTRPOINT(p, KTR_GENIO) && error == 0)
+		ktrgenio(p, fd, UIO_WRITE, &ktriov, cnt, error);
+#endif
+	*retval = cnt;
+ out:
+	FRELE(fp);
+	return (error);
 }
 
 /*
@@ -254,21 +360,14 @@ sys_write(struct proc *p, void *v, register_t *retval)
 int
 sys_writev(struct proc *p, void *v, register_t *retval)
 {
-	struct iovec aiov[UIO_SMALLIOV];
 	struct sys_writev_args /* {
 		syscallarg(int) fd;
 		syscallarg(const struct iovec *) iovp;
 		syscallarg(int) iovcnt;
 	} */ *uap = v;
-	struct filedesc *fdp = p->p_fd;
-	struct file *fp;
-	struct iovec *needfree = NULL;
-	struct iovec *iov;
-	const struct iovec *iovp = SCARG(uap, iovp);
 	int fd = SCARG(uap, fd);
-	int iovcnt = SCARG(uap, iovcnt);
-	u_int iovlen;
-	int error;
+	struct file *fp;
+	struct filedesc *fdp = p->p_fd;
 
 	if ((fp = fd_getfile(fdp, fd)) == NULL)
 		return (EBADF);
@@ -277,6 +376,25 @@ sys_writev(struct proc *p, void *v, register_t *retval)
 
 	FREF(fp);
 
+	/* dofilewritev() will FRELE the descriptor for us */
+	return (dofilewritev(p, fd, fp, SCARG(uap, iovp), SCARG(uap, iovcnt),
+	    &fp->f_offset, retval));
+}
+
+int
+dofilewritev(struct proc *p, int fd, struct file *fp, const struct iovec *iovp,
+    int iovcnt, off_t *offset, register_t *retval)
+{
+	struct uio auio;
+	struct iovec *iov;
+	struct iovec *needfree;
+	struct iovec aiov[UIO_SMALLIOV];
+	long i, cnt, error = 0;
+	u_int iovlen;
+#ifdef KTRACE
+	struct iovec *ktriov = NULL;
+#endif
+
 	/* note: can't use iovlen until iovcnt is validated */
 	iovlen = iovcnt * sizeof(struct iovec);
 	if ((u_int)iovcnt > UIO_SMALLIOV) {
@@ -292,35 +410,15 @@ sys_writev(struct proc *p, void *v, register_t *retval)
 		error = EINVAL;
 		goto out;
 	}
-	error = copyin(iovp, iov, iovlen);
-	if (error)
-		goto out;
 
-	/* dofilewritev() will FRELE the descriptor for us */
-	error = dofilewritev(p, fd, fp, iov, iovcnt, &fp->f_offset, retval);
-out:
-	if (needfree)
-		free(needfree, M_IOV);
-
-	return (error);
-}
-
-int
-dofilewritev(struct proc *p, int fd, struct file *fp, const struct iovec *iov,
-    int iovcnt, off_t *offset, register_t *retval)
-{
-	struct uio auio;
-	long i, cnt, error = 0;
-#ifdef KTRACE
-	u_int iovlen;
-	struct iovec *ktriov = NULL;
-#endif
-
-	auio.uio_iov = (struct iovec *)iov;
+	auio.uio_iov = iov;
 	auio.uio_iovcnt = iovcnt;
 	auio.uio_rw = UIO_WRITE;
 	auio.uio_segflg = UIO_USERSPACE;
 	auio.uio_procp = p;
+	error = copyin(iovp, iov, iovlen);
+	if (error)
+		goto done;
 	auio.uio_resid = 0;
 	for (i = 0; i < iovcnt; i++) {
 		auio.uio_resid += iov->iov_len;
@@ -331,7 +429,7 @@ dofilewritev(struct proc *p, int fd, struct file *fp, const struct iovec *iov,
 		 */
 		if (iov->iov_len > SSIZE_MAX || auio.uio_resid > SSIZE_MAX) {
 			error = EINVAL;
-			goto out;
+			goto done;
 		}
 		iov++;
 	}
@@ -365,6 +463,9 @@ dofilewritev(struct proc *p, int fd, struct file *fp, const struct iovec *iov,
 	}
 #endif
 	*retval = cnt;
+ done:
+	if (needfree)
+		free(needfree, M_IOV);
  out:
 	FRELE(fp);
 	return (error);
@@ -478,7 +579,8 @@ sys_ioctl(struct proc *p, void *v, register_t *retval)
 			}
 			tmp = p1->p_pgrp->pg_id;
 		}
-		error = (*fp->f_ops->fo_ioctl)(fp, TIOCSPGRP, (caddr_t)&tmp, p);
+		error = (*fp->f_ops->fo_ioctl)
+			(fp, TIOCSPGRP, (caddr_t)&tmp, p);
 		break;
 
 	case FIOGETOWN:
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index a69d7f9c148..19ff031b110 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfs_syscalls.c,v 1.153 2009/06/02 03:04:09 blambert Exp $	*/
+/*	$OpenBSD: vfs_syscalls.c,v 1.154 2009/06/03 03:57:20 blambert Exp $	*/
 /*	$NetBSD: vfs_syscalls.c,v 1.71 1996/04/23 10:29:02 mycroft Exp $	*/
 
 /*
@@ -2439,7 +2439,6 @@ sys_pread(struct proc *p, void *v, register_t *retval)
 		syscallarg(int) pad;
 		syscallarg(off_t) offset;
 	} */ *uap = v;
-	struct iovec iov;
 	struct filedesc *fdp = p->p_fd;
 	struct file *fp;
 	struct vnode *vp;
@@ -2456,14 +2455,13 @@ sys_pread(struct proc *p, void *v, register_t *retval)
 		return (ESPIPE);
 	}
 
-	iov.iov_base = (void *)SCARG(uap, buf);
-	iov.iov_len = SCARG(uap, nbyte);
 	offset = SCARG(uap, offset);
 
 	FREF(fp);
 
-	/* dofilereadv() will FRELE the descriptor for us */
-	return (dofilereadv(p, fd, fp, &iov, 1, &offset, retval));
+	/* dofileread() will FRELE the descriptor for us */
+	return (dofileread(p, fd, fp, SCARG(uap, buf), SCARG(uap, nbyte),
+	    &offset, retval));
 }
 
 /*
@@ -2517,7 +2515,6 @@ sys_pwrite(struct proc *p, void *v, register_t *retval)
 		syscallarg(int) pad;
 		syscallarg(off_t) offset;
 	} */ *uap = v;
-	struct iovec iov;
 	struct filedesc *fdp = p->p_fd;
 	struct file *fp;
 	struct vnode *vp;
@@ -2536,12 +2533,11 @@ sys_pwrite(struct proc *p, void *v, register_t *retval)
 
 	FREF(fp);
 
-	iov.iov_base = (void *)SCARG(uap, buf);
-	iov.iov_len = SCARG(uap, nbyte);
 	offset = SCARG(uap, offset);
 
-	/* dofilewritev() will FRELE the descriptor for us */
-	return (dofilewritev(p, fd, fp, &iov, 1, &offset, retval));
+	/* dofilewrite() will FRELE the descriptor for us */
+	return (dofilewrite(p, fd, fp, SCARG(uap, buf), SCARG(uap, nbyte),
+	    &offset, retval));
 }
 
 /*