Re: ACER: Add support for accelerometer sensor

[Marek Vasut <marex@denx.de>]

Dear Dan Carpenter,

> On Wed, Jun 27, 2012 at 05:01:23PM +0200, Marek Vasut wrote:
> > Dear Dan Carpenter,
> > 
> > > On Wed, Jun 27, 2012 at 03:59:16PM +0200, Marek Vasut wrote:
> > > > Dear Dan Carpenter,
> > > > 
> > > > > On Wed, Jun 27, 2012 at 03:32:12PM +0200, Marek Vasut wrote:
> > > > > > Dear Dan Carpenter,
> > > > > > 
> > > > > > > Hello Marek Vasut,
> > > > > > > 
> > > > > > > The patch 6ae3a0876185: "ACER: Add support for accelerometer
> > > > > > > sensor" from Jun 1, 2012, leads to the following Smatch
> > > > > > > warning: drivers/platform/x86/acer-wmi.c:1886
> > > > > > > acer_wmi_accel_destroy()
> > > > > > > 
> > > > > > > 	 error: don't call input_free_device() after
> > > > > > > 	 input_unregister_device()
> > > > > > > 
> > > > > > > drivers/platform/x86/acer-wmi.c
> > > > > > > 
> > > > > > >   1883  static void acer_wmi_accel_destroy(void)
> > > > > > >   1884  {
> > > > > > >   1885          input_unregister_device(acer_wmi_accel_dev);
> > > > > > >   1886          input_free_device(acer_wmi_accel_dev);
> > > > > > >   1887  }
> > > > > > > 
> > > > > > > It is a double free.
> > > > > > 
> > > > > > I see, understood ... shall I submit subsequent patch?
> > > > > 
> > > > > Yes, please.  Could you give me a:
> > > > > 
> > > > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > 
> > > > Looking through input_unregister_device(), that call doesn't free the
> > > > structure. Actually, many drivers call explicitly kfree() on it.
> > > > 
> > > > Where do you see the double_free() ?
> > > 
> > > It's been a while since I looked at this code...
> > > 
> > > This is described in the comments for input_unregister_device().
> > > It's a refcounted thing.  It is freed when the last reference is
> > > dropped.
> > 
> > So kfree() eg. in here drivers/input/joystick/magellan.c is also wrong?
> 
> You are talking about this:?
> 
> 	input_unregister_device(magellan->dev);
> 	kfree(magellan);
> 
> The kfree() is fine.  It's just the ->dev pointer that you are not
> allowed to touch again after the unregister.

Ah, now it makes sense. Thanks for explaining :)

> regards,
> dan carpenter

Best regards,
Marek Vasut