Hi,
static analysis with Coverity has detected a out of range write issue on
the assigment rx_ring->buf[i] = skb in rtw_pci_init_rx_ring. The commit
in question is:
commit a5697a65ecd109ce7f8e3661b89a5dffae73b512
Author: Yan-Hsuan Chuang <yhchu...@realtek.com>
Date: Thu Mar 12 16:08:51 2020 +0800
rtw88: pci: define a mask for TX/RX BD indexes
Analysis is as follows:
2. Condition len > 4095UL /* (int)sizeof (struct
rtw_pci_init_rx_ring::[unnamed type]) + (~0UL - (1UL << 0) + 1 & (~0UL
>> 64 - 1 - 11)) */, taking false branch.
3. cond_at_most: Checking len > 4095UL implies that len may be up to
4095 on the false branch.
267 if (len > TRX_BD_IDX_MASK) {
268 rtw_err(rtwdev, "len %d exceeds maximum RX
entries\n", len);
269 return -EINVAL;
270 }
271
272 head = pci_zalloc_consistent(pdev, ring_sz, &dma);
4. Condition !head, taking false branch.
273 if (!head) {
274 rtw_err(rtwdev, "failed to allocate rx ring\n");
275 return -ENOMEM;
276 }
277 rx_ring->r.head = head;
278
5. Condition i < len, taking true branch.
9. Condition i < len, taking true branch.
10. cond_at_most: Checking i < len implies that i may be up to 4094
on the true branch.
279 for (i = 0; i < len; i++) {
280 skb = dev_alloc_skb(buf_sz);
6. Condition !skb, taking false branch.
11. Condition !skb, taking false branch.
281 if (!skb) {
282 allocated = i;
283 ret = -ENOMEM;
284 goto err_out;
285 }
286
287 memset(skb->data, 0, buf_sz);
CID (#1 of 1): Out-of-bounds write (OVERRUN)
12. overrun-local: Overrunning array rx_ring->buf of 512 8-byte
elements at element index 4094 (byte offset 32759) using index i (which
evaluates to 4094).
288 rx_ring->buf[i] = skb;
The rx_ring->buf array is declared in
/drivers/net/wireless/realtek/rtw88/pci.h as:
struct sk_buff *buf[RTK_MAX_RX_DESC_NUM];
where RTK_MAX_RX_DESC_NUM is 512.
There are two places where the length check is incorrect:
rtw_pci_init_tx_ring():
if (len > TRX_BD_IDX_MASK) {
rtw_err(rtwdev, "len %d exceeds maximum TX entries\n", len);
return -EINVAL;
}
and rtw_pci_init_rx_ring():
if (len > TRX_BD_IDX_MASK) {
rtw_err(rtwdev, "len %d exceeds maximum RX entries\n", len);
return -EINVAL;
}
The length check should not be against TRX_BD_IDX_MASK but against the
number of elements in rx_ring->buf[]. Also the rx_ring->buf[] is only
512 items. Not sure if one or both of those are errors.
Colin
