| Message ID | 20241101103416.1064930-3-srasheed@marvell.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Double free fixes and NULL pointer checks | expand |
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; ...
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;
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(-)