wifi: mac80211_hwsim: reject undersized HWSIM_ATTR_TX_INFO
Subsystems (3)
PATCH 1/1 wifi: mac80211_hwsim: reject undersized HWSIM_ATTR_TX_INFO
hwsim_tx_info_frame_received_nl() casts the HWSIM_ATTR_TX_INFO attribute payload to a struct hwsim_tx_rate * and then unconditionally walks IEEE80211_TX_MAX_RATES (4) entries of it:
tx_attempts = (struct hwsim_tx_rate *)nla_data(
info->attrs[HWSIM_ATTR_TX_INFO]);
...
for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) {
txi->status.rates[i].idx = tx_attempts[i].idx;
txi->status.rates[i].count = tx_attempts[i].count;
}struct hwsim_tx_rate is {s8 idx; u8 count;} __packed, i.e. 2 bytes, so the loop reads a fixed 8 bytes out of the attribute regardless of its actual length.
The attribute's policy entry only bounds the payload from above:
[HWSIM_ATTR_TX_INFO] = { .type = NLA_BINARY,
.len = IEEE80211_TX_MAX_RATES *
sizeof(struct hwsim_tx_rate)},For NLA_BINARY, nla_policy.len is a maximum, not a minimum - nla_validate() happily accepts a shorter attribute, including a zero-length one. The HWSIM_CMD_TX_INFO_FRAME op also sets GENL_DONT_VALIDATE_STRICT, so no implicit minimum-length enforcement kicks in either. The handler itself never calls nla_len() on HWSIM_ATTR_TX_INFO before dereferencing it.
The net effect: any process that has completed HWSIM_CMD_REGISTER as the netgroup's wmediumd (CAP_NET_ADMIN at registration time, e.g. via an unprivileged user+net namespace) can send a HWSIM_CMD_TX_INFO_FRAME with a 0-length HWSIM_ATTR_TX_INFO and drive tx_attempts[0..3] to read up to 8 bytes past the end of the attribute payload.
This driver already validates NLA_BINARY payload length before use in two other handlers in this same file:
- hwsim_cloned_frame_received_nl() bounds-checks HWSIM_ATTR_FRAME before touching it:
frame_data_len = nla_len(info->attrs[HWSIM_ATTR_FRAME]); ... if (frame_data_len < sizeof(struct ieee80211_hdr_3addr) || frame_data_len > IEEE80211_MAX_DATA_LEN) goto err;
- hwsim_new_radio_nl() bounds-checks HWSIM_ATTR_CIPHER_SUPPORT before use, rejecting a length that doesn't cleanly divide into whole elements:
u32 len = nla_len(info->attrs[HWSIM_ATTR_CIPHER_SUPPORT]);
...
if (len % sizeof(u32)) {
NL_SET_ERR_MSG_ATTR(...);
return -EINVAL;
}hwsim_tx_info_frame_received_nl() is missing the equivalent check for HWSIM_ATTR_TX_INFO. Add a minimum-length check next to the existing attrs-presence check at the top of the function (before any pending skb is unlinked from data2->pending, to avoid holding/leaking that skb on the new error path), requiring at least IEEE80211_TX_MAX_RATES * sizeof(struct hwsim_tx_rate) bytes - matching the exact quantity the loop below unconditionally reads. A too-short attribute now takes the existing goto out; / -EINVAL path instead of reading past the attribute payload.
On a v6.19 KASAN-enabled stand, the differential reproducer confirmed the unpatched handler accepts a 0-length HWSIM_ATTR_TX_INFO and runs the fixed-count read loop to completion (ack err = 0); KASAN did not report anything here, since the resulting 8-byte over-read stays inside the same skb's kmalloc/shinfo tailroom rather than crossing a slab redzone. The check added above makes the same 0-length attribute fail before that loop runs, taking the existing goto out / -EINVAL path instead.
--- drivers/net/wireless/virtual/mac80211_hwsim_main.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
drivers/net/wireless/virtual/mac80211_hwsim_main.c +11 -0
@@ -6340,6 +6340,17 @@ static int hwsim_tx_info_frame_received_nl(struct sk_buff *skb_2, !info->attrs[HWSIM_ATTR_TX_INFO]) goto out; +/* +* HWSIM_ATTR_TX_INFO is NLA_BINARY with only a maximum length in +* its policy entry, so nla_validate() accepts any shorter (incl. +* zero-length) payload. The read loop below unconditionally walks +* IEEE80211_TX_MAX_RATES entries, so reject undersized attributes +* here before any data is touched. +*/ +if (nla_len (info->attrs[HWSIM_ATTR_TX_INFO ]) < +IEEE80211_TX_MAX_RATES *sizeof (struct hwsim_tx_rate )) + goto out; + src = (void *)nla_data (info->attrs[HWSIM_ATTR_ADDR_TRANSMITTER ]); hwsim_flags =nla_get_u32 (info->attrs[HWSIM_ATTR_FLAGS ]); ret_skb_cookie =nla_get_u64 (info->attrs[HWSIM_ATTR_COOKIE ]);
Signature · 1 line
2.50.1 (Apple Git-155)
Johannes Berg
maintainerQuoted 3 lines · Ibrahim Hashimov
hwsim_tx_info_frame_received_nl()casts theHWSIM_ATTR_TX_INFOattribute payload to astruct hwsim_tx_rate *and then unconditionally walksIEEE80211_TX_MAX_RATES(4) entries of it:
[snip]
Way too much text. Please edit manually, LLMs like to be super verbose.
Quoted 3 lines
[HWSIM_ATTR_TX_INFO] = { .type = NLA_BINARY,
.len = IEEE80211_TX_MAX_RATES *
sizeof(struct hwsim_tx_rate)},So, you found the policy, why not fix that instead?
johannes