* 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.