Re: [PATCH] staging: ks7010_sdio: fix NULL pointer dereference and memory leak

[Gustavo A. R. Silva]

Hi Dan,

On 04/12/2018 10:08 AM, Dan Carpenter wrote:
> I added Colin to the Cc list.
>
> On Thu, Apr 12, 2018 at 09:30:09AM -0500, Gustavo A. R. Silva wrote:
> > priv is being dereferenced when it is still null, hence there is an
> > explicit null pointer dereference at line 935: free_netdev(priv->net_dev)
> >
> > Also, memory allocated for netdev at line 854:
> > netdev = alloc_etherdev(sizeof(*priv));
> > is not being free'd, hence there is a memory leak.
> >
> > Fix this by null checking priv before dererefencing it and free netdev
> > before return.
> >
> > Addresses-Coverity-ID: 1467844 ("Explicit null dereferenced")
> > Signed-off-by: Gustavo A. R. Silva <gustavo@xxxxxxxxxxxxxx>
> > ---
> >   drivers/staging/ks7010/ks7010_sdio.c | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> > index b8f55a1..f5d4c62 100644
> > --- a/drivers/staging/ks7010/ks7010_sdio.c
> > +++ b/drivers/staging/ks7010/ks7010_sdio.c
> > @@ -932,8 +932,12 @@ static int ks7010_sdio_probe(struct sdio_func *func,
> >   	return 0;
> >   
> >    err_free_netdev:
> > -	free_netdev(priv->net_dev);
> > -	card->priv = NULL;
> > +	if (priv) {
> > +		free_netdev(priv->net_dev);
> > +		card->priv = NULL;
>
> This isn't required because the next thing we do to card is kfree(card).

I got it.

> > +	} else {
> > +		free_netdev(netdev);
> > +	}
>
>
> That's too complicated.  Just do:
>
> err_free_netdev:
> 	free_netdev(net_dev);
>
> err_release_irq:
> 	...
>
> Please send a v2 patch.

Sure thing, I'll send it shortly.

Thanks for the feedback.
--
Gustavo