Skip to content

Commit 853d68d

Browse files
Shruti ParabNipaLocal
Shruti Parab
authored and
NipaLocal
committed
bnxt_en: Fix out-of-bound memcpy() during ethtool -w
When retrieving the FW coredump using ethtool, it can sometimes cause memory corruption: BUG: KFENCE: memory corruption in __bnxt_get_coredump+0x3ef/0x670 [bnxt_en] Corrupted memory at 0x000000008f0f30e8 [ ! ! ! ! ! ! ! ! ! ! ! ! ! ! ! ! ] (in kfence-torvalds#45): __bnxt_get_coredump+0x3ef/0x670 [bnxt_en] ethtool_get_dump_data+0xdc/0x1a0 __dev_ethtool+0xa1e/0x1af0 dev_ethtool+0xa8/0x170 dev_ioctl+0x1b5/0x580 sock_do_ioctl+0xab/0xf0 sock_ioctl+0x1ce/0x2e0 __x64_sys_ioctl+0x87/0xc0 do_syscall_64+0x5c/0xf0 entry_SYSCALL_64_after_hwframe+0x78/0x80 ... This happens when copying the coredump segment list in bnxt_hwrm_dbg_dma_data() with the HWRM_DBG_COREDUMP_LIST FW command. The info->dest_buf buffer is allocated based on the number of coredump segments returned by the FW. The segment list is then DMA'ed by the FW and the length of the DMA is returned by FW. The driver then copies this DMA'ed segment list to info->dest_buf. In some cases, this DMA length may exceed the info->dest_buf length and cause the above BUG condition. Fix it by capping the copy length to not exceed the length of info->dest_buf. The extra DMA data contains no useful information. This code path is shared for the HWRM_DBG_COREDUMP_LIST and the HWRM_DBG_COREDUMP_RETRIEVE FW commands. The buffering is different for these 2 FW commands. To simplify the logic, we need to move the line to adjust the buffer length for HWRM_DBG_COREDUMP_RETRIEVE up, so that the new check to cap the copy length will work for both commands. Fixes: c74751f ("bnxt_en: Return error if FW returns more data than dump length") Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Signed-off-by: Shruti Parab <shruti.parab@broadcom.com> Signed-off-by: Michael Chan <michael.chan@broadcom.com> Signed-off-by: NipaLocal <nipa@local>
1 parent 30ae153 commit 853d68d

File tree

1 file changed

+10
-5
lines changed

1 file changed

+10
-5
lines changed

drivers/net/ethernet/broadcom/bnxt/bnxt_coredump.c

+10-5
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,19 @@ static int bnxt_hwrm_dbg_dma_data(struct bnxt *bp, void *msg,
110110
}
111111
}
112112

113+
if (cmn_req->req_type ==
114+
cpu_to_le16(HWRM_DBG_COREDUMP_RETRIEVE))
115+
info->dest_buf_size += len;
116+
113117
if (info->dest_buf) {
114118
if ((info->seg_start + off + len) <=
115119
BNXT_COREDUMP_BUF_LEN(info->buf_len)) {
116-
memcpy(info->dest_buf + off, dma_buf, len);
120+
u16 copylen = min_t(u16, len,
121+
info->dest_buf_size - off);
122+
123+
memcpy(info->dest_buf + off, dma_buf, copylen);
124+
if (copylen < len)
125+
break;
117126
} else {
118127
rc = -ENOBUFS;
119128
if (cmn_req->req_type ==
@@ -125,10 +134,6 @@ static int bnxt_hwrm_dbg_dma_data(struct bnxt *bp, void *msg,
125134
}
126135
}
127136

128-
if (cmn_req->req_type ==
129-
cpu_to_le16(HWRM_DBG_COREDUMP_RETRIEVE))
130-
info->dest_buf_size += len;
131-
132137
if (!(cmn_resp->flags & HWRM_DBG_CMN_FLAGS_MORE))
133138
break;
134139

0 commit comments

Comments
 (0)