* re: ACER: Add support for accelerometer sensor
@ 2012-06-27 13:15 Dan Carpenter
2012-06-27 13:32 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2012-06-27 13:15 UTC (permalink / raw)
To: marex; +Cc: platform-driver-x86
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.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ACER: Add support for accelerometer sensor
2012-06-27 13:15 ACER: Add support for accelerometer sensor Dan Carpenter
@ 2012-06-27 13:32 ` Marek Vasut
2012-06-27 13:43 ` Dan Carpenter
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2012-06-27 13:32 UTC (permalink / raw)
To: Dan Carpenter; +Cc: platform-driver-x86
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?
> regards,
> dan carpenter
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ACER: Add support for accelerometer sensor
2012-06-27 13:32 ` Marek Vasut
@ 2012-06-27 13:43 ` Dan Carpenter
2012-06-27 13:59 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2012-06-27 13:43 UTC (permalink / raw)
To: Marek Vasut; +Cc: platform-driver-x86
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>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ACER: Add support for accelerometer sensor
2012-06-27 13:43 ` Dan Carpenter
@ 2012-06-27 13:59 ` Marek Vasut
2012-06-27 14:19 ` Dan Carpenter
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2012-06-27 13:59 UTC (permalink / raw)
To: Dan Carpenter; +Cc: platform-driver-x86
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() ?
> regards,
> dan carpenter
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ACER: Add support for accelerometer sensor
2012-06-27 13:59 ` Marek Vasut
@ 2012-06-27 14:19 ` Dan Carpenter
2012-06-27 15:01 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2012-06-27 14:19 UTC (permalink / raw)
To: Marek Vasut; +Cc: platform-driver-x86
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.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ACER: Add support for accelerometer sensor
2012-06-27 14:19 ` Dan Carpenter
@ 2012-06-27 15:01 ` Marek Vasut
2012-06-27 15:34 ` Dan Carpenter
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2012-06-27 15:01 UTC (permalink / raw)
To: Dan Carpenter; +Cc: platform-driver-x86
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?
> regards,
> dan carpenter
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ACER: Add support for accelerometer sensor
2012-06-27 15:01 ` Marek Vasut
@ 2012-06-27 15:34 ` Dan Carpenter
2012-06-27 16:22 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2012-06-27 15:34 UTC (permalink / raw)
To: Marek Vasut; +Cc: platform-driver-x86
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.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ACER: Add support for accelerometer sensor
2012-06-27 15:34 ` Dan Carpenter
@ 2012-06-27 16:22 ` Marek Vasut
0 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2012-06-27 16:22 UTC (permalink / raw)
To: Dan Carpenter; +Cc: platform-driver-x86
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-06-27 16:22 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-27 13:15 ACER: Add support for accelerometer sensor Dan Carpenter
2012-06-27 13:32 ` Marek Vasut
2012-06-27 13:43 ` Dan Carpenter
2012-06-27 13:59 ` Marek Vasut
2012-06-27 14:19 ` Dan Carpenter
2012-06-27 15:01 ` Marek Vasut
2012-06-27 15:34 ` Dan Carpenter
2012-06-27 16:22 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.