mm81x_yaps_read_pkt() reads struct mm81x_skb_hdr from the start of an skb whose length comes from the YAPS delimiter, without checking that the skb holds the header. The length the header claims, sizeof(*hdr) + hdr->offset + hdr->len, is not checked against skb->len either: mm81x_skbq_validate_checksum() sizes its walk of the skb from hdr->len, skb_trim() cannot grow the skb to the claimed length, and mm81x_skbq_dispatch_work() then pulls sizeof(*hdr) + hdr->offset from an skb that may be shorter than that. Validate skb->len against sizeof(*hdr) before the header is read, and the header-claimed length against skb->len right after the sync check, before the checksum is validated and the packet is trimmed and queued. Assisted-by: LLM Fixes: b1906cea00b0 ("wifi: mm81x: add mm81x Wi-Fi HaLow driver") Signed-off-by: Aamir Ahmed --- v2: - check the header-claimed length right after the sync check, before mm81x_skbq_validate_checksum() walks the skb; v1 checked it after that call (Sashiko) - move the existing skb_len computation up to the new check - return -EIO like the sync and channel checks instead of -EINVAL, and word the errors like the existing ones - commit message rewritten - Assisted-by: LLM added, before Fixes:, and wireless-next named in the subject (Lachlan, on the n_channels patch) - no Cc: stable, the driver is only in v7.3-rc; recipients from get_maintainer v1: https://lore.kernel.org/linux-wireless/AS8P251MB0001B650C31E9F37C3B7907BC8B32@AS8P251MB0001.EURP251.PROD.OUTLOOK.COM/ The other items Sashiko raised on v1 - mm81x_skbq_validate_checksum() reading frame_control past the header of a header-only data packet and ignoring hdr->offset, and the skb leak on the -ENOMEM path of mm81x_yaps_hw_read_pkts() - are left for separate patches. The command response length is validated by "wifi: mm81x: validate response length in mm81x_cmd_resp_process()", sent alongside. Compile-tested only (W=1 and sparse on the base-commit below); I have no mm81x hardware. drivers/net/wireless/morsemicro/mm81x/yaps.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/morsemicro/mm81x/yaps.c b/drivers/net/wireless/morsemicro/mm81x/yaps.c index e98a2a587..ba03bd9b3 100644 --- a/drivers/net/wireless/morsemicro/mm81x/yaps.c +++ b/drivers/net/wireless/morsemicro/mm81x/yaps.c @@ -147,6 +147,13 @@ static int mm81x_yaps_read_pkt(struct mm81x_yaps *yaps, struct sk_buff *skb) __skb_queue_head_init(&skbq); + if (skb->len < sizeof(*hdr)) { + dev_err(mors->dev, "packet length error [%u < %zu]", skb->len, + sizeof(*hdr)); + ret = -EIO; + goto exit_return_page; + } + hdr = (struct mm81x_skb_hdr *)skb->data; if (hdr->sync != MM81X_SKB_HEADER_SYNC) { dev_err(mors->dev, "sync value error [0xAA:%d], hdr.len %d", @@ -155,6 +162,14 @@ static int mm81x_yaps_read_pkt(struct mm81x_yaps *yaps, struct sk_buff *skb) goto exit_return_page; } + skb_len = sizeof(*hdr) + hdr->offset + le16_to_cpu(hdr->len); + if (skb_len > skb->len) { + dev_err(mors->dev, "hdr length error [%d > %u]", skb_len, + skb->len); + ret = -EIO; + goto exit_return_page; + } + if (yaps->mors->hif.validate_skb_checksum && !mm81x_skbq_validate_checksum(skb->data)) { dev_dbg(yaps->mors->dev, @@ -185,7 +200,6 @@ static int mm81x_yaps_read_pkt(struct mm81x_yaps *yaps, struct sk_buff *skb) goto exit_return_page; } - skb_len = sizeof(*hdr) + hdr->offset + le16_to_cpu(hdr->len); skb_bytes_remaining = mm81x_skbq_space(mq); if (skb_len > skb_bytes_remaining) { base-commit: 13e51269b6656768dc595ed6025e4974f2026543 -- 2.43.0