Commit 9b8a9e6c authored by Jérôme Pouiller's avatar Jérôme Pouiller Committed by Greg Kroah-Hartman

staging: wfx: improve protection against malformed HIF messages

As discussed here[1], if a message was smaller than the size of the
message header, it could be incorrectly processed.

[1] https://lore.kernel.org/driverdev-devel/2302785.6C7ODC2LYm@pc-42/Signed-off-by: default avatarJérôme Pouiller <jerome.pouiller@silabs.com>
Link: https://lore.kernel.org/r/20200701150707.222985-7-Jerome.Pouiller@silabs.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent a9408ad7
...@@ -57,7 +57,6 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf) ...@@ -57,7 +57,6 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
int release_count; int release_count;
int piggyback = 0; int piggyback = 0;
WARN(read_len < 4, "corrupted read");
WARN(read_len > round_down(0xFFF, 2) * sizeof(u16), WARN(read_len > round_down(0xFFF, 2) * sizeof(u16),
"%s: request exceed WFx capability", __func__); "%s: request exceed WFx capability", __func__);
...@@ -76,20 +75,17 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf) ...@@ -76,20 +75,17 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
hif = (struct hif_msg *)skb->data; hif = (struct hif_msg *)skb->data;
WARN(hif->encrypted & 0x1, "unsupported encryption type"); WARN(hif->encrypted & 0x1, "unsupported encryption type");
if (hif->encrypted == 0x2) { if (hif->encrypted == 0x2) {
if (wfx_sl_decode(wdev, (void *)hif)) { if (WARN(read_len < sizeof(struct hif_sl_msg), "corrupted read"))
dev_kfree_skb(skb); goto err;
// If frame was a confirmation, expect trouble in next computed_len = le16_to_cpu(((struct hif_sl_msg *)hif)->len);
// exchange. However, it is harmless to fail to decode computed_len = round_up(computed_len - sizeof(u16), 16);
// an indication frame, so try to continue. Anyway, computed_len += sizeof(struct hif_sl_msg);
// piggyback is probably correct. computed_len += sizeof(struct hif_sl_tag);
return piggyback;
}
computed_len =
round_up(le16_to_cpu(hif->len) - sizeof(hif->len), 16) +
sizeof(struct hif_sl_msg) +
sizeof(struct hif_sl_tag);
} else { } else {
computed_len = round_up(le16_to_cpu(hif->len), 2); if (WARN(read_len < sizeof(struct hif_msg), "corrupted read"))
goto err;
computed_len = le16_to_cpu(hif->len);
computed_len = round_up(computed_len, 2);
} }
if (computed_len != read_len) { if (computed_len != read_len) {
dev_err(wdev->dev, "inconsistent message length: %zu != %zu\n", dev_err(wdev->dev, "inconsistent message length: %zu != %zu\n",
...@@ -98,6 +94,16 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf) ...@@ -98,6 +94,16 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
hif, read_len, true); hif, read_len, true);
goto err; goto err;
} }
if (hif->encrypted == 0x2) {
if (wfx_sl_decode(wdev, (struct hif_sl_msg *)hif)) {
dev_kfree_skb(skb);
// If frame was a confirmation, expect trouble in next
// exchange. However, it is harmless to fail to decode
// an indication frame, so try to continue. Anyway,
// piggyback is probably correct.
return piggyback;
}
}
if (!(hif->id & HIF_ID_IS_INDICATION)) { if (!(hif->id & HIF_ID_IS_INDICATION)) {
(*is_cnf)++; (*is_cnf)++;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment