[PATCH] staging: comedi: pcmmio: fix possible NULL deref on detach

[Ian Abbott]

On 2013-08-20 11:50, Ian Abbott wrote:
> pcmmio_detach is called by the comedi core even if pcmmio_attach()
> returned an error, so `dev->private` might be `NULL`.  Check for that
> before dereferencing it.
>
> Also, as pointed out by Dan carpenter for the similar pcmuio driver,
> there is no need to check the pointer passed to `kfree()`, so remove
> that check.
>
> Signed-off-by: Ian Abbott <abbotti at mev.co.uk>
> Cc: Dan Carpenter <dan.carpenter at oracle.com>
> ---
>   drivers/staging/comedi/drivers/pcmmio.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/pcmmio.c b/drivers/staging/comedi/drivers/pcmmio.c
> index aa3c030..cbd5624 100644
> --- a/drivers/staging/comedi/drivers/pcmmio.c
> +++ b/drivers/staging/comedi/drivers/pcmmio.c
> @@ -1156,12 +1156,13 @@ static void pcmmio_detach(struct comedi_device *dev)
>   	struct pcmmio_private *devpriv = dev->private;
>   	int i;
>
> -	for (i = 0; i < MAX_ASICS; ++i) {
> -		if (devpriv && devpriv->asics[i].irq)
> -			free_irq(devpriv->asics[i].irq, dev);
> -	}
> -	if (devpriv && devpriv->sprivs)
> +	if (devpriv) {
> +		for (i = 0; i < MAX_ASICS; ++i) {
> +			if (devpriv && devpriv->asics[i].irq)
> +				free_irq(devpriv->asics[i].irq, dev);
> +		}
>   		kfree(devpriv->sprivs);
> +	}
>   	comedi_legacy_detach(dev);
>   }

Actually, "pcmmio.c" doesn't have the NULL dereference bug.  The 
original code does in fact check the pointer before dereferencing it, 
unlike the similar function in "pcmuio.c".  The replacement code also 
checks the pointer twice.

I'll post a v2 version of this patch that avoids the unnecessary double 
checking of `devpriv`.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-