Re: Fix for kern/44310 (write to /dev/bpf truncates size_t to int)

[Alexander Nasonov <alnsn%yandex.ru@localhost>]

I have a small addition to the previous which got lost in my
notes. I think that (m->m_pkthdr.len > ifp->if_mtu) check after
the bpf_movein() call is redundant.

Alexander Nasonov wrote:
> Hi,
> I have a fix for kern/44310 at the end of my post but first some
> commentary:
> ...
> -     if (len < hlen || len - hlen > mtu)
> +     if (len - hlen > mtu)
>               return (EMSGSIZE);

From this point, (len - hlen) <= mtu.

>
> -     if ((unsigned)len > MCLBYTES - align)
> +     if (len + align > MCLBYTES)
>               return (EIO);
>  
>       m = m_gethdr(M_WAIT, MT_DATA);
>       m->m_pkthdr.rcvif = 0;
> -     m->m_pkthdr.len = len - hlen;
> -     if (len > MHLEN - align) {
> +     m->m_pkthdr.len = (int)(len - hlen);
> +     if (len + align > MHLEN) {

m->m_pkthdr.len is <= mtu now and nothing changes it in the rest
of the function (my note tells me to check whether m_clget(m)
further down the function can change m_pkthdr.len but it doesn't
seem to change anything in m_pkthdr at all).

BTW, m_clget is not documented. If it's obsolete, shall it be
replaced with MCLGET macro? My patch below has a fix for this
as well.

Alex

$ cvs diff -U7 sys/net/bpf.c
Index: sys/net/bpf.c
===================================================================
RCS file: /cvsroot/src/sys/net/bpf.c,v
retrieving revision 1.162
diff -u -U 7 -r1.162 bpf.c
--- sys/net/bpf.c       22 Jan 2011 19:12:58 -0000      1.162
+++ sys/net/bpf.c       23 Jan 2011 12:34:09 -0000
@@ -264,15 +264,15 @@
        if (len + align > MCLBYTES)
                return (EIO);

        m = m_gethdr(M_WAIT, MT_DATA);
        m->m_pkthdr.rcvif = 0;
        m->m_pkthdr.len = (int)(len - hlen);
        if (len + align > MHLEN) {
-               m_clget(m, M_WAIT);
+               MCLGET(m, M_WAIT);
                if ((m->m_flags & M_EXT) == 0) {
                        error = ENOBUFS;
                        goto bad;
                }
        }

        /* Insure the data is properly aligned */
@@ -647,20 +647,14 @@
        error = bpf_movein(uio, (int)d->bd_bif->bif_dlt, ifp->if_mtu, &m,
                (struct sockaddr *) &dst);
        if (error) {
                KERNEL_UNLOCK_ONE(NULL);
                return (error);
        }

-       if (m->m_pkthdr.len > ifp->if_mtu) {
-               KERNEL_UNLOCK_ONE(NULL);
-               m_freem(m);
-               return (EMSGSIZE);
-       }
-
        if (d->bd_hdrcmplt)
                dst.ss_family = pseudo_AF_HDRCMPLT;

        if (d->bd_feedback) {
                mc = m_dup(m, 0, M_COPYALL, M_NOWAIT);
                if (mc != NULL)
                        mc->m_pkthdr.rcvif = ifp;