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@, 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> @@ -3958,7 +3958,7 @@ 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> @@ -4044,7 +4044,7 @@ 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> @@ -95,6 +95,7 @@ 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> @@ -102,6 +103,7 @@ 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> @@ -117,6 +119,8 @@ 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> @@ -516,7 +520,7 @@ 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> @@ -534,7 +538,7 @@ 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> @@ -560,26 +564,6 @@ 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> @@ -668,11 +652,23 @@ 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> @@ -944,6 +940,17 @@ 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> @@ -965,7 +972,7 @@ 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> @@ -112,7 +112,7 @@ 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> @@ -376,11 +376,11 @@ 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> @@ -594,7 +594,6 @@ 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@freebsd.org mailing list
GS> T> https://lists.freebsd.org/mailman/listinfo/freebsd-current
GS> T> To unsubscribe, send any mail to
"freebsd-current-unsubscr...@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> @@ -1653,6 +1653,19 @@ 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> @@ -1670,7 +1683,7 @@ 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> @@ -3958,7 +3958,7 @@ 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> @@ -4044,7 +4044,7 @@ 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> @@ -95,6 +95,7 @@ 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> @@ -102,6 +103,7 @@ 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> @@ -117,6 +119,9 @@ 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> @@ -516,7 +521,7 @@ 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> @@ -534,7 +539,7 @@ 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> @@ -560,26 +565,6 @@ 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> @@ -668,11 +653,23 @@ 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> @@ -944,6 +941,19 @@ 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> @@ -965,7 +975,7 @@ 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> @@ -112,7 +112,7 @@ 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> @@ -376,11 +376,11 @@ 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> @@ -594,7 +594,6 @@ 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@freebsd.org mailing list
GS> https://lists.freebsd.org/mailman/listinfo/freebsd-current
GS> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
_______________________________________________
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
