Skip to content

Commit c33f944

Browse files
committed
Merge branch 'net-enetc-sr-iov-robustness-and-security-fixes'
Wei Fang says: ==================== net: enetc: SR-IOV robustness and security fixes This patch series addresses a number of robustness, security, and correctness issues in the ENETC driver's SR-IOV subsystem, focusing primarily on the VF-to-PF mailbox communication path. The series can be grouped into the following categories: 1. DoS and security fixes: - Prevent an unbounded loop DoS in the VF-to-PF message handler, which could be triggered by a malicious or misbehaving VF. - Fix a TOCTOU (Time-of-Check-Time-of-Use) race and add proper validation of VF MAC addresses to prevent spoofing or invalid configuration from being applied. 2. Race condition fixes: - Fix a race condition in VF MAC address configuration that could lead to inconsistent state between the VF request and PF application. - Fix a race condition during SR-IOV teardown that could cause VF->PF mailbox operations to time out, resulting in unnecessary errors during shutdown. 3. Memory safety fixes: - Fix a DMA write to freed memory in enetc_msg_free_mbx(), which could cause silent memory corruption or system instability. 4. Error handling and initialization fixes: - Fix missing error code propagation when pf->vf_state allocation fails, ensuring callers receive a proper errno instead of succeeding silently. - Fix incorrect mailbox message status values returned to VFs, which could cause VFs to misinterpret PF responses. - Fix initialization order to prevent the use of uninitialized resources during driver probe, which could cause undefined behavior on certain configurations. 5. Diagnostics improvement: - Add rate limiting to VF mailbox error messages to prevent log flooding in the presence of a misbehaving VF. These fixes improve the overall stability and security of the ENETC SR-IOV implementation, particularly in multi-tenant environments where VFs may be assigned to untrusted guests. ==================== Link: https://patch.msgid.link/20260520064421.91569-1-wei.fang@nxp.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents bdd3957 + 9e68817 commit c33f944

4 files changed

Lines changed: 127 additions & 70 deletions

File tree

drivers/net/ethernet/freescale/enetc/enetc_hw.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,21 @@ static inline u32 enetc_vsi_set_msize(u32 size)
5656
}
5757

5858
#define ENETC_PSIMSGRR 0x204
59-
#define ENETC_PSIMSGRR_MR_MASK GENMASK(2, 1)
60-
#define ENETC_PSIMSGRR_MR(n) BIT((n) + 1) /* n = VSI index */
6159
#define ENETC_PSIVMSGRCVAR0(n) (0x210 + (n) * 0x8) /* n = VSI index */
6260
#define ENETC_PSIVMSGRCVAR1(n) (0x214 + (n) * 0x8)
6361

62+
/* Message received mask, n is the active number of VSIs.
63+
* It is available for ENETC_PSIMSGRR, ENETC_PSIIER, and
64+
* ENETC_PSIIDR registers.
65+
*/
66+
#define ENETC_PSIMR_MASK(n) \
67+
({ typeof(n) _n = (n); (_n) ? GENMASK((_n), 1) : 0; })
68+
69+
/* Message received bit, n is VSI index. It is available for
70+
* ENETC_PSIMSGRR, ENETC_PSIIER, and ENETC_PSIIDR registers.
71+
*/
72+
#define ENETC_PSIMR_BIT(n) BIT((n) + 1)
73+
6474
#define ENETC_VSIMSGSR 0x204 /* RO */
6575
#define ENETC_VSIMSGSR_MB BIT(0)
6676
#define ENETC_VSIMSGSR_MS BIT(1)
@@ -94,7 +104,6 @@ static inline u32 enetc_vsi_set_msize(u32 size)
94104
#define ENETC_SICAPR1 0x904
95105

96106
#define ENETC_PSIIER 0xa00
97-
#define ENETC_PSIIER_MR_MASK GENMASK(2, 1)
98107
#define ENETC_PSIIDR 0xa08
99108
#define ENETC_SITXIDR 0xa18
100109
#define ENETC_SIRXIDR 0xa28

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

Lines changed: 58 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3,26 +3,33 @@
33

44
#include "enetc_pf.h"
55

6-
static void enetc_msg_disable_mr_int(struct enetc_hw *hw)
6+
static void enetc_msg_disable_mr_int(struct enetc_pf *pf)
77
{
8-
u32 psiier = enetc_rd(hw, ENETC_PSIIER);
8+
struct enetc_hw *hw = &pf->si->hw;
9+
u32 psiier;
10+
11+
psiier = enetc_rd(hw, ENETC_PSIIER) & ~ENETC_PSIMR_MASK(pf->num_vfs);
12+
913
/* disable MR int source(s) */
10-
enetc_wr(hw, ENETC_PSIIER, psiier & ~ENETC_PSIIER_MR_MASK);
14+
enetc_wr(hw, ENETC_PSIIER, psiier);
1115
}
1216

13-
static void enetc_msg_enable_mr_int(struct enetc_hw *hw)
17+
static void enetc_msg_enable_mr_int(struct enetc_pf *pf)
1418
{
15-
u32 psiier = enetc_rd(hw, ENETC_PSIIER);
19+
struct enetc_hw *hw = &pf->si->hw;
20+
u32 psiier;
21+
22+
psiier = enetc_rd(hw, ENETC_PSIIER) | ENETC_PSIMR_MASK(pf->num_vfs);
1623

17-
enetc_wr(hw, ENETC_PSIIER, psiier | ENETC_PSIIER_MR_MASK);
24+
enetc_wr(hw, ENETC_PSIIER, psiier);
1825
}
1926

2027
static irqreturn_t enetc_msg_psi_msix(int irq, void *data)
2128
{
2229
struct enetc_si *si = (struct enetc_si *)data;
2330
struct enetc_pf *pf = enetc_si_priv(si);
2431

25-
enetc_msg_disable_mr_int(&si->hw);
32+
enetc_msg_disable_mr_int(pf);
2633
schedule_work(&pf->msg_task);
2734

2835
return IRQ_HANDLED;
@@ -31,33 +38,35 @@ static irqreturn_t enetc_msg_psi_msix(int irq, void *data)
3138
static void enetc_msg_task(struct work_struct *work)
3239
{
3340
struct enetc_pf *pf = container_of(work, struct enetc_pf, msg_task);
41+
u32 mr_mask = ENETC_PSIMR_MASK(pf->num_vfs);
3442
struct enetc_hw *hw = &pf->si->hw;
35-
unsigned long mr_mask;
43+
u32 mr_status;
3644
int i;
3745

38-
for (;;) {
39-
mr_mask = enetc_rd(hw, ENETC_PSIMSGRR) & ENETC_PSIMSGRR_MR_MASK;
40-
if (!mr_mask) {
41-
/* re-arm MR interrupts, w1c the IDR reg */
42-
enetc_wr(hw, ENETC_PSIIDR, ENETC_PSIIER_MR_MASK);
43-
enetc_msg_enable_mr_int(hw);
44-
return;
45-
}
46+
mr_status = (enetc_rd(hw, ENETC_PSIMSGRR) & mr_mask) |
47+
(enetc_rd(hw, ENETC_PSIIDR) & mr_mask);
48+
if (!mr_status)
49+
goto out;
4650

47-
for (i = 0; i < pf->num_vfs; i++) {
48-
u32 psimsgrr;
49-
u16 msg_code;
51+
for (i = 0; i < pf->num_vfs; i++) {
52+
u32 psimsgrr;
53+
u16 msg_code;
54+
55+
if (!(ENETC_PSIMR_BIT(i) & mr_status))
56+
continue;
5057

51-
if (!(ENETC_PSIMSGRR_MR(i) & mr_mask))
52-
continue;
58+
enetc_msg_handle_rxmsg(pf, i, &msg_code);
5359

54-
enetc_msg_handle_rxmsg(pf, i, &msg_code);
60+
/* w1c to clear the corresponding VF MR bit */
61+
enetc_wr(hw, ENETC_PSIIDR, ENETC_PSIMR_BIT(i));
5562

56-
psimsgrr = ENETC_SIMSGSR_SET_MC(msg_code);
57-
psimsgrr |= ENETC_PSIMSGRR_MR(i); /* w1c */
58-
enetc_wr(hw, ENETC_PSIMSGRR, psimsgrr);
59-
}
63+
psimsgrr = ENETC_SIMSGSR_SET_MC(msg_code);
64+
psimsgrr |= ENETC_PSIMR_BIT(i); /* w1c */
65+
enetc_wr(hw, ENETC_PSIMSGRR, psimsgrr);
6066
}
67+
68+
out:
69+
enetc_msg_enable_mr_int(pf);
6170
}
6271

6372
/* Init */
@@ -96,19 +105,28 @@ static void enetc_msg_free_mbx(struct enetc_si *si, int idx)
96105
struct enetc_hw *hw = &si->hw;
97106
struct enetc_msg_swbd *msg;
98107

108+
enetc_wr(hw, ENETC_PSIVMSGRCVAR0(idx), 0);
109+
enetc_wr(hw, ENETC_PSIVMSGRCVAR1(idx), 0);
110+
99111
msg = &pf->rxmsg[idx];
100112
dma_free_coherent(&si->pdev->dev, msg->size, msg->vaddr, msg->dma);
101113
memset(msg, 0, sizeof(*msg));
102-
103-
enetc_wr(hw, ENETC_PSIVMSGRCVAR0(idx), 0);
104-
enetc_wr(hw, ENETC_PSIVMSGRCVAR1(idx), 0);
105114
}
106115

107116
int enetc_msg_psi_init(struct enetc_pf *pf)
108117
{
109118
struct enetc_si *si = pf->si;
110119
int vector, i, err;
111120

121+
for (i = 0; i < pf->num_vfs; i++) {
122+
err = enetc_msg_alloc_mbx(si, i);
123+
if (err)
124+
goto free_mbx;
125+
}
126+
127+
/* initialize PSI mailbox */
128+
INIT_WORK(&pf->msg_task, enetc_msg_task);
129+
112130
/* register message passing interrupt handler */
113131
snprintf(pf->msg_int_name, sizeof(pf->msg_int_name), "%s-vfmsg",
114132
si->ndev->name);
@@ -117,32 +135,21 @@ int enetc_msg_psi_init(struct enetc_pf *pf)
117135
if (err) {
118136
dev_err(&si->pdev->dev,
119137
"PSI messaging: request_irq() failed!\n");
120-
return err;
138+
goto free_mbx;
121139
}
122140

123141
/* set one IRQ entry for PSI message receive notification (SI int) */
124142
enetc_wr(&si->hw, ENETC_SIMSIVR, ENETC_SI_INT_IDX);
125143

126-
/* initialize PSI mailbox */
127-
INIT_WORK(&pf->msg_task, enetc_msg_task);
128-
129-
for (i = 0; i < pf->num_vfs; i++) {
130-
err = enetc_msg_alloc_mbx(si, i);
131-
if (err)
132-
goto err_init_mbx;
133-
}
134-
135144
/* enable MR interrupts */
136-
enetc_msg_enable_mr_int(&si->hw);
145+
enetc_msg_enable_mr_int(pf);
137146

138147
return 0;
139148

140-
err_init_mbx:
149+
free_mbx:
141150
for (i--; i >= 0; i--)
142151
enetc_msg_free_mbx(si, i);
143152

144-
free_irq(vector, si);
145-
146153
return err;
147154
}
148155

@@ -151,14 +158,17 @@ void enetc_msg_psi_free(struct enetc_pf *pf)
151158
struct enetc_si *si = pf->si;
152159
int i;
153160

161+
/* disable MR interrupts */
162+
enetc_msg_disable_mr_int(pf);
163+
164+
/* de-register message passing interrupt handler */
165+
free_irq(pci_irq_vector(si->pdev, ENETC_SI_INT_IDX), si);
166+
154167
cancel_work_sync(&pf->msg_task);
155168

156-
/* disable MR interrupts */
157-
enetc_msg_disable_mr_int(&si->hw);
169+
/* MR interrupts may be re-enabled by workqueue */
170+
enetc_msg_disable_mr_int(pf);
158171

159172
for (i = 0; i < pf->num_vfs; i++)
160173
enetc_msg_free_mbx(si, i);
161-
162-
/* de-register message passing interrupt handler */
163-
free_irq(pci_irq_vector(si->pdev, ENETC_SI_INT_IDX), si);
164174
}

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

Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,12 @@ static int enetc_pf_set_vf_mac(struct net_device *ndev, int vf, u8 *mac)
252252
return -EADDRNOTAVAIL;
253253

254254
vf_state = &pf->vf_state[vf];
255+
256+
mutex_lock(&vf_state->lock);
255257
vf_state->flags |= ENETC_VF_FLAG_PF_SET_MAC;
256258
enetc_pf_set_primary_mac_addr(&priv->si->hw, vf + 1, mac);
259+
mutex_unlock(&vf_state->lock);
260+
257261
return 0;
258262
}
259263

@@ -478,49 +482,77 @@ static void enetc_configure_port(struct enetc_pf *pf)
478482

479483
/* Messaging */
480484
static u16 enetc_msg_pf_set_vf_primary_mac_addr(struct enetc_pf *pf,
481-
int vf_id)
485+
int vf_id, void *msg)
482486
{
483487
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;
488+
struct enetc_msg_cmd_set_primary_mac *cmd = msg;
486489
struct device *dev = &pf->si->pdev->dev;
487-
u16 cmd_id;
490+
u16 cmd_id = cmd->header.id;
488491
char *addr;
489492

490-
cmd = (struct enetc_msg_cmd_set_primary_mac *)msg->vaddr;
491-
cmd_id = cmd->header.id;
492493
if (cmd_id != ENETC_MSG_CMD_MNG_ADD)
493494
return ENETC_MSG_CMD_STATUS_FAIL;
494495

495496
addr = cmd->mac.sa_data;
496-
if (vf_state->flags & ENETC_VF_FLAG_PF_SET_MAC)
497-
dev_warn(dev, "Attempt to override PF set mac addr for VF%d\n",
498-
vf_id);
499-
else
500-
enetc_pf_set_primary_mac_addr(&pf->si->hw, vf_id + 1, addr);
497+
if (!is_valid_ether_addr(addr)) {
498+
dev_err_ratelimited(dev, "VF%d attempted to set invalid MAC\n",
499+
vf_id);
500+
return ENETC_MSG_CMD_STATUS_FAIL;
501+
}
502+
503+
mutex_lock(&vf_state->lock);
504+
if (vf_state->flags & ENETC_VF_FLAG_PF_SET_MAC) {
505+
mutex_unlock(&vf_state->lock);
506+
dev_err_ratelimited(dev,
507+
"VF%d attempted to override PF set MAC\n",
508+
vf_id);
509+
return ENETC_MSG_CMD_STATUS_FAIL;
510+
}
511+
512+
enetc_pf_set_primary_mac_addr(&pf->si->hw, vf_id + 1, addr);
513+
mutex_unlock(&vf_state->lock);
501514

502515
return ENETC_MSG_CMD_STATUS_OK;
503516
}
504517

505518
void enetc_msg_handle_rxmsg(struct enetc_pf *pf, int vf_id, u16 *status)
506519
{
507-
struct enetc_msg_swbd *msg = &pf->rxmsg[vf_id];
520+
struct enetc_msg_swbd *msg_swbd = &pf->rxmsg[vf_id];
508521
struct device *dev = &pf->si->pdev->dev;
509522
struct enetc_msg_cmd_header *cmd_hdr;
510523
u16 cmd_type;
524+
u8 *msg;
511525

512-
*status = ENETC_MSG_CMD_STATUS_OK;
513-
cmd_hdr = (struct enetc_msg_cmd_header *)msg->vaddr;
526+
msg = kzalloc_objs(*msg, msg_swbd->size);
527+
if (!msg) {
528+
dev_err_ratelimited(dev,
529+
"Failed to allocate message buffer\n");
530+
*status = ENETC_MSG_CMD_STATUS_FAIL;
531+
return;
532+
}
533+
534+
/* Currently, only ENETC_MSG_CMD_MNG_MAC command is supported, so
535+
* only sizeof(struct enetc_msg_cmd_set_primary_mac) bytes need to
536+
* be copied. This data already includes the cmd_type field, so it
537+
* can correctly return an error code.
538+
*/
539+
memcpy(msg, msg_swbd->vaddr,
540+
sizeof(struct enetc_msg_cmd_set_primary_mac));
541+
cmd_hdr = (struct enetc_msg_cmd_header *)msg;
514542
cmd_type = cmd_hdr->type;
515543

516544
switch (cmd_type) {
517545
case ENETC_MSG_CMD_MNG_MAC:
518-
*status = enetc_msg_pf_set_vf_primary_mac_addr(pf, vf_id);
546+
*status = enetc_msg_pf_set_vf_primary_mac_addr(pf, vf_id, msg);
519547
break;
520548
default:
521-
dev_err(dev, "command not supported (cmd_type: 0x%x)\n",
522-
cmd_type);
549+
*status = ENETC_MSG_CMD_STATUS_FAIL;
550+
dev_err_ratelimited(dev,
551+
"command not supported (cmd_type: 0x%x)\n",
552+
cmd_type);
523553
}
554+
555+
kfree(msg);
524556
}
525557

526558
#ifdef CONFIG_PCI_IOV
@@ -531,9 +563,9 @@ static int enetc_sriov_configure(struct pci_dev *pdev, int num_vfs)
531563
int err;
532564

533565
if (!num_vfs) {
566+
pci_disable_sriov(pdev);
534567
enetc_msg_psi_free(pf);
535568
pf->num_vfs = 0;
536-
pci_disable_sriov(pdev);
537569
} else {
538570
pf->num_vfs = num_vfs;
539571

@@ -960,8 +992,13 @@ static int enetc_pf_probe(struct pci_dev *pdev,
960992
if (pf->total_vfs) {
961993
pf->vf_state = kzalloc_objs(struct enetc_vf_state,
962994
pf->total_vfs);
963-
if (!pf->vf_state)
995+
if (!pf->vf_state) {
996+
err = -ENOMEM;
964997
goto err_alloc_vf_state;
998+
}
999+
1000+
for (int i = 0; i < pf->total_vfs; i++)
1001+
mutex_init(&pf->vf_state[i].lock);
9651002
}
9661003

9671004
err = enetc_setup_mac_addresses(node, pf);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ enum enetc_vf_flags {
1414
};
1515

1616
struct enetc_vf_state {
17+
struct mutex lock; /* Prevent concurrent access */
1718
enum enetc_vf_flags flags;
1819
};
1920

0 commit comments

Comments
 (0)