| Message ID | 20200302000208.18260-1-hauke@hauke-m.de |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2,1/2] ag71xx: Handle allocation errors in ag71xx_rings_init() | expand |
Hi Hauke, Am 02.03.20 um 01:02 schrieb Hauke Mehrtens: > Free the allocated resources in ag71xx_rings_init() in case > ag71xx_ring_rx_init() returns an error. > > This is only a potential problem, I did not ran into this one. Thank you for spotting it! > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver") > --- > > v2: > * rebased onm top of "net: ag71xx: port to phylink" > > drivers/net/ethernet/atheros/ag71xx.c | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c > index 02b7705393ca..38683224b70b 100644 > --- a/drivers/net/ethernet/atheros/ag71xx.c > +++ b/drivers/net/ethernet/atheros/ag71xx.c > @@ -1169,6 +1169,7 @@ static int ag71xx_rings_init(struct ag71xx *ag) > struct ag71xx_ring *tx = &ag->tx_ring; > struct ag71xx_ring *rx = &ag->rx_ring; > int ring_size, tx_size; > + int ret; > > ring_size = BIT(tx->order) + BIT(rx->order); > tx_size = BIT(tx->order); > @@ -1181,9 +1182,8 @@ static int ag71xx_rings_init(struct ag71xx *ag) > ring_size * AG71XX_DESC_SIZE, > &tx->descs_dma, GFP_KERNEL); > if (!tx->descs_cpu) { > - kfree(tx->buf); > - tx->buf = NULL; > - return -ENOMEM; > + ret = -ENOMEM; > + goto err_free_buf; > } > > rx->buf = &tx->buf[tx_size]; > @@ -1191,7 +1191,21 @@ static int ag71xx_rings_init(struct ag71xx *ag) > rx->descs_dma = tx->descs_dma + tx_size * AG71XX_DESC_SIZE; > > ag71xx_ring_tx_init(ag); > - return ag71xx_ring_rx_init(ag); > + ret = ag71xx_ring_rx_init(ag); > + if (ret) > + goto err_free_dma; > + > + return 0; > + > +err_free_dma: > + dma_free_coherent(&ag->pdev->dev, ring_size * AG71XX_DESC_SIZE, > + tx->descs_cpu, tx->descs_dma); > + rx->buf = NULL; > +err_free_buf: > + kfree(tx->buf); > + tx->buf = NULL; > + > + return ret; > } Hm... i think it will be better to reuse ag71xx_rings_free() at this place. > static void ag71xx_rings_free(struct ag71xx *ag) > -- Regards, Oleksij
diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c index 02b7705393ca..38683224b70b 100644 --- a/drivers/net/ethernet/atheros/ag71xx.c +++ b/drivers/net/ethernet/atheros/ag71xx.c @@ -1169,6 +1169,7 @@ static int ag71xx_rings_init(struct ag71xx *ag) struct ag71xx_ring *tx = &ag->tx_ring; struct ag71xx_ring *rx = &ag->rx_ring; int ring_size, tx_size; + int ret; ring_size = BIT(tx->order) + BIT(rx->order); tx_size = BIT(tx->order); @@ -1181,9 +1182,8 @@ static int ag71xx_rings_init(struct ag71xx *ag) ring_size * AG71XX_DESC_SIZE, &tx->descs_dma, GFP_KERNEL); if (!tx->descs_cpu) { - kfree(tx->buf); - tx->buf = NULL; - return -ENOMEM; + ret = -ENOMEM; + goto err_free_buf; } rx->buf = &tx->buf[tx_size]; @@ -1191,7 +1191,21 @@ static int ag71xx_rings_init(struct ag71xx *ag) rx->descs_dma = tx->descs_dma + tx_size * AG71XX_DESC_SIZE; ag71xx_ring_tx_init(ag); - return ag71xx_ring_rx_init(ag); + ret = ag71xx_ring_rx_init(ag); + if (ret) + goto err_free_dma; + + return 0; + +err_free_dma: + dma_free_coherent(&ag->pdev->dev, ring_size * AG71XX_DESC_SIZE, + tx->descs_cpu, tx->descs_dma); + rx->buf = NULL; +err_free_buf: + kfree(tx->buf); + tx->buf = NULL; + + return ret; } static void ag71xx_rings_free(struct ag71xx *ag)
Free the allocated resources in ag71xx_rings_init() in case ag71xx_ring_rx_init() returns an error. This is only a potential problem, I did not ran into this one. Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver") --- v2: * rebased onm top of "net: ag71xx: port to phylink" drivers/net/ethernet/atheros/ag71xx.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)