Re: [PATCH] tm6000: Don't use pointer after freeing it in tm6000_ir_fini()

[Thierry Reding <thierry.reding@avionic-design.de>]

[-- Attachment #1: Type: text/plain, Size: 2063 bytes --]

* Jesper Juhl wrote:
> In tm6000_ir_fini() there seems to be a problem. 
> rc_unregister_device(ir->rc); calls rc_free_device() on the pointer it is 
> given, which frees it.
> 
> Subsequently the function does:
> 
>   if (!ir->polling)
>     __tm6000_ir_int_stop(ir->rc);
> 
> and __tm6000_ir_int_stop() dereferences the pointer it is given, which
> has already been freed.
> 
> and it also does:
> 
>   tm6000_ir_stop(ir->rc);
> 
> which also dereferences the (already freed) pointer.
> 
> So, it seems that the call to rc_unregister_device() should be move
> below the calls to __tm6000_ir_int_stop() and tm6000_ir_stop(), so
> those don't operate on a already freed pointer.
> 
> But, I must admit that I don't know this code *at all*, so someone who
> knows the code should take a careful look before applying this
> patch. It is based purely on inspection of facts of what is beeing
> freed where and not at all on understanding what the code does or why.
> I don't even have a means to test it, so beyond testing that the
> change compiles it has seen no testing what-so-ever.
> 
> Anyway, here's a proposed patch.
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  drivers/media/video/tm6000/tm6000-input.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/video/tm6000/tm6000-input.c b/drivers/media/video/tm6000/tm6000-input.c
> index 7844607..859eb90 100644
> --- a/drivers/media/video/tm6000/tm6000-input.c
> +++ b/drivers/media/video/tm6000/tm6000-input.c
> @@ -481,8 +481,6 @@ int tm6000_ir_fini(struct tm6000_core *dev)
>  
>  	dprintk(2, "%s\n",__func__);
>  
> -	rc_unregister_device(ir->rc);
> -
>  	if (!ir->polling)
>  		__tm6000_ir_int_stop(ir->rc);
>  
> @@ -492,6 +490,7 @@ int tm6000_ir_fini(struct tm6000_core *dev)
>  	tm6000_flash_led(dev, 0);
>  	ir->pwled = 0;
>  
> +	rc_unregister_device(ir->rc);
>  
>  	kfree(ir);
>  	dev->ir = NULL;
> -- 
> 1.7.8.4

Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]