Skip to content

Commit c666fa6

Browse files
WeiFang-NXPkuba-moo
authored andcommitted
net: enetc: fix TOCTOU race and validate VF MAC address
Sashiko reported that the PF driver accepts arbitrary MAC address from from VF mailbox messages without proper validation, creating a security vulnerability [1]. In enetc_msg_pf_set_vf_primary_mac_addr(), the MAC address is extracted directly from the message buffer (cmd->mac.sa_data) and programmed into hardware via pf->ops->set_si_primary_mac() without any validity checks. A malicious VF can configure a multicast, broadcast, or all-zero MAC address. Therefore, a validation to check the MAC address provided by VF is required. However, simply checking the MAC address is not enough, because it also has the potential TOCTOU race [2]: The code reads the MAC address from the DMA buffer to validate it via is_valid_ether_addr(), if validation passes, reads the same DMA buffer a second time when calling enetc_pf_set_primary_mac_addr() to program the hardware. A malicious VF can exploit this window by overwriting the MAC address in the DMA buffer between the validation check and the hardware programming, bypassing the validation entirely. Therefore, allocate a local buffer in enetc_msg_handle_rxmsg() and copy the message content from the DMA buffer via memcpy() before processing. This ensures the PF operates on a stable snapshot that the VF cannot modify. Link: https://sashiko.dev/#/patchset/20260511080805.2052495-1-wei.fang%40nxp.com #1 Link: https://sashiko.dev/#/patchset/20260513103021.2190593-1-wei.fang%40nxp.com #2 Fixes: beb74ac ("enetc: Add vf to pf messaging support") Signed-off-by: Wei Fang <wei.fang@nxp.com> Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com> Link: https://patch.msgid.link/20260520064421.91569-5-wei.fang@nxp.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent 4a995d3 commit c666fa6

1 file changed

Lines changed: 30 additions & 9 deletions

File tree

drivers/net/ethernet/freescale/enetc/enetc_pf.c

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -478,21 +478,24 @@ static void enetc_configure_port(struct enetc_pf *pf)
478478

479479
/* Messaging */
480480
static u16 enetc_msg_pf_set_vf_primary_mac_addr(struct enetc_pf *pf,
481-
int vf_id)
481+
int vf_id, void *msg)
482482
{
483483
struct enetc_vf_state *vf_state = &pf->vf_state[vf_id];
484-
struct enetc_msg_swbd *msg = &pf->rxmsg[vf_id];
485-
struct enetc_msg_cmd_set_primary_mac *cmd;
484+
struct enetc_msg_cmd_set_primary_mac *cmd = msg;
486485
struct device *dev = &pf->si->pdev->dev;
487-
u16 cmd_id;
486+
u16 cmd_id = cmd->header.id;
488487
char *addr;
489488

490-
cmd = (struct enetc_msg_cmd_set_primary_mac *)msg->vaddr;
491-
cmd_id = cmd->header.id;
492489
if (cmd_id != ENETC_MSG_CMD_MNG_ADD)
493490
return ENETC_MSG_CMD_STATUS_FAIL;
494491

495492
addr = cmd->mac.sa_data;
493+
if (!is_valid_ether_addr(addr)) {
494+
dev_err_ratelimited(dev, "VF%d attempted to set invalid MAC\n",
495+
vf_id);
496+
return ENETC_MSG_CMD_STATUS_FAIL;
497+
}
498+
496499
if (vf_state->flags & ENETC_VF_FLAG_PF_SET_MAC) {
497500
dev_err_ratelimited(dev,
498501
"VF%d attempted to override PF set MAC\n",
@@ -507,24 +510,42 @@ static u16 enetc_msg_pf_set_vf_primary_mac_addr(struct enetc_pf *pf,
507510

508511
void enetc_msg_handle_rxmsg(struct enetc_pf *pf, int vf_id, u16 *status)
509512
{
510-
struct enetc_msg_swbd *msg = &pf->rxmsg[vf_id];
513+
struct enetc_msg_swbd *msg_swbd = &pf->rxmsg[vf_id];
511514
struct device *dev = &pf->si->pdev->dev;
512515
struct enetc_msg_cmd_header *cmd_hdr;
513516
u16 cmd_type;
517+
u8 *msg;
514518

515-
cmd_hdr = (struct enetc_msg_cmd_header *)msg->vaddr;
519+
msg = kzalloc_objs(*msg, msg_swbd->size);
520+
if (!msg) {
521+
dev_err_ratelimited(dev,
522+
"Failed to allocate message buffer\n");
523+
*status = ENETC_MSG_CMD_STATUS_FAIL;
524+
return;
525+
}
526+
527+
/* Currently, only ENETC_MSG_CMD_MNG_MAC command is supported, so
528+
* only sizeof(struct enetc_msg_cmd_set_primary_mac) bytes need to
529+
* be copied. This data already includes the cmd_type field, so it
530+
* can correctly return an error code.
531+
*/
532+
memcpy(msg, msg_swbd->vaddr,
533+
sizeof(struct enetc_msg_cmd_set_primary_mac));
534+
cmd_hdr = (struct enetc_msg_cmd_header *)msg;
516535
cmd_type = cmd_hdr->type;
517536

518537
switch (cmd_type) {
519538
case ENETC_MSG_CMD_MNG_MAC:
520-
*status = enetc_msg_pf_set_vf_primary_mac_addr(pf, vf_id);
539+
*status = enetc_msg_pf_set_vf_primary_mac_addr(pf, vf_id, msg);
521540
break;
522541
default:
523542
*status = ENETC_MSG_CMD_STATUS_FAIL;
524543
dev_err_ratelimited(dev,
525544
"command not supported (cmd_type: 0x%x)\n",
526545
cmd_type);
527546
}
547+
548+
kfree(msg);
528549
}
529550

530551
#ifdef CONFIG_PCI_IOV

0 commit comments

Comments
 (0)