[PATCH] wifi: ath9k: validate RX stream lengths before copying

[PATCH] wifi: ath9k: validate RX stream lengths before copying

[Pengpeng Hou]

ath9k_hif_usb_rx_stream() reads RX stream headers and copies payload
bytes from the current skb into newly allocated skbs. It also completes
packets that span two URBs by copying the remaining bytes from the next
skb into hif_dev->remain_skb.

The parser checked the stream tag and an upper bound on pkt_len, but it
did not first prove that the fixed header, the non-fragmented payload,
or the bytes needed to complete a fragmented packet are present in the
current skb. Reject malformed RX stream data before reading or copying
beyond the received buffer.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/net/wireless/ath/ath9k/hif_usb.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 821909b8..f47b0ae0 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -571,6 +571,16 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev,
 			ptr = (u8 *) remain_skb->data;
 
 			index = rx_remain_len;
+			if (rx_remain_len < hif_dev->rx_pad_len ||
+			    len < rx_remain_len - hif_dev->rx_pad_len) {
+				dev_kfree_skb_any(remain_skb);
+				hif_dev->remain_skb = NULL;
+				hif_dev->rx_remain_len = 0;
+				RX_STAT_INC(hif_dev, skb_dropped);
+				spin_unlock(&hif_dev->rx_lock);
+				return;
+			}
+
 			rx_remain_len -= hif_dev->rx_pad_len;
 			ptr += rx_pkt_len;
 
@@ -597,6 +607,11 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev,
 
 		ptr = (u8 *) skb->data;
 
+		if (len - index < 4) {
+			RX_STAT_INC(hif_dev, skb_dropped);
+			goto invalid_pkt;
+		}
+
 		pkt_len = get_unaligned_le16(ptr + index);
 		pkt_tag = get_unaligned_le16(ptr + index + 2);
 
@@ -625,6 +640,11 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev,
 		index = index + 4 + pkt_len + pad_len;
 
 		if (index > MAX_RX_BUF_SIZE) {
+			if (len < MAX_RX_BUF_SIZE) {
+				RX_STAT_INC(hif_dev, skb_dropped);
+				goto invalid_pkt;
+			}
+
 			spin_lock(&hif_dev->rx_lock);
 			nskb = __dev_alloc_skb(pkt_len + 32, GFP_ATOMIC);
 			if (!nskb) {
@@ -649,6 +669,11 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev,
 			hif_dev->remain_skb = nskb;
 			spin_unlock(&hif_dev->rx_lock);
 		} else {
+			if (pkt_len > len - chk_idx - 4) {
+				RX_STAT_INC(hif_dev, skb_dropped);
+				goto invalid_pkt;
+			}
+
 			if (pool_index == MAX_PKT_NUM_IN_TRANSFER) {
 				dev_err(&hif_dev->udev->dev,
 					"ath9k_htc: over RX MAX_PKT_NUM\n");

Re: [PATCH] wifi: ath9k: validate RX stream lengths before copying

[Toke Høiland-Jørgensen]

Pengpeng Hou <pengpeng@iscas.ac.cn> writes:

> ath9k_hif_usb_rx_stream() reads RX stream headers and copies payload
> bytes from the current skb into newly allocated skbs. It also completes
> packets that span two URBs by copying the remaining bytes from the next
> skb into hif_dev->remain_skb.
>
> The parser checked the stream tag and an upper bound on pkt_len, but it
> did not first prove that the fixed header, the non-fragmented payload,
> or the bytes needed to complete a fragmented packet are present in the
> current skb. Reject malformed RX stream data before reading or copying
> beyond the received buffer.
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>

Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>