Date: Fri, 06 May 2022 19:15:21 +0000 From: bugzilla-noreply@freebsd.org To: bugs@FreeBSD.org Subject: [Bug 263824] [genet] genet driver interface may overwrite memory in a consecutive memory copy operations when parse TX packet Message-ID: <bug-263824-227@https.bugs.freebsd.org/bugzilla/>
next in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D263824 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 =3D size; \ if (copy) { \ if (shift) { \ u_char *p0; \ shift =3D false; \ p0 =3D mtodo(m0, sizeof(struct statusblock)); \ m0->m_data =3D m0->m_pktdat; \ bcopy(p0, mtodo(m0, sizeof(struct statusblock)),\ m0->m_len - sizeof(struct statusblock)); \ copy_p =3D mtodo(m0, sizeof(struct statusblock)); \ } \ bcopy(p, copy_p, hsize); \ m0->m_len +=3D hsize; \ m0->m_pkthdr.len +=3D hsize; /* unneeded */ \ m->m_len -=3D hsize; \ m->m_data +=3D hsize; \ } \ copy_p +=3D 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 c= opy operations in the "COPY()" function. The first will be performed if the "sh= ift" variable is set as true. The second memory copy operation is performed whet= her the first memory copy operation is performed or not. The "m0->m_len" is the data length of the original "m0->m_data". "p0" point= s to "m0->m_data + sizeof(statusblock)". The "m0->m_data" will then be changed to point to "m0->m_pktdat".=20 The memory overwrite will occur when the "shift" variable is true and "m0->m_len" is larger than the "sizeof(struct statusblock)".=20 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_da= ta + 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?=20 - copy_p =3D mtodo(m0, sizeof(struct statusblock)); \ + copy_p =3D 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 w= hen I tried to send a large ping packet, for example "ping -s 2048 .....". --=20 You are receiving this mail because: You are the assignee for the bug.=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-263824-227>