tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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



Hi,
I have a fix for kern/44310 at the end of my post but first some
commentary:

-static int     bpf_movein(struct uio *, int, int,
+static int     bpf_movein(struct uio *, int, uint64_t,
                                struct mbuf **, struct sockaddr *);

I change mtu type because the actual argument ifp->if_mtu at the call
point is uint64_t.


-       int len;
-       int hlen;
-       int align;
+       size_t len;
+       size_t hlen;
+       size_t align;

There is len = uio->uio_resid assignment later in the fuction
and len should be size_t, two other variables can as well be
unsigned int.

Interestingly, these variables are ints in FreeBSD and u_int in OpenBSD.


-       if (len < hlen || len - hlen > mtu)
+       if (len - hlen > mtu)
                return (EMSGSIZE);
 
Since mtu is always less than (SIZE_MAX-hlen), we can elimitate the first
check and rely on "wrapping" of unsigned subtraction when (len<hlen).

Patched code looks like FreeBSD code (but len and hlen are ints in
FreeBSD).

-       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) {

I make (len+align) a common expression (I checked with objdump -D that
the compiler really reuses the expression).

Thoughts, comments?

Thansk,
Alex

Index: sys/net/bpf.c
===================================================================
RCS file: /cvsroot/src/sys/net/bpf.c,v
retrieving revision 1.159
diff -u -u -r1.159 bpf.c
--- sys/net/bpf.c       8 Dec 2010 17:10:13 -0000       1.159
+++ sys/net/bpf.c       2 Jan 2011 14:16:15 -0000
@@ -134,7 +134,7 @@
 static void    bpf_freed(struct bpf_d *);
 static void    bpf_ifname(struct ifnet *, struct ifreq *);
 static void    *bpf_mcpy(void *, const void *, size_t);
-static int     bpf_movein(struct uio *, int, int,
+static int     bpf_movein(struct uio *, int, uint64_t,
                                struct mbuf **, struct sockaddr *);
 static void    bpf_attachd(struct bpf_d *, struct bpf_if *);
 static void    bpf_detachd(struct bpf_d *);
@@ -179,14 +179,14 @@
 };
 
 static int
-bpf_movein(struct uio *uio, int linktype, int mtu, struct mbuf **mp,
+bpf_movein(struct uio *uio, int linktype, uint64_t mtu, struct mbuf **mp,
           struct sockaddr *sockp)
 {
        struct mbuf *m;
        int error;
-       int len;
-       int hlen;
-       int align;
+       size_t len;
+       size_t hlen;
+       size_t align;
 
        /*
         * Build a sockaddr based on the data link layer type.
@@ -253,7 +253,7 @@
         * If there aren't enough bytes for a link level header or the
         * packet length exceeds the interface mtu, return an error.
         */
-       if (len < hlen || len - hlen > mtu)
+       if (len - hlen > mtu)
                return (EMSGSIZE);
 
        /*
@@ -261,13 +261,13 @@
         * bail if it won't fit in a single mbuf.
         * (Take into account possible alignment bytes)
         */
-       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_clget(m, M_WAIT);
                if ((m->m_flags & M_EXT) == 0) {
                        error = ENOBUFS;
@@ -278,7 +278,7 @@
        /* Insure the data is properly aligned */
        if (align > 0) {
                m->m_data += align;
-               m->m_len -= align;
+               m->m_len -= (int)align;
        }
 
        error = uiomove(mtod(m, void *), len, uio);
@@ -289,7 +289,7 @@
                m->m_data += hlen; /* XXX */
                len -= hlen;
        }
-       m->m_len = len;
+       m->m_len = (int)len;
        *mp = m;
        return (0);
 


Home | Main Index | Thread Index | Old Index