Hi,
[email protected] has corrected an use-after-free on NetBSD on similar
code we have.
Fix use-after-free, m_cat can free m.
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/net80211/ieee80211_input.c.diff?r1=1.111&r2=1.112
>From code reading on us side, I think the same problem is present.
net80211/ieee80211_input.c
538 /*
539 * Handle defragmentation (see 9.5 and Annex C). We support the
concurrent
540 * reception of fragments of three fragmented MSDUs or MMPDUs.
541 */
542 struct mbuf *
543 ieee80211_defrag(struct ieee80211com *ic, struct mbuf *m, int hdrlen)
544 {
...
597 /* strip 802.11 header and concatenate fragment */
598 m_adj(m, hdrlen);
599 m_cat(df->df_m, m);
600 df->df_m->m_pkthdr.len += m->m_pkthdr.len;
the second argument of m_cat() `m' could be freed: m_cat() could call
m_free() on it. so I assume it isn't safe to deference it to get
`m_pkthdr.len`.
Here m_cat() code:
kern/uipc_mbuf.c
772 /*
773 * Concatenate mbuf chain n to m.
774 * n might be copied into m (when n->m_len is small), therefore data
portion of
775 * n could be copied into an mbuf of different mbuf type.
776 * Therefore both chains should be of the same type (e.g. MT_DATA).
777 * Any m_pkthdr is not updated.
778 */
779 void
780 m_cat(struct mbuf *m, struct mbuf *n)
781 {
782 while (m->m_next)
783 m = m->m_next;
784 while (n) {
785 if (M_READONLY(m) || n->m_len > M_TRAILINGSPACE(m)) {
786 /* just join the two chains */
787 m->m_next = n;
788 return;
789 }
790 /* splat the data from one into the other */
791 memcpy(mtod(m, caddr_t) + m->m_len, mtod(n, caddr_t),
792 n->m_len);
793 m->m_len += n->m_len;
794 n = m_free(n);
795 }
796 }
the NetBSD diff seems to mix 2 things. I understand the first (saving
`m->m_pkthdr.len' in intermediate variable for adjust
`df->df_m->m_pkthdr.len' later), but I am unsure to the second.
The diff below contains only the mlen part.
I quickly checked other uses of m_cat() inside the kernel, and they
seems corrects.
Thanks.
--
Sebastien Marie
Index: net80211/ieee80211_input.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.201
diff -u -p -r1.201 ieee80211_input.c
--- net80211/ieee80211_input.c 5 May 2018 06:58:05 -0000 1.201
+++ net80211/ieee80211_input.c 21 Jun 2018 17:37:41 -0000
@@ -546,7 +546,7 @@ ieee80211_defrag(struct ieee80211com *ic
struct ieee80211_defrag *df;
u_int16_t rxseq, seq;
u_int8_t frag;
- int i;
+ int i, mlen;
wh = mtod(m, struct ieee80211_frame *);
rxseq = letoh16(*(const u_int16_t *)wh->i_seq);
@@ -596,8 +596,9 @@ ieee80211_defrag(struct ieee80211com *ic
df->df_frag = frag;
/* strip 802.11 header and concatenate fragment */
m_adj(m, hdrlen);
+ mlen = m->m_pkthdr.len;
m_cat(df->df_m, m);
- df->df_m->m_pkthdr.len += m->m_pkthdr.len;
+ df->df_m->m_pkthdr.len += mlen;
if (wh->i_fc[1] & IEEE80211_FC1_MORE_FRAG)
return NULL; /* MSDU or MMPDU not yet complete */
