Vitalij, here is latest version of the patch. If you already run the previous one, no need to switch to this one, keep running as is. The update covers only FreeBSD 4 and i386 compatibilties. current_at_, a review is appreciated. The patch not only fixes a recent bug, but also fixes a long standing problem that headers were not checked against socket buffer size. One could push unlimited data into sendfile() with headers. The patch also pushes also compat code under ifdef, so it is cut away if you aren't interested in COMPAT_FREEBSD4. On Wed, Mar 23, 2016 at 04:59:25PM -0700, Gleb Smirnoff wrote: T> Vitalij, T> T> although the first patch should fixup the panic, can you please T> instead run this one. And if it is possible, can you please T> monitor this sysctl: T> T> sysctl kern.ipc.sf_long_headers T> T> T> -- T> Totus tuus, Glebius. T> Index: sys/kern/kern_descrip.c T> =================================================================== T> --- sys/kern/kern_descrip.c (revision 297217) T> +++ sys/kern/kern_descrip.c (working copy) T> _at__at_ -3958,7 +3958,7 _at__at_ badfo_chown(struct file *fp, uid_t uid, gid_t gid, T> static int T> badfo_sendfile(struct file *fp, int sockfd, struct uio *hdr_uio, T> struct uio *trl_uio, off_t offset, size_t nbytes, off_t *sent, int flags, T> - int kflags, struct thread *td) T> + struct thread *td) T> { T> T> return (EBADF); T> _at__at_ -4044,7 +4044,7 _at__at_ invfo_chown(struct file *fp, uid_t uid, gid_t gid, T> int T> invfo_sendfile(struct file *fp, int sockfd, struct uio *hdr_uio, T> struct uio *trl_uio, off_t offset, size_t nbytes, off_t *sent, int flags, T> - int kflags, struct thread *td) T> + struct thread *td) T> { T> T> return (EINVAL); T> Index: sys/kern/kern_sendfile.c T> =================================================================== T> --- sys/kern/kern_sendfile.c (revision 297217) T> +++ sys/kern/kern_sendfile.c (working copy) T> _at__at_ -95,6 +95,7 _at__at_ struct sendfile_sync { T> }; T> T> counter_u64_t sfstat[sizeof(struct sfstat) / sizeof(uint64_t)]; T> +static counter_u64_t sf_long_headers; /* QQQGL */ T> T> static void T> sfstat_init(const void *unused) T> _at__at_ -102,6 +103,7 _at__at_ sfstat_init(const void *unused) T> T> COUNTER_ARRAY_ALLOC(sfstat, sizeof(struct sfstat) / sizeof(uint64_t), T> M_WAITOK); T> + sf_long_headers = counter_u64_alloc(M_WAITOK); /* QQQGL */ T> } T> SYSINIT(sfstat, SI_SUB_MBUF, SI_ORDER_FIRST, sfstat_init, NULL); T> T> _at__at_ -117,6 +119,8 _at__at_ sfstat_sysctl(SYSCTL_HANDLER_ARGS) T> } T> SYSCTL_PROC(_kern_ipc, OID_AUTO, sfstat, CTLTYPE_OPAQUE | CTLFLAG_RW, T> NULL, 0, sfstat_sysctl, "I", "sendfile statistics"); T> +SYSCTL_COUNTER_U64(_kern_ipc, OID_AUTO, sf_long_headers, CTLFLAG_RW, T> + &sf_long_headers, "times headers did not fit into socket buffer"); T> T> /* T> * Detach mapped page and release resources back to the system. Called T> _at__at_ -516,7 +520,7 _at__at_ sendfile_getsock(struct thread *td, int s, struct T> int T> vn_sendfile(struct file *fp, int sockfd, struct uio *hdr_uio, T> struct uio *trl_uio, off_t offset, size_t nbytes, off_t *sent, int flags, T> - int kflags, struct thread *td) T> + struct thread *td) T> { T> struct file *sock_fp; T> struct vnode *vp; T> _at__at_ -534,7 +538,7 _at__at_ vn_sendfile(struct file *fp, int sockfd, struct ui T> so = NULL; T> m = mh = NULL; T> sfs = NULL; T> - sbytes = 0; T> + hdrlen = sbytes = 0; T> softerr = 0; T> T> error = sendfile_getobj(td, fp, &obj, &vp, &shmfd, &obj_size, &bsize); T> _at__at_ -560,26 +564,6 _at__at_ vn_sendfile(struct file *fp, int sockfd, struct ui T> cv_init(&sfs->cv, "sendfile"); T> } T> T> - /* If headers are specified copy them into mbufs. */ T> - if (hdr_uio != NULL && hdr_uio->uio_resid > 0) { T> - hdr_uio->uio_td = td; T> - hdr_uio->uio_rw = UIO_WRITE; T> - /* T> - * In FBSD < 5.0 the nbytes to send also included T> - * the header. If compat is specified subtract the T> - * header size from nbytes. T> - */ T> - if (kflags & SFK_COMPAT) { T> - if (nbytes > hdr_uio->uio_resid) T> - nbytes -= hdr_uio->uio_resid; T> - else T> - nbytes = 0; T> - } T> - mh = m_uiotombuf(hdr_uio, M_WAITOK, 0, 0, 0); T> - hdrlen = m_length(mh, &mhtail); T> - } else T> - hdrlen = 0; T> - T> rem = nbytes ? omin(nbytes, obj_size - offset) : obj_size - offset; T> T> /* T> _at__at_ -668,11 +652,23 _at__at_ retry_space: T> SOCKBUF_UNLOCK(&so->so_snd); T> T> /* T> - * Reduce space in the socket buffer by the size of T> - * the header mbuf chain. T> - * hdrlen is set to 0 after the first loop. T> + * At the beginning of the first loop check if any headers T> + * are specified and copy them into mbufs. Reduce space in T> + * the socket buffer by the size of the header mbuf chain. T> + * Clear hdr_uio here and hdrlen at the end of the first loop. T> */ T> - space -= hdrlen; T> + if (hdr_uio != NULL) { T> + hdr_uio->uio_td = td; T> + hdr_uio->uio_rw = UIO_WRITE; T> + /* QQQGL remove counter */ T> + if (space < hdr_uio->uio_resid) T> + counter_u64_add(sf_long_headers, 1); T> + hdr_uio->uio_resid = min(hdr_uio->uio_resid, space); T> + mh = m_uiotombuf(hdr_uio, M_WAITOK, 0, 0, 0); T> + hdrlen = m_length(mh, &mhtail); T> + space -= hdrlen; T> + hdr_uio = NULL; T> + } T> T> if (vp != NULL) { T> error = vn_lock(vp, LK_SHARED); T> _at__at_ -944,6 +940,17 _at__at_ sendfile(struct thread *td, struct sendfile_args * T> &hdr_uio); T> if (error != 0) T> goto out; T> + /* T> + * In FBSD < 5.0 the nbytes to send also included T> + * the header. If compat is specified subtract the T> + * header size from nbytes. T> + */ T> + if (compat) { T> + if (uap->nbytes > hdr_uio->uio_resid) T> + uap->nbytes -= hdr_uio->uio_resid; T> + else T> + uap->nbytes = 0; T> + } T> } T> if (hdtr.trailers != NULL) { T> error = copyinuio(hdtr.trailers, hdtr.trl_cnt, T> _at__at_ -965,7 +972,7 _at__at_ sendfile(struct thread *td, struct sendfile_args * T> } T> T> error = fo_sendfile(fp, uap->s, hdr_uio, trl_uio, uap->offset, T> - uap->nbytes, &sbytes, uap->flags, compat ? SFK_COMPAT : 0, td); T> + uap->nbytes, &sbytes, uap->flags, td); T> fdrop(fp, td); T> T> if (uap->sbytes != NULL) T> Index: sys/sys/file.h T> =================================================================== T> --- sys/sys/file.h (revision 297217) T> +++ sys/sys/file.h (working copy) T> _at__at_ -112,7 +112,7 _at__at_ typedef int fo_chown_t(struct file *fp, uid_t uid, T> struct ucred *active_cred, struct thread *td); T> typedef int fo_sendfile_t(struct file *fp, int sockfd, struct uio *hdr_uio, T> struct uio *trl_uio, off_t offset, size_t nbytes, T> - off_t *sent, int flags, int kflags, struct thread *td); T> + off_t *sent, int flags, struct thread *td); T> typedef int fo_seek_t(struct file *fp, off_t offset, int whence, T> struct thread *td); T> typedef int fo_fill_kinfo_t(struct file *fp, struct kinfo_file *kif, T> _at__at_ -376,11 +376,11 _at__at_ fo_chown(struct file *fp, uid_t uid, gid_t gid, st T> static __inline int T> fo_sendfile(struct file *fp, int sockfd, struct uio *hdr_uio, T> struct uio *trl_uio, off_t offset, size_t nbytes, off_t *sent, int flags, T> - int kflags, struct thread *td) T> + struct thread *td) T> { T> T> return ((*fp->f_ops->fo_sendfile)(fp, sockfd, hdr_uio, trl_uio, offset, T> - nbytes, sent, flags, kflags, td)); T> + nbytes, sent, flags, td)); T> } T> T> static __inline int T> Index: sys/sys/socket.h T> =================================================================== T> --- sys/sys/socket.h (revision 297217) T> +++ sys/sys/socket.h (working copy) T> _at__at_ -594,7 +594,6 _at__at_ struct sf_hdtr { T> #define SF_FLAGS(rh, flags) (((rh) << 16) | (flags)) T> T> #ifdef _KERNEL T> -#define SFK_COMPAT 0x00000001 T> #define SF_READAHEAD(flags) ((flags) >> 16) T> #endif /* _KERNEL */ T> T> _______________________________________________ T> freebsd-current_at_freebsd.org mailing list T> https://lists.freebsd.org/mailman/listinfo/freebsd-current T> To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org" -- Totus tuus, Glebius.
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:41:03 UTC