* 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@xxxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxx>
Attachment:
pgptMQJVX23p9.pgp
Description: PGP signature
