* [PATCH net v1 1/3] octeon_ep: Add checks to fix double free crashes.
2024-11-01 10:34 [PATCH net v1 0/3] Double free fixes and NULL pointer checks Shinas Rasheed
@ 2024-11-01 10:34 ` Shinas Rasheed
2024-11-01 10:34 ` [PATCH net v1 2/3] octeon_ep: add checks to fix NULL pointer dereferences Shinas Rasheed
2024-11-01 10:34 ` [PATCH net v1 3/3] octeon_ep_vf: " Shinas Rasheed
2 siblings, 0 replies; 5+ messages in thread
From: Shinas Rasheed @ 2024-11-01 10:34 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: hgani, sedara, vimleshk, wizhao, horms, kongyen, pabeni, kheib,
mschmidt, Shinas Rasheed, Veerasenareddy Burru, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Abhijit Ayarekar,
Satananda Burla
From: Vimlesh Kumar <vimleshk@marvell.com>
Add required checks to avoid double free. Crashes were
observed due to the same on reset scenarios
Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt support")
Signed-off-by: Vimlesh Kumar <vimleshk@marvell.com>
Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
---
.../ethernet/marvell/octeon_ep/octep_main.c | 39 +++++++++++--------
.../net/ethernet/marvell/octeon_ep/octep_tx.c | 2 +
2 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index 549436efc204..ff72b796bd25 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -154,9 +154,11 @@ static int octep_enable_msix_range(struct octep_device *oct)
*/
static void octep_disable_msix(struct octep_device *oct)
{
- pci_disable_msix(oct->pdev);
- kfree(oct->msix_entries);
- oct->msix_entries = NULL;
+ if (oct->msix_entries) {
+ pci_disable_msix(oct->pdev);
+ kfree(oct->msix_entries);
+ oct->msix_entries = NULL;
+ }
dev_info(&oct->pdev->dev, "Disabled MSI-X\n");
}
@@ -496,16 +498,18 @@ static void octep_free_irqs(struct octep_device *oct)
{
int i;
- /* First few MSI-X interrupts are non queue interrupts; free them */
- for (i = 0; i < CFG_GET_NON_IOQ_MSIX(oct->conf); i++)
- free_irq(oct->msix_entries[i].vector, oct);
- kfree(oct->non_ioq_irq_names);
-
- /* Free IRQs for Input/Output (Tx/Rx) queues */
- for (i = CFG_GET_NON_IOQ_MSIX(oct->conf); i < oct->num_irqs; i++) {
- irq_set_affinity_hint(oct->msix_entries[i].vector, NULL);
- free_irq(oct->msix_entries[i].vector,
- oct->ioq_vector[i - CFG_GET_NON_IOQ_MSIX(oct->conf)]);
+ if (oct->msix_entries) {
+ /* First few MSI-X interrupts are non queue interrupts; free them */
+ for (i = 0; i < CFG_GET_NON_IOQ_MSIX(oct->conf); i++)
+ free_irq(oct->msix_entries[i].vector, oct);
+ kfree(oct->non_ioq_irq_names);
+
+ /* Free IRQs for Input/Output (Tx/Rx) queues */
+ for (i = CFG_GET_NON_IOQ_MSIX(oct->conf); i < oct->num_irqs; i++) {
+ irq_set_affinity_hint(oct->msix_entries[i].vector, NULL);
+ free_irq(oct->msix_entries[i].vector,
+ oct->ioq_vector[i - CFG_GET_NON_IOQ_MSIX(oct->conf)]);
+ }
}
netdev_info(oct->netdev, "IRQs freed\n");
}
@@ -635,8 +639,10 @@ static void octep_napi_delete(struct octep_device *oct)
for (i = 0; i < oct->num_oqs; i++) {
netdev_dbg(oct->netdev, "Deleting NAPI on Q-%d\n", i);
- netif_napi_del(&oct->ioq_vector[i]->napi);
- oct->oq[i]->napi = NULL;
+ if (oct->oq[i]->napi) {
+ netif_napi_del(&oct->ioq_vector[i]->napi);
+ oct->oq[i]->napi = NULL;
+ }
}
}
@@ -666,7 +672,8 @@ static void octep_napi_disable(struct octep_device *oct)
for (i = 0; i < oct->num_oqs; i++) {
netdev_dbg(oct->netdev, "Disabling NAPI on Q-%d\n", i);
- napi_disable(&oct->ioq_vector[i]->napi);
+ if (oct->oq[i]->napi)
+ napi_disable(&oct->ioq_vector[i]->napi);
}
}
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c
index 06851b78aa28..157bf489ae19 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c
@@ -323,6 +323,8 @@ void octep_free_iqs(struct octep_device *oct)
int i;
for (i = 0; i < CFG_GET_PORTS_ACTIVE_IO_RINGS(oct->conf); i++) {
+ if (!oct->iq[i])
+ continue;
octep_free_iq(oct->iq[i]);
dev_dbg(&oct->pdev->dev,
"Successfully destroyed IQ(TxQ)-%d.\n", i);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH net v1 2/3] octeon_ep: add checks to fix NULL pointer dereferences
2024-11-01 10:34 [PATCH net v1 0/3] Double free fixes and NULL pointer checks Shinas Rasheed
2024-11-01 10:34 ` [PATCH net v1 1/3] octeon_ep: Add checks to fix double free crashes Shinas Rasheed
@ 2024-11-01 10:34 ` Shinas Rasheed
2024-11-05 12:25 ` Simon Horman
2024-11-01 10:34 ` [PATCH net v1 3/3] octeon_ep_vf: " Shinas Rasheed
2 siblings, 1 reply; 5+ messages in thread
From: Shinas Rasheed @ 2024-11-01 10:34 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: hgani, sedara, vimleshk, wizhao, horms, kongyen, pabeni, kheib,
mschmidt, Shinas Rasheed, Veerasenareddy Burru, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Satananda Burla,
Abhijit Ayarekar
Add Checks to avoid NULL pointer references that might
happen in rare and corner cases
Fixes: 6a610a46bad1 ("octeon_ep: add support for ndo ops")
Fixes: 1f2c2d0cee02 ("octeon_ep: add hardware configuration APIs")
Fixes: 0807dc76f3bf ("octeon_ep: support Octeon CN10K devices")
Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
---
drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c | 9 ++++++++-
drivers/net/ethernet/marvell/octeon_ep/octep_cnxk_pf.c | 9 ++++++++-
drivers/net/ethernet/marvell/octeon_ep/octep_main.c | 3 +++
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
index b5805969404f..b87336b2e4b9 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
@@ -617,7 +617,14 @@ static irqreturn_t octep_rsvd_intr_handler_cn93_pf(void *dev)
static irqreturn_t octep_ioq_intr_handler_cn93_pf(void *data)
{
struct octep_ioq_vector *vector = (struct octep_ioq_vector *)data;
- struct octep_oq *oq = vector->oq;
+ struct octep_oq *oq;
+
+ if (!vector)
+ return IRQ_HANDLED;
+ oq = vector->oq;
+
+ if (!oq || !(oq->napi))
+ return IRQ_HANDLED;
napi_schedule_irqoff(oq->napi);
return IRQ_HANDLED;
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cnxk_pf.c b/drivers/net/ethernet/marvell/octeon_ep/octep_cnxk_pf.c
index 5de0b5ecbc5f..65a8dc1d492b 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_cnxk_pf.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cnxk_pf.c
@@ -638,7 +638,14 @@ static irqreturn_t octep_rsvd_intr_handler_cnxk_pf(void *dev)
static irqreturn_t octep_ioq_intr_handler_cnxk_pf(void *data)
{
struct octep_ioq_vector *vector = (struct octep_ioq_vector *)data;
- struct octep_oq *oq = vector->oq;
+ struct octep_oq *oq;
+
+ if (!vector)
+ return IRQ_HANDLED;
+ oq = vector->oq;
+
+ if (!oq || !(oq->napi))
+ return IRQ_HANDLED;
napi_schedule_irqoff(oq->napi);
return IRQ_HANDLED;
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index ff72b796bd25..dc783c568e2c 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -1016,6 +1016,9 @@ static void octep_get_stats64(struct net_device *netdev,
struct octep_iq *iq = oct->iq[q];
struct octep_oq *oq = oct->oq[q];
+ if (!iq || !oq)
+ return;
+
tx_packets += iq->stats.instr_completed;
tx_bytes += iq->stats.bytes_sent;
rx_packets += oq->stats.packets;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net v1 2/3] octeon_ep: add checks to fix NULL pointer dereferences
2024-11-01 10:34 ` [PATCH net v1 2/3] octeon_ep: add checks to fix NULL pointer dereferences Shinas Rasheed
@ 2024-11-05 12:25 ` Simon Horman
0 siblings, 0 replies; 5+ messages in thread
From: Simon Horman @ 2024-11-05 12:25 UTC (permalink / raw)
To: Shinas Rasheed
Cc: netdev, linux-kernel, hgani, sedara, vimleshk, wizhao, kongyen,
pabeni, kheib, mschmidt, Veerasenareddy Burru, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Satananda Burla,
Abhijit Ayarekar
On Fri, Nov 01, 2024 at 03:34:14AM -0700, Shinas Rasheed wrote:
> Add Checks to avoid NULL pointer references that might
> happen in rare and corner cases
>
> Fixes: 6a610a46bad1 ("octeon_ep: add support for ndo ops")
> Fixes: 1f2c2d0cee02 ("octeon_ep: add hardware configuration APIs")
> Fixes: 0807dc76f3bf ("octeon_ep: support Octeon CN10K devices")
Hi Shinas,
As this has both three Fixes tags and three hunks, I suspect
it is fixing three separate but similar problems. And if so,
would be best split into three patches, one patch per problem.
Further, as an overall comment for the entire series, I think more
explanation of how these problems can arise is needed. Are they race
conditions, artifacts of tear-down or error handling, ... And what
execution paths lead to them? Stack traces, if available, would also be
useful to include.
> Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
> ---
> drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c | 9 ++++++++-
> drivers/net/ethernet/marvell/octeon_ep/octep_cnxk_pf.c | 9 ++++++++-
> drivers/net/ethernet/marvell/octeon_ep/octep_main.c | 3 +++
> 3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> index b5805969404f..b87336b2e4b9 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> @@ -617,7 +617,14 @@ static irqreturn_t octep_rsvd_intr_handler_cn93_pf(void *dev)
> static irqreturn_t octep_ioq_intr_handler_cn93_pf(void *data)
> {
> struct octep_ioq_vector *vector = (struct octep_ioq_vector *)data;
> - struct octep_oq *oq = vector->oq;
> + struct octep_oq *oq;
> +
> + if (!vector)
> + return IRQ_HANDLED;
> + oq = vector->oq;
> +
> + if (!oq || !(oq->napi))
nit: I don't think you need parentheses around op->napi. Likeiwse in patch 3/3.
> + return IRQ_HANDLED;
>
> napi_schedule_irqoff(oq->napi);
> return IRQ_HANDLED;
...
--
pw-bot: changes-requested
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net v1 3/3] octeon_ep_vf: add checks to fix NULL pointer dereferences
2024-11-01 10:34 [PATCH net v1 0/3] Double free fixes and NULL pointer checks Shinas Rasheed
2024-11-01 10:34 ` [PATCH net v1 1/3] octeon_ep: Add checks to fix double free crashes Shinas Rasheed
2024-11-01 10:34 ` [PATCH net v1 2/3] octeon_ep: add checks to fix NULL pointer dereferences Shinas Rasheed
@ 2024-11-01 10:34 ` Shinas Rasheed
2 siblings, 0 replies; 5+ messages in thread
From: Shinas Rasheed @ 2024-11-01 10:34 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: hgani, sedara, vimleshk, wizhao, horms, kongyen, pabeni, kheib,
mschmidt, Shinas Rasheed, Veerasenareddy Burru, Satananda Burla,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski
Add checks to avoid NULL pointer dereferences that might
happen in rare and corner cases
Fixes: cb7dd712189f ("octeon_ep_vf: Add driver framework and device initialization")
Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
---
.../ethernet/marvell/octeon_ep_vf/octep_vf_cn9k.c | 8 +++++++-
.../ethernet/marvell/octeon_ep_vf/octep_vf_cnxk.c | 12 +++++++++++-
.../ethernet/marvell/octeon_ep_vf/octep_vf_main.c | 3 +++
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_cn9k.c b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_cn9k.c
index 88937fce75f1..f1b7eda3fa42 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_cn9k.c
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_cn9k.c
@@ -273,8 +273,14 @@ static irqreturn_t octep_vf_ioq_intr_handler_cn93(void *data)
struct octep_vf_oq *oq;
u64 reg_val;
- oct = vector->octep_vf_dev;
+ if (!vector)
+ return IRQ_HANDLED;
+
oq = vector->oq;
+ if (!oq)
+ return IRQ_HANDLED;
+
+ oct = vector->octep_vf_dev;
/* Mailbox interrupt arrives along with interrupt of tx/rx ring pair 0 */
if (oq->q_no == 0) {
reg_val = octep_vf_read_csr64(oct, CN93_VF_SDP_R_MBOX_PF_VF_INT(0));
diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_cnxk.c b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_cnxk.c
index 1f79dfad42c6..31c0d7c0492a 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_cnxk.c
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_cnxk.c
@@ -284,8 +284,14 @@ static irqreturn_t octep_vf_ioq_intr_handler_cnxk(void *data)
struct octep_vf_oq *oq;
u64 reg_val;
- oct = vector->octep_vf_dev;
+ if (!vector)
+ return IRQ_HANDLED;
+
oq = vector->oq;
+ if (!oq)
+ return IRQ_HANDLED;
+
+ oct = vector->octep_vf_dev;
/* Mailbox interrupt arrives along with interrupt of tx/rx ring pair 0 */
if (oq->q_no == 0) {
reg_val = octep_vf_read_csr64(oct, CNXK_VF_SDP_R_MBOX_PF_VF_INT(0));
@@ -294,6 +300,10 @@ static irqreturn_t octep_vf_ioq_intr_handler_cnxk(void *data)
octep_vf_write_csr64(oct, CNXK_VF_SDP_R_MBOX_PF_VF_INT(0), reg_val);
}
}
+
+ if (!(oq->napi))
+ return IRQ_HANDLED;
+
napi_schedule_irqoff(oq->napi);
return IRQ_HANDLED;
}
diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c
index 7e6771c9cdbb..79d9ffd593eb 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c
@@ -790,6 +790,9 @@ static void octep_vf_get_stats64(struct net_device *netdev,
struct octep_vf_iq *iq = oct->iq[q];
struct octep_vf_oq *oq = oct->oq[q];
+ if (!iq || !oq)
+ return;
+
tx_packets += iq->stats.instr_completed;
tx_bytes += iq->stats.bytes_sent;
rx_packets += oq->stats.packets;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread