diff options
| author |   Jérôme Pouiller <jerome.pouiller@silabs.com> | 2020-07-01 17:07:00 +0200 |
|---|---|---|
| committer |   Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2020-07-03 10:33:07 +0200 |
| commit | 9b8a9e6c2cb52374fed526e7db1961069fcd55ad (patch) | |
| tree | 79ec23063f5352b476f07d81dfb99032f80dd52a /drivers/staging/wfx/bh.c | |
| parent | staging: wfx: load the firmware faster (diff) | |
| download | wireguard-linux-9b8a9e6c2cb52374fed526e7db1961069fcd55ad.tar.xz wireguard-linux-9b8a9e6c2cb52374fed526e7db1961069fcd55ad.zip | |
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: Jérôme Pouiller <jerome.pouiller@silabs.com>
Link: https://lore.kernel.org/r/20200701150707.222985-7-Jerome.Pouiller@silabs.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers/staging/wfx/bh.c')
| -rw-r--r-- | drivers/staging/wfx/bh.c | 34 |
1 files changed, 20 insertions, 14 deletions
diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c index 1cbaf8bb4fa3..53ae0b5abcdd 100644 --- a/drivers/staging/wfx/bh.c +++ b/drivers/staging/wfx/bh.c @@ -57,7 +57,6 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf) int release_count; int piggyback = 0; - WARN(read_len < 4, "corrupted read"); WARN(read_len > round_down(0xFFF, 2) * sizeof(u16), "%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) hif = (struct hif_msg *)skb->data; WARN(hif->encrypted & 0x1, "unsupported encryption type"); if (hif->encrypted == 0x2) { - if (wfx_sl_decode(wdev, (void *)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; - } - computed_len = - round_up(le16_to_cpu(hif->len) - sizeof(hif->len), 16) + - sizeof(struct hif_sl_msg) + - sizeof(struct hif_sl_tag); + if (WARN(read_len < sizeof(struct hif_sl_msg), "corrupted read")) + goto err; + computed_len = le16_to_cpu(((struct hif_sl_msg *)hif)->len); + computed_len = round_up(computed_len - sizeof(u16), 16); + computed_len += sizeof(struct hif_sl_msg); + computed_len += sizeof(struct hif_sl_tag); } 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) { 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) hif, read_len, true); 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)) { (*is_cnf)++; |