I have debugged an mbuf corruption issue which can occur in net80211
hostap mode, with help from claudio, kettenis, and dlg.
The gist of the fix is this:
m = ieee80211_getmgmt(M_DONTWAIT, MT_DATA,
8 + 2 + 2 +
- 2 + ni->ni_esslen +
+ 2 + ic->ic_bss->ni_esslen +
This code allocates a frame for a probe response the AP will send to an
interested client. When sizing this allocation the old code used the SSID
length stored in the node structure which represents the client. Later on
ic_bss->ni_esslen (in the AP's own node structure) is used when copying
the AP's SSID into the probe response frame.
If this length is longer than ni->ni_esslen we end up writing past the
memory buffer, which can result in corruption of an adjacent mbuf on the
free list. Then bad things happen later on when the adjacent mbuf is used.
To prevent such a mistake from occuring again I am removing 'ni' from
this function altogether. All information in the probe response should
be based on local parameters of the AP (stored in ic_bss).
Ok?
This bug is likely the reason for several past crash reports, including
https://marc.info/?l=openbsd-bugs&m=154729090512697&w=2
and https://marc.info/?l=openbsd-bugs&m=155916096205470&w=2
Index: ieee80211_output.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_output.c,v
retrieving revision 1.126
diff -u -p -r1.126 ieee80211_output.c
--- ieee80211_output.c 29 Jul 2019 10:50:09 -0000 1.126
+++ ieee80211_output.c 17 Feb 2020 15:27:27 -0000
@@ -73,8 +73,7 @@ struct mbuf *ieee80211_getmgmt(int, int,
struct mbuf *ieee80211_get_probe_req(struct ieee80211com *,
struct ieee80211_node *);
#ifndef IEEE80211_STA_ONLY
-struct mbuf *ieee80211_get_probe_resp(struct ieee80211com *,
- struct ieee80211_node *);
+struct mbuf *ieee80211_get_probe_resp(struct ieee80211com *);
#endif
struct mbuf *ieee80211_get_auth(struct ieee80211com *,
struct ieee80211_node *, u_int16_t, u_int16_t);
@@ -1239,7 +1238,7 @@ ieee80211_get_probe_req(struct ieee80211
* [tlv] HT Operation (802.11n)
*/
struct mbuf *
-ieee80211_get_probe_resp(struct ieee80211com *ic, struct ieee80211_node *ni)
+ieee80211_get_probe_resp(struct ieee80211com *ic)
{
const struct ieee80211_rateset *rs = &ic->ic_bss->ni_rates;
struct mbuf *m;
@@ -1247,7 +1246,7 @@ ieee80211_get_probe_resp(struct ieee8021
m = ieee80211_getmgmt(M_DONTWAIT, MT_DATA,
8 + 2 + 2 +
- 2 + ni->ni_esslen +
+ 2 + ic->ic_bss->ni_esslen +
2 + min(rs->rs_nrates, IEEE80211_RATE_SIZE) +
2 + 1 +
((ic->ic_opmode == IEEE80211_M_IBSS) ? 2 + 2 : 0) +
@@ -1268,13 +1267,13 @@ ieee80211_get_probe_resp(struct ieee8021
frm = mtod(m, u_int8_t *);
memset(frm, 0, 8); frm += 8; /* timestamp is set by hardware */
LE_WRITE_2(frm, ic->ic_bss->ni_intval); frm += 2;
- frm = ieee80211_add_capinfo(frm, ic, ni);
+ frm = ieee80211_add_capinfo(frm, ic, ic->ic_bss);
frm = ieee80211_add_ssid(frm, ic->ic_bss->ni_essid,
ic->ic_bss->ni_esslen);
frm = ieee80211_add_rates(frm, rs);
- frm = ieee80211_add_ds_params(frm, ic, ni);
+ frm = ieee80211_add_ds_params(frm, ic, ic->ic_bss);
if (ic->ic_opmode == IEEE80211_M_IBSS)
- frm = ieee80211_add_ibss_params(frm, ni);
+ frm = ieee80211_add_ibss_params(frm, ic->ic_bss);
if (ic->ic_curmode == IEEE80211_MODE_11G)
frm = ieee80211_add_erp(frm, ic);
if (rs->rs_nrates > IEEE80211_RATE_SIZE)
@@ -1777,7 +1776,7 @@ ieee80211_send_mgmt(struct ieee80211com
break;
#ifndef IEEE80211_STA_ONLY
case IEEE80211_FC0_SUBTYPE_PROBE_RESP:
- if ((m = ieee80211_get_probe_resp(ic, ni)) == NULL)
+ if ((m = ieee80211_get_probe_resp(ic)) == NULL)
senderr(ENOMEM, is_tx_nombuf);
break;
#endif
