Re: [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt

[Tony Lindgren]

* Qi Hou  [180110 20:54]:
> When more than one GP timers are used as kernel system timers and the
> corresponding nodes in device-tree are marked with the same "disabled"
> property, then the "attr" field of the property will be initialized
> more than once as the property being added to sys file system via
> __of_add_property_sysfs().
> 
> In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
> directly to the return value of safe_name(), without taking care of
> whether it's already a valid pointer to a memory block. If it is, its
> old value will always be overwritten by the new one and the memory block
> allocated before will a "ghost", then a kmemleak happened.
> 
> That the same "disabled" property being added to different nodes of device
> tree would cause that kind of kmemleak overhead, at leat once.
> 
> To fix it, allocate the property dynamically, and delete static one.

Thanks applying into omap-for-v4.16/fixes.

Tony

Re: [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt

[Tony Lindgren]

* Qi Hou  [180110 20:54]:
> When more than one GP timers are used as kernel system timers and the
> corresponding nodes in device-tree are marked with the same "disabled"
> property, then the "attr" field of the property will be initialized
> more than once as the property being added to sys file system via
> __of_add_property_sysfs().
> 
> In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
> directly to the return value of safe_name(), without taking care of
> whether it's already a valid pointer to a memory block. If it is, its
> old value will always be overwritten by the new one and the memory block
> allocated before will a "ghost", then a kmemleak happened.
> 
> That the same "disabled" property being added to different nodes of device
> tree would cause that kind of kmemleak overhead, at leat once.
> 
> To fix it, allocate the property dynamically, and delete static one.

Thanks applying into omap-for-v4.16/fixes.

Tony

Re: [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt

[Keerthy]

On 1/13/2018 2:47 AM, Grygorii Strashko wrote:



On 01/12/2018 03:08 PM, Tony Lindgren wrote:

* Grygorii Strashko  [180112 20:36]:

Hi

On 01/10/2018 10:54 PM, Qi Hou wrote:

When more than one GP timers are used as kernel system timers and the
corresponding nodes in device-tree are marked with the same "disabled"
property, then the "attr" field of the property will be initialized
more than once as the property being added to sys file system via
__of_add_property_sysfs().

In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
directly to the return value of safe_name(), without taking care of
whether it's already a valid pointer to a memory block. If it is, its
old value will always be overwritten by the new one and the memory 
block

allocated before will a "ghost", then a kmemleak happened.

That the same "disabled" property being added to different nodes of 
device

tree would cause that kind of kmemleak overhead, at leat once.

To fix it, allocate the property dynamically, and delete static one.


Does it in sync with Keerthy's work [1]


First fixes, then new stuff! Keerthy's work will have to
wait for v4.17, we want that series sitting in Linux next
for several weeks.


Np. just want to be sure every party is aware about each other's work


Thanks Grygorii. I am not touching the mach-omap2 timer.c file as part 
of my migration series.


I applied the above patch and it seems my series applies cleanly after 
this patch. Compiled fine.

Re: [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt

[Keerthy]

On 1/13/2018 2:47 AM, Grygorii Strashko wrote:



On 01/12/2018 03:08 PM, Tony Lindgren wrote:

* Grygorii Strashko  [180112 20:36]:

Hi

On 01/10/2018 10:54 PM, Qi Hou wrote:

When more than one GP timers are used as kernel system timers and the
corresponding nodes in device-tree are marked with the same "disabled"
property, then the "attr" field of the property will be initialized
more than once as the property being added to sys file system via
__of_add_property_sysfs().

In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
directly to the return value of safe_name(), without taking care of
whether it's already a valid pointer to a memory block. If it is, its
old value will always be overwritten by the new one and the memory 
block

allocated before will a "ghost", then a kmemleak happened.

That the same "disabled" property being added to different nodes of 
device

tree would cause that kind of kmemleak overhead, at leat once.

To fix it, allocate the property dynamically, and delete static one.


Does it in sync with Keerthy's work [1]


First fixes, then new stuff! Keerthy's work will have to
wait for v4.17, we want that series sitting in Linux next
for several weeks.


Np. just want to be sure every party is aware about each other's work


Thanks Grygorii. I am not touching the mach-omap2 timer.c file as part 
of my migration series.


I applied the above patch and it seems my series applies cleanly after 
this patch. Compiled fine.

Re: [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt

[Grygorii Strashko]

On 01/12/2018 03:08 PM, Tony Lindgren wrote:

* Grygorii Strashko  [180112 20:36]:

Hi

On 01/10/2018 10:54 PM, Qi Hou wrote:

When more than one GP timers are used as kernel system timers and the
corresponding nodes in device-tree are marked with the same "disabled"
property, then the "attr" field of the property will be initialized
more than once as the property being added to sys file system via
__of_add_property_sysfs().

In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
directly to the return value of safe_name(), without taking care of
whether it's already a valid pointer to a memory block. If it is, its
old value will always be overwritten by the new one and the memory block
allocated before will a "ghost", then a kmemleak happened.

That the same "disabled" property being added to different nodes of device
tree would cause that kind of kmemleak overhead, at leat once.

To fix it, allocate the property dynamically, and delete static one.


Does it in sync with Keerthy's work [1]


First fixes, then new stuff! Keerthy's work will have to
wait for v4.17, we want that series sitting in Linux next
for several weeks.


Np. just want to be sure every party is aware about each other's work

--
regards,
-grygorii

Re: [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt

[Grygorii Strashko]

On 01/12/2018 03:08 PM, Tony Lindgren wrote:

* Grygorii Strashko  [180112 20:36]:

Hi

On 01/10/2018 10:54 PM, Qi Hou wrote:

When more than one GP timers are used as kernel system timers and the
corresponding nodes in device-tree are marked with the same "disabled"
property, then the "attr" field of the property will be initialized
more than once as the property being added to sys file system via
__of_add_property_sysfs().

In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
directly to the return value of safe_name(), without taking care of
whether it's already a valid pointer to a memory block. If it is, its
old value will always be overwritten by the new one and the memory block
allocated before will a "ghost", then a kmemleak happened.

That the same "disabled" property being added to different nodes of device
tree would cause that kind of kmemleak overhead, at leat once.

To fix it, allocate the property dynamically, and delete static one.


Does it in sync with Keerthy's work [1]


First fixes, then new stuff! Keerthy's work will have to
wait for v4.17, we want that series sitting in Linux next
for several weeks.


Np. just want to be sure every party is aware about each other's work

--
regards,
-grygorii

Re: [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt

[Tony Lindgren]

* Grygorii Strashko  [180112 20:36]:
> Hi
> 
> On 01/10/2018 10:54 PM, Qi Hou wrote:
> > When more than one GP timers are used as kernel system timers and the
> > corresponding nodes in device-tree are marked with the same "disabled"
> > property, then the "attr" field of the property will be initialized
> > more than once as the property being added to sys file system via
> > __of_add_property_sysfs().
> > 
> > In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
> > directly to the return value of safe_name(), without taking care of
> > whether it's already a valid pointer to a memory block. If it is, its
> > old value will always be overwritten by the new one and the memory block
> > allocated before will a "ghost", then a kmemleak happened.
> > 
> > That the same "disabled" property being added to different nodes of device
> > tree would cause that kind of kmemleak overhead, at leat once.
> > 
> > To fix it, allocate the property dynamically, and delete static one.
> 
> Does it in sync with Keerthy's work [1]

First fixes, then new stuff! Keerthy's work will have to
wait for v4.17, we want that series sitting in Linux next
for several weeks.

Regards,

Tony

> [1] https://www.spinics.net/lists/linux-omap/msg141100.html

Re: [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt

[Tony Lindgren]

* Grygorii Strashko  [180112 20:36]:
> Hi
> 
> On 01/10/2018 10:54 PM, Qi Hou wrote:
> > When more than one GP timers are used as kernel system timers and the
> > corresponding nodes in device-tree are marked with the same "disabled"
> > property, then the "attr" field of the property will be initialized
> > more than once as the property being added to sys file system via
> > __of_add_property_sysfs().
> > 
> > In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
> > directly to the return value of safe_name(), without taking care of
> > whether it's already a valid pointer to a memory block. If it is, its
> > old value will always be overwritten by the new one and the memory block
> > allocated before will a "ghost", then a kmemleak happened.
> > 
> > That the same "disabled" property being added to different nodes of device
> > tree would cause that kind of kmemleak overhead, at leat once.
> > 
> > To fix it, allocate the property dynamically, and delete static one.
> 
> Does it in sync with Keerthy's work [1]

First fixes, then new stuff! Keerthy's work will have to
wait for v4.17, we want that series sitting in Linux next
for several weeks.

Regards,

Tony

> [1] https://www.spinics.net/lists/linux-omap/msg141100.html

Re: [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt

[Grygorii Strashko]

Hi

On 01/10/2018 10:54 PM, Qi Hou wrote:

When more than one GP timers are used as kernel system timers and the
corresponding nodes in device-tree are marked with the same "disabled"
property, then the "attr" field of the property will be initialized
more than once as the property being added to sys file system via
__of_add_property_sysfs().

In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
directly to the return value of safe_name(), without taking care of
whether it's already a valid pointer to a memory block. If it is, its
old value will always be overwritten by the new one and the memory block
allocated before will a "ghost", then a kmemleak happened.

That the same "disabled" property being added to different nodes of device
tree would cause that kind of kmemleak overhead, at leat once.

To fix it, allocate the property dynamically, and delete static one.


Does it in sync with Keerthy's work [1]

[1] https://www.spinics.net/lists/linux-omap/msg141100.html


Signed-off-by: Qi Hou 
---
  arch/arm/mach-omap2/timer.c | 19 +++
  1 file changed, 11 insertions(+), 8 deletions(-)


--
regards,
-grygorii

Re: [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt

[Grygorii Strashko]

Hi

On 01/10/2018 10:54 PM, Qi Hou wrote:

When more than one GP timers are used as kernel system timers and the
corresponding nodes in device-tree are marked with the same "disabled"
property, then the "attr" field of the property will be initialized
more than once as the property being added to sys file system via
__of_add_property_sysfs().

In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
directly to the return value of safe_name(), without taking care of
whether it's already a valid pointer to a memory block. If it is, its
old value will always be overwritten by the new one and the memory block
allocated before will a "ghost", then a kmemleak happened.

That the same "disabled" property being added to different nodes of device
tree would cause that kind of kmemleak overhead, at leat once.

To fix it, allocate the property dynamically, and delete static one.


Does it in sync with Keerthy's work [1]

[1] https://www.spinics.net/lists/linux-omap/msg141100.html


Signed-off-by: Qi Hou 
---
  arch/arm/mach-omap2/timer.c | 19 +++
  1 file changed, 11 insertions(+), 8 deletions(-)


--
regards,
-grygorii

[PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt

[Qi Hou]

When more than one GP timers are used as kernel system timers and the
corresponding nodes in device-tree are marked with the same "disabled"
property, then the "attr" field of the property will be initialized
more than once as the property being added to sys file system via
__of_add_property_sysfs().

In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
directly to the return value of safe_name(), without taking care of
whether it's already a valid pointer to a memory block. If it is, its
old value will always be overwritten by the new one and the memory block
allocated before will a "ghost", then a kmemleak happened.

That the same "disabled" property being added to different nodes of device
tree would cause that kind of kmemleak overhead, at leat once.

To fix it, allocate the property dynamically, and delete static one.

Signed-off-by: Qi Hou 
---
 arch/arm/mach-omap2/timer.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index ece09c9..d61fbd7 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -156,12 +156,6 @@ static struct clock_event_device clockevent_gpt = {
.tick_resume= omap2_gp_timer_shutdown,
 };
 
-static struct property device_disabled = {
-   .name = "status",
-   .length = sizeof("disabled"),
-   .value = "disabled",
-};
-
 static const struct of_device_id omap_timer_match[] __initconst = {
{ .compatible = "ti,omap2420-timer", },
{ .compatible = "ti,omap3430-timer", },
@@ -203,8 +197,17 @@ static struct device_node * __init omap_get_timer_dt(const 
struct of_device_id *
  of_get_property(np, "ti,timer-secure", NULL)))
continue;
 
-   if (!of_device_is_compatible(np, "ti,omap-counter32k"))
-   of_add_property(np, _disabled);
+   if (!of_device_is_compatible(np, "ti,omap-counter32k")) {
+   struct property *prop;
+
+   prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+   if (!prop)
+   return NULL;
+   prop->name = "status";
+   prop->value = "disabled";
+   prop->length = strlen(prop->value);
+   of_add_property(np, prop);
+   }
return np;
}
 
-- 
2.7.4

[PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt

[Qi Hou]

When more than one GP timers are used as kernel system timers and the
corresponding nodes in device-tree are marked with the same "disabled"
property, then the "attr" field of the property will be initialized
more than once as the property being added to sys file system via
__of_add_property_sysfs().

In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
directly to the return value of safe_name(), without taking care of
whether it's already a valid pointer to a memory block. If it is, its
old value will always be overwritten by the new one and the memory block
allocated before will a "ghost", then a kmemleak happened.

That the same "disabled" property being added to different nodes of device
tree would cause that kind of kmemleak overhead, at leat once.

To fix it, allocate the property dynamically, and delete static one.

Signed-off-by: Qi Hou 
---
 arch/arm/mach-omap2/timer.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index ece09c9..d61fbd7 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -156,12 +156,6 @@ static struct clock_event_device clockevent_gpt = {
.tick_resume= omap2_gp_timer_shutdown,
 };
 
-static struct property device_disabled = {
-   .name = "status",
-   .length = sizeof("disabled"),
-   .value = "disabled",
-};
-
 static const struct of_device_id omap_timer_match[] __initconst = {
{ .compatible = "ti,omap2420-timer", },
{ .compatible = "ti,omap3430-timer", },
@@ -203,8 +197,17 @@ static struct device_node * __init omap_get_timer_dt(const 
struct of_device_id *
  of_get_property(np, "ti,timer-secure", NULL)))
continue;
 
-   if (!of_device_is_compatible(np, "ti,omap-counter32k"))
-   of_add_property(np, _disabled);
+   if (!of_device_is_compatible(np, "ti,omap-counter32k")) {
+   struct property *prop;
+
+   prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+   if (!prop)
+   return NULL;
+   prop->name = "status";
+   prop->value = "disabled";
+   prop->length = strlen(prop->value);
+   of_add_property(np, prop);
+   }
return np;
}
 
-- 
2.7.4