| Message ID | 20220106114801.20563-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | can: rcar_canfd: Make sure we free CAN network device | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Series ignored based on subject |
Hi Kieran, Thank you for the review. On Thu, Jan 6, 2022 at 3:46 PM Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Quoting Lad Prabhakar (2022-01-06 11:48:00) > > Make sure we free CAN network device in the error path. There are several > > jumps to fail label after allocating the CAN network device successfully. > > This patch places the free_candev() under fail label so that in failure > > path a jump to fail label frees the CAN network device. > > > > Fixes: 76e9353a80e9 ("can: rcar_canfd: Add support for RZ/G2L family") > > Reported-by: Pavel Machek <pavel@denx.de> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > drivers/net/can/rcar/rcar_canfd.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c > > index ff9d0f5ae0dd..388521e70837 100644 > > --- a/drivers/net/can/rcar/rcar_canfd.c > > +++ b/drivers/net/can/rcar/rcar_canfd.c > > @@ -1640,8 +1640,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch, > > ndev = alloc_candev(sizeof(*priv), RCANFD_FIFO_DEPTH); > > if (!ndev) { > > dev_err(&pdev->dev, "alloc_candev() failed\n"); > > - err = -ENOMEM; > > - goto fail; > > + return -ENOMEM; > > Aha good - so we don't try to call free_candev() on a null pointer. > (which doesn't look null-safe, in free_netdev). > Yep. > > } > > priv = netdev_priv(ndev); > > > > @@ -1735,8 +1734,8 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch, > > > > fail_candev: > > Is this label still appropriately named now that the free_candev is > moved out of it? I wonder if it should be fail_netif: > I was tempted for this change, but wanted to keep the changes minimal. Maybe I'll do it anyway to improve the readability. > So aside from potential naming, the !ndev case is safely handled, so it > looks fine to me. > > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > netif_napi_del(&priv->napi); > > - free_candev(ndev); > > fail: > > + free_candev(ndev); > > > > > return err; > > } > > > > -- > > 2.17.1 > > Cheers, Prabhakar
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c index ff9d0f5ae0dd..388521e70837 100644 --- a/drivers/net/can/rcar/rcar_canfd.c +++ b/drivers/net/can/rcar/rcar_canfd.c @@ -1640,8 +1640,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch, ndev = alloc_candev(sizeof(*priv), RCANFD_FIFO_DEPTH); if (!ndev) { dev_err(&pdev->dev, "alloc_candev() failed\n"); - err = -ENOMEM; - goto fail; + return -ENOMEM; } priv = netdev_priv(ndev); @@ -1735,8 +1734,8 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch, fail_candev: netif_napi_del(&priv->napi); - free_candev(ndev); fail: + free_candev(ndev); return err; }
Make sure we free CAN network device in the error path. There are several jumps to fail label after allocating the CAN network device successfully. This patch places the free_candev() under fail label so that in failure path a jump to fail label frees the CAN network device. Fixes: 76e9353a80e9 ("can: rcar_canfd: Add support for RZ/G2L family") Reported-by: Pavel Machek <pavel@denx.de> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- drivers/net/can/rcar/rcar_canfd.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)