re: mmc: moxart: Add MOXA ART SD/MMC driver

re: mmc: moxart: Add MOXA ART SD/MMC driver

[Dan Carpenter]

Hello Jonas Jensen,

The patch 1b66e94e6b99: "mmc: moxart: Add MOXA ART SD/MMC driver"
from Apr 9, 2014, leads to the following static checker warning:

	drivers/mmc/host/moxart-mmc.c:691 moxart_remove()
	warn: variable dereferenced before check 'mmc' (see line 687)

drivers/mmc/host/moxart-mmc.c
   684  static int moxart_remove(struct platform_device *pdev)
   685  {
   686          struct mmc_host *mmc = dev_get_drvdata(&pdev->dev);
   687          struct moxart_host *host = mmc_priv(mmc);
                                           ^^^^^^^^^^^^^
Dereference.

   688  
   689          dev_set_drvdata(&pdev->dev, NULL);
   690  
   691          if (mmc) {
                    ^^^
Check.

   692                  if (!IS_ERR(host->dma_chan_tx))

regards,
dan carpenter

Re: mmc: moxart: Add MOXA ART SD/MMC driver

[Arnd Bergmann]

On Wednesday 21 May 2014 15:28:25 Dan Carpenter wrote:
> Hello Jonas Jensen,
> 
> The patch 1b66e94e6b99: "mmc: moxart: Add MOXA ART SD/MMC driver"
> from Apr 9, 2014, leads to the following static checker warning:
> 
>         drivers/mmc/host/moxart-mmc.c:691 moxart_remove()
>         warn: variable dereferenced before check 'mmc' (see line 687)
> 
> drivers/mmc/host/moxart-mmc.c
>    684  static int moxart_remove(struct platform_device *pdev)
>    685  {
>    686          struct mmc_host *mmc = dev_get_drvdata(&pdev->dev);
>    687          struct moxart_host *host = mmc_priv(mmc);
>                                            ^^^^^^^^^^^^^
> Dereference.
> 
>    688  
>    689          dev_set_drvdata(&pdev->dev, NULL);
>    690  
>    691          if (mmc) {
>                     ^^^
> Check.

Right, the check should just be removed, because the moxart_remove
function will not be called unless moxart_probe() has successfully
set the drvdata.


On a related note, I also found two bugs using automated build testing:

a) The driver should use dma_request_slave_channel_reason() instead of
of_dma_request_slave_channel() so it can be compiled when CONFIG_OF is
disabled.

b) moxart_remove() contains an extraneous kfree(host), where host is
a variable that has already been freed at that point.

	Arnd

Re: mmc: moxart: Add MOXA ART SD/MMC driver

[Jonas Jensen]

On 21 May 2014 14:28, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Hello Jonas Jensen,
>
> The patch 1b66e94e6b99: "mmc: moxart: Add MOXA ART SD/MMC driver"
> from Apr 9, 2014, leads to the following static checker warning:
>
>         drivers/mmc/host/moxart-mmc.c:691 moxart_remove()
>         warn: variable dereferenced before check 'mmc' (see line 687)


Thanks, I'll post a patch with fixes for this and bugs spotted by Arnd.

Regards,
Jonas