sendfile: stop abusing kern_writev()

Provide convenient wrapper kern_filewrite() around fo_write().
Switch to use it in vn_sendfile().  This allows to avoid duplicate
fget() when we already have the reference to the file, which creates a
correctness race with the userspace.  Also td_retval[0] clearing hack
can be removed.

Reviewed by:	glebius, markj
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D58035

3 files changed, 45 insertions, 23 deletions

diff --git a/sys/kern/kern_sendfile.c b/sys/kern/kern_sendfile.c
index cf9716560c07..1a534210ca0c 100644
--- a/sys/kern/kern_sendfile.c
+++ b/sys/kern/kern_sendfile.c
@@ -1168,14 +1168,17 @@ prepend_header:
 	}
 
 	/*
-	 * Send trailers. Wimp out and use writev(2).
+	 * Send trailers.
 	 */
 	if (trl_uio != NULL) {
+		ssize_t cnt;
+
 		SOCK_IO_SEND_UNLOCK(so);
 		CURVNET_RESTORE();
-		error = kern_writev(td, sockfd, trl_uio);
+		error = kern_filewrite(td, sockfd, sock_fp, trl_uio,
+		    (off_t)-1, 0, &cnt);
 		if (error == 0)
-			sbytes += td->td_retval[0];
+			sbytes += cnt;
 		goto out;
 	}
 
@@ -1183,15 +1186,8 @@ done:
 	SOCK_IO_SEND_UNLOCK(so);
 	CURVNET_RESTORE();
 out:
-	/*
-	 * If there was no error we have to clear td->td_retval[0]
-	 * because it may have been set by writev.
-	 */
-	if (error == 0) {
-		td->td_retval[0] = 0;
-		if (sbytes > 0 && vp != NULL)
-			INOTIFY(vp, IN_ACCESS);
-	}
+	if (error == 0 && sbytes > 0 && vp != NULL)
+		INOTIFY(vp, IN_ACCESS);
 	if (sent != NULL) {
 		(*sent) = sbytes;
 	}
@@ -1260,6 +1256,11 @@ sendfile(struct thread *td, struct sendfile_args *uap, int compat)
 			    &trl_uio);
 			if (error != 0)
 				goto out;
+			if (trl_uio->uio_rw != UIO_WRITE) {
+				error = EINVAL;
+				goto out;
+			}
+			trl_uio->uio_td = td;
 		}
 	}
 
diff --git a/sys/kern/sys_generic.c b/sys/kern/sys_generic.c
index b84f675d1dcb..07b98613e087 100644
--- a/sys/kern/sys_generic.c
+++ b/sys/kern/sys_generic.c
@@ -549,21 +549,16 @@ dofilewrite(struct thread *td, int fd, struct file *fp, struct uio *auio,
 {
 	ssize_t cnt;
 	int error;
-#ifdef KTRACE
-	struct uio *ktruio = NULL;
-#endif
 
 	AUDIT_ARG_FD(fd);
+
 	auio->uio_rw = UIO_WRITE;
 	auio->uio_td = td;
 	auio->uio_offset = offset;
-#ifdef KTRACE
-	if (KTRPOINT(td, KTR_GENIO))
-		ktruio = cloneuio(auio);
-#endif
-	cnt = auio->uio_resid;
-	error = fo_write(fp, auio, td->td_ucred, flags, td);
+	error = kern_filewrite(td, fd, fp, auio, offset, flags, &cnt);
+
 	/*
+	 * Handle short writes and generate SIGPIPE if needed.
 	 * Socket layer is responsible for special error handling,
 	 * see sousrsend().
 	 */
@@ -577,15 +572,39 @@ dofilewrite(struct thread *td, int fd, struct file *fp, struct uio *auio,
 			PROC_UNLOCK(td->td_proc);
 		}
 	}
-	cnt -= auio->uio_resid;
+
+	if (error == 0)
+		td->td_retval[0] = cnt;
+	return (error);
+}
+
+/*
+ * Write io request specified by auio into the file fp.  If fd != -1,
+ * might generate the ktrace io point.
+ */
+int
+kern_filewrite(struct thread *td, int fd, struct file *fp, struct uio *auio,
+    off_t offset, int flags, ssize_t *cntp)
+{
+	ssize_t cnt;
+	int error;
+#ifdef KTRACE
+	struct uio *ktruio;
+
+	ktruio = fd != -1 && KTRPOINT(td, KTR_GENIO) ? cloneuio(auio) : NULL;
+#endif
+	cnt = auio->uio_resid;
+	error = fo_write(fp, auio, td->td_ucred, flags, td);
 #ifdef KTRACE
 	if (ktruio != NULL) {
+		cnt -= auio->uio_resid;
 		if (error == 0)
 			ktruio->uio_resid = cnt;
 		ktrgenio(fd, UIO_WRITE, ktruio, error);
 	}
 #endif
-	td->td_retval[0] = cnt;
+
+	*cntp = cnt;
 	return (error);
 }
 
diff --git a/sys/sys/syscallsubr.h b/sys/sys/syscallsubr.h
index c6bfe89dcf35..131a8925a2d4 100644
--- a/sys/sys/syscallsubr.h
+++ b/sys/sys/syscallsubr.h
@@ -176,6 +176,8 @@ int	kern_fcntl_freebsd(struct thread *td, int fd, int cmd, intptr_t arg);
 int	kern_fhopen(struct thread *td, const struct fhandle *u_fhp, int flags);
 int	kern_fhstat(struct thread *td, fhandle_t fh, struct stat *buf);
 int	kern_fhstatfs(struct thread *td, fhandle_t fh, struct statfs *buf);
+int	kern_filewrite(struct thread *td, int fd, struct file *fp,
+	    struct uio *auio, off_t offset, int flags, ssize_t *cntp);
 int	kern_fpathconf(struct thread *td, int fd, int name, long *valuep);
 int	kern_freebsd11_getfsstat(struct thread *td,
 	    struct freebsd11_statfs *ubuf, long bufsize, int mode);