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