Date: Tue, 29 Mar 2016 19:57:11 +0000 (UTC) From: Gleb Smirnoff <glebius@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r297400 - in head/sys: compat/freebsd32 kern sys Message-ID: <201603291957.u2TJvBsf036848@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: glebius Date: Tue Mar 29 19:57:11 2016 New Revision: 297400 URL: https://svnweb.freebsd.org/changeset/base/297400 Log: The sendfile(2) allows to send extra data from userspace before the file data (headers). Historically the size of the headers was not checked against the socket buffer space. Application could easily overcommit the socket buffer space. With the new sendfile (r293439) the problem remained, but a KASSERT was inserted that checked that amount of data written to the socket matches its space. In case when size of headers is bigger that socket space, KASSERT fires. Without INVARIANTS the new sendfile won't panic, but would report incorrect amount of bytes sent. o With this change, the headers copyin is moved down into the cycle, after the sbspace() check. The uio size is trimmed by socket space there, which fixes the overcommit problem and its consequences. o The compatibility handling for FreeBSD 4 sendfile headers API is pushed up the stack to syscall wrappers. This required a copy and paste of the code, but in turn this allowed to remove extra stack carried parameter from fo_sendfile_t, and embrace entire compat code into #ifdef. If in future we got more fo_sendfile_t function, the copy and paste level would even reduce. Reviewed by: emax, gallatin, Maxim Dounin <mdounin mdounin.ru> Tested by: Vitalij Satanivskij <satan ukr.net> Sponsored by: Netflix Modified: head/sys/compat/freebsd32/freebsd32_misc.c head/sys/kern/kern_descrip.c head/sys/kern/kern_sendfile.c head/sys/sys/file.h head/sys/sys/socket.h Modified: head/sys/compat/freebsd32/freebsd32_misc.c ============================================================================== --- head/sys/compat/freebsd32/freebsd32_misc.c Tue Mar 29 19:56:48 2016 (r297399) +++ head/sys/compat/freebsd32/freebsd32_misc.c Tue Mar 29 19:57:11 2016 (r297400) @@ -1653,6 +1653,19 @@ freebsd32_do_sendfile(struct thread *td, hdtr32.hdr_cnt, &hdr_uio); if (error) goto out; +#ifdef COMPAT_FREEBSD4 + /* + * In FreeBSD < 5.0 the nbytes to send also included + * the header. If compat is specified subtract the + * header size from nbytes. + */ + if (compat) { + if (uap->nbytes > hdr_uio->uio_resid) + uap->nbytes -= hdr_uio->uio_resid; + else + uap->nbytes = 0; + } +#endif } if (hdtr.trailers != NULL) { iov32 = PTRIN(hdtr32.trailers); @@ -1670,7 +1683,7 @@ freebsd32_do_sendfile(struct thread *td, goto out; error = fo_sendfile(fp, uap->s, hdr_uio, trl_uio, offset, - uap->nbytes, &sbytes, uap->flags, compat ? SFK_COMPAT : 0, td); + uap->nbytes, &sbytes, uap->flags, td); fdrop(fp, td); if (uap->sbytes != NULL) Modified: head/sys/kern/kern_descrip.c ============================================================================== --- head/sys/kern/kern_descrip.c Tue Mar 29 19:56:48 2016 (r297399) +++ head/sys/kern/kern_descrip.c Tue Mar 29 19:57:11 2016 (r297400) @@ -3958,7 +3958,7 @@ badfo_chown(struct file *fp, uid_t uid, static int badfo_sendfile(struct file *fp, int sockfd, struct uio *hdr_uio, struct uio *trl_uio, off_t offset, size_t nbytes, off_t *sent, int flags, - int kflags, struct thread *td) + struct thread *td) { return (EBADF); @@ -4044,7 +4044,7 @@ invfo_chown(struct file *fp, uid_t uid, int invfo_sendfile(struct file *fp, int sockfd, struct uio *hdr_uio, struct uio *trl_uio, off_t offset, size_t nbytes, off_t *sent, int flags, - int kflags, struct thread *td) + struct thread *td) { return (EINVAL); Modified: head/sys/kern/kern_sendfile.c ============================================================================== --- head/sys/kern/kern_sendfile.c Tue Mar 29 19:56:48 2016 (r297399) +++ head/sys/kern/kern_sendfile.c Tue Mar 29 19:57:11 2016 (r297400) @@ -516,7 +516,7 @@ sendfile_getsock(struct thread *td, int int vn_sendfile(struct file *fp, int sockfd, struct uio *hdr_uio, struct uio *trl_uio, off_t offset, size_t nbytes, off_t *sent, int flags, - int kflags, struct thread *td) + struct thread *td) { struct file *sock_fp; struct vnode *vp; @@ -534,7 +534,7 @@ vn_sendfile(struct file *fp, int sockfd, so = NULL; m = mh = NULL; sfs = NULL; - sbytes = 0; + hdrlen = sbytes = 0; softerr = 0; error = sendfile_getobj(td, fp, &obj, &vp, &shmfd, &obj_size, &bsize); @@ -560,26 +560,6 @@ vn_sendfile(struct file *fp, int sockfd, cv_init(&sfs->cv, "sendfile"); } - /* If headers are specified copy them into mbufs. */ - if (hdr_uio != NULL && hdr_uio->uio_resid > 0) { - hdr_uio->uio_td = td; - hdr_uio->uio_rw = UIO_WRITE; - /* - * In FBSD < 5.0 the nbytes to send also included - * the header. If compat is specified subtract the - * header size from nbytes. - */ - if (kflags & SFK_COMPAT) { - if (nbytes > hdr_uio->uio_resid) - nbytes -= hdr_uio->uio_resid; - else - nbytes = 0; - } - mh = m_uiotombuf(hdr_uio, M_WAITOK, 0, 0, 0); - hdrlen = m_length(mh, &mhtail); - } else - hdrlen = 0; - rem = nbytes ? omin(nbytes, obj_size - offset) : obj_size - offset; /* @@ -668,11 +648,20 @@ retry_space: SOCKBUF_UNLOCK(&so->so_snd); /* - * Reduce space in the socket buffer by the size of - * the header mbuf chain. - * hdrlen is set to 0 after the first loop. + * At the beginning of the first loop check if any headers + * are specified and copy them into mbufs. Reduce space in + * the socket buffer by the size of the header mbuf chain. + * Clear hdr_uio here and hdrlen at the end of the first loop. */ - space -= hdrlen; + if (hdr_uio != NULL && hdr_uio->uio_resid > 0) { + hdr_uio->uio_td = td; + hdr_uio->uio_rw = UIO_WRITE; + hdr_uio->uio_resid = min(hdr_uio->uio_resid, space); + mh = m_uiotombuf(hdr_uio, M_WAITOK, 0, 0, 0); + hdrlen = m_length(mh, &mhtail); + space -= hdrlen; + hdr_uio = NULL; + } if (vp != NULL) { error = vn_lock(vp, LK_SHARED); @@ -944,6 +933,19 @@ sendfile(struct thread *td, struct sendf &hdr_uio); if (error != 0) goto out; +#ifdef COMPAT_FREEBSD4 + /* + * In FreeBSD < 5.0 the nbytes to send also included + * the header. If compat is specified subtract the + * header size from nbytes. + */ + if (compat) { + if (uap->nbytes > hdr_uio->uio_resid) + uap->nbytes -= hdr_uio->uio_resid; + else + uap->nbytes = 0; + } +#endif } if (hdtr.trailers != NULL) { error = copyinuio(hdtr.trailers, hdtr.trl_cnt, @@ -965,7 +967,7 @@ sendfile(struct thread *td, struct sendf } error = fo_sendfile(fp, uap->s, hdr_uio, trl_uio, uap->offset, - uap->nbytes, &sbytes, uap->flags, compat ? SFK_COMPAT : 0, td); + uap->nbytes, &sbytes, uap->flags, td); fdrop(fp, td); if (uap->sbytes != NULL) Modified: head/sys/sys/file.h ============================================================================== --- head/sys/sys/file.h Tue Mar 29 19:56:48 2016 (r297399) +++ head/sys/sys/file.h Tue Mar 29 19:57:11 2016 (r297400) @@ -112,7 +112,7 @@ typedef int fo_chown_t(struct file *fp, struct ucred *active_cred, struct thread *td); typedef int fo_sendfile_t(struct file *fp, int sockfd, struct uio *hdr_uio, struct uio *trl_uio, off_t offset, size_t nbytes, - off_t *sent, int flags, int kflags, struct thread *td); + off_t *sent, int flags, struct thread *td); typedef int fo_seek_t(struct file *fp, off_t offset, int whence, struct thread *td); typedef int fo_fill_kinfo_t(struct file *fp, struct kinfo_file *kif, @@ -376,11 +376,11 @@ fo_chown(struct file *fp, uid_t uid, gid static __inline int fo_sendfile(struct file *fp, int sockfd, struct uio *hdr_uio, struct uio *trl_uio, off_t offset, size_t nbytes, off_t *sent, int flags, - int kflags, struct thread *td) + struct thread *td) { return ((*fp->f_ops->fo_sendfile)(fp, sockfd, hdr_uio, trl_uio, offset, - nbytes, sent, flags, kflags, td)); + nbytes, sent, flags, td)); } static __inline int Modified: head/sys/sys/socket.h ============================================================================== --- head/sys/sys/socket.h Tue Mar 29 19:56:48 2016 (r297399) +++ head/sys/sys/socket.h Tue Mar 29 19:57:11 2016 (r297400) @@ -594,7 +594,6 @@ struct sf_hdtr { #define SF_FLAGS(rh, flags) (((rh) << 16) | (flags)) #ifdef _KERNEL -#define SFK_COMPAT 0x00000001 #define SF_READAHEAD(flags) ((flags) >> 16) #endif /* _KERNEL */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201603291957.u2TJvBsf036848>