[Bug 263824] [genet] genet driver interface may overwrite memory in a consecutive memory copy operations when parse TX packet

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263824

            Bug ID: 263824
           Summary: [genet] genet driver interface may overwrite memory in
                    a consecutive memory copy operations when parse TX
                    packet
           Product: Base System
           Version: CURRENT
          Hardware: arm64
                OS: Any
            Status: New
          Severity: Affects Some People
          Priority: ---
         Component: bin
          Assignee: bugs@FreeBSD.org
          Reporter: jiahali@blackberry.com

if_genet.c beginning line 1247 in function gen_parse_tx()

#define COPY(size) {                                                    \
        int hsize = size;                                               \
        if (copy) {                                                     \
                if (shift) {                                            \
                        u_char *p0;                                     \
                        shift = false;                                  \
                        p0 = mtodo(m0, sizeof(struct statusblock));     \
                        m0->m_data = m0->m_pktdat;                      \
                        bcopy(p0, mtodo(m0, sizeof(struct statusblock)),\
                            m0->m_len - sizeof(struct statusblock));    \
                        copy_p = mtodo(m0, sizeof(struct statusblock)); \
                }                                                       \
                bcopy(p, copy_p, hsize);                                \
                m0->m_len += hsize;                                     \
                m0->m_pkthdr.len += hsize;      /* unneeded */          \
                m->m_len -= hsize;                                      \
                m->m_data += hsize;                                     \
        }                                                               \
        copy_p += hsize;                                                \
}

In mbuf.h line 116, the definition of mtodo() is
#define mtodo(m, o)     ((void *)(((m)->m_data) + (o)))

In genet, the "COPY()" macro function will copy the Link Layer Header and
Network Layer Header into a contiguous memory space. There are two memory copy
operations in the "COPY()" function. The first will be performed if the "shift"
variable is set as true. The second memory copy operation is performed whether
the first memory copy operation is performed or not.

The "m0->m_len" is the data length of the original "m0->m_data". "p0" points to
"m0->m_data + sizeof(statusblock)". The "m0->m_data" will then be changed to
point to "m0->m_pktdat". 

The memory overwrite will occur when the "shift" variable is true and
"m0->m_len" is larger than the "sizeof(struct statusblock)". 

The first "bcopy()" will copy all the contents excepting "statusblock" from the
old "m0->m_data", starting at "p0", to the position starting at "m0->m_data +
sizeof(struct statusblock)". The "copy_p" will be set to point to "m0->m_data +
sizeof(struct statusblock)".

The second "bcopy()" will copy the contents from "p" to "copy_p" which will
overwrite some/all of the contents which are copied at the first copy.

Should "copy_p" point to the "m0->m_data + sizeof(struct statusblock) +
m0->m_len - sizeof(struct statusblock)" to prevent memory overwrite? 

-                       copy_p = mtodo(m0, sizeof(struct statusblock)); \
+                       copy_p = mtodo(m0, m0->m_len);  \

It is a rare case that only happens when the content of a packet is located in
different mbufs in a mbuf chain. It happens in my development environment when
I tried to send a large ping packet, for example "ping -s 2048 .....".

-- 
You are receiving this mail because:
You are the assignee for the bug.