[PATCH 1/2] clk: Fix __clk_get access to already freed owner field.

[PATCH 1/2] clk: Fix __clk_get access to already freed owner field.

[Alban Browaeys]

On the second call to __set_clk_parents from of_clk_set_defaults, here
when registering the second fimc device the kernel OOPS in an "unhandled
paging request at virtual address 6b6b6b77". This in __clk_get when
dereferencing clk->owner.

Move the clk free in the kref managed _clk_release call instead of
plain __clk_put.

Fixes: 035a61c314eb ("clk: Make clk API return per-user struct clk
instances)

Signed-off-by: Alban Browaeys <prahal@yahoo.com>
---
 drivers/clk/clk.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index df94668..8f33722 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2485,15 +2485,18 @@ EXPORT_SYMBOL_GPL(clk_register);
  */
 static void __clk_release(struct kref *ref)
 {
-	struct clk_core *clk = container_of(ref, struct clk_core, ref);
-	int i = clk->num_parents;
+	struct clk_core *core = container_of(ref, struct clk_core, ref);
+	struct clk *clk = container_of(&core, struct clk, core);
+	int i = core->num_parents;
 
-	kfree(clk->parents);
+	kfree(core->parents);
 	while (--i >= 0)
-		kfree_const(clk->parent_names[i]);
+		kfree_const(core->parent_names[i]);
+
+	kfree(core->parent_names);
+	kfree_const(core->name);
+	kfree(core);
 
-	kfree(clk->parent_names);
-	kfree_const(clk->name);
 	kfree(clk);
 }
 
@@ -2671,8 +2674,6 @@ void __clk_put(struct clk *clk)
 	clk_prepare_unlock();
 
 	module_put(owner);
-
-	kfree(clk);
 }
 
 /***        clk rate change notifiers        ***/
-- 
2.1.4

[PATCH 2/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.

[Alban Browaeys]

Put is called more than once, thus hlist_del ends up on a poisoned
list.

Move hlist_del to the __clk_release handler managed by kref instead
of calling it in _clk_put.

Fixes: 1c8e600440c7 ("clk: Add rate constraints to clocks")
Signed-off-by: Alban Browaeys <prahal@yahoo.com>
---
 drivers/clk/clk.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8f33722..f1d4b33 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2489,6 +2489,9 @@ static void __clk_release(struct kref *ref)
 	struct clk *clk = container_of(&core, struct clk, core);
 	int i = core->num_parents;
 
+	hlist_del(&clk->child_node);
+	clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
+
 	kfree(core->parents);
 	while (--i >= 0)
 		kfree_const(core->parent_names[i]);
@@ -2666,8 +2669,6 @@ void __clk_put(struct clk *clk)
 
 	clk_prepare_lock();
 
-	hlist_del(&clk->child_node);
-	clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
 	owner = clk->core->owner;
 	kref_put(&clk->core->ref, __clk_release);
 
-- 
2.1.4

Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.

[Stephen Boyd]

On 02/05/15 10:24, Alban Browaeys wrote:
> On the second call to __set_clk_parents from of_clk_set_defaults, here
> when registering the second fimc device the kernel OOPS in an "unhandled
> paging request at virtual address 6b6b6b77". This in __clk_get when
> dereferencing clk->owner.
>
> Move the clk free in the kref managed _clk_release call instead of
> plain __clk_put.
>
> Fixes: 035a61c314eb ("clk: Make clk API return per-user struct clk
> instances)
>
> Signed-off-by: Alban Browaeys <prahal@yahoo.com>
> ---
>  drivers/clk/clk.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index df94668..8f33722 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2485,15 +2485,18 @@ EXPORT_SYMBOL_GPL(clk_register);
>   */
>  static void __clk_release(struct kref *ref)
>  {
> -	struct clk_core *clk = container_of(ref, struct clk_core, ref);
> -	int i = clk->num_parents;
> +	struct clk_core *core = container_of(ref, struct clk_core, ref);
> +	struct clk *clk = container_of(&core, struct clk, core);

How does this work? struct clk_core doesn't have a struct clk inside it.

> +	int i = core->num_parents;
>  
> -	kfree(clk->parents);
> +	kfree(core->parents);
>  	while (--i >= 0)
> -		kfree_const(clk->parent_names[i]);
> +		kfree_const(core->parent_names[i]);

We don't have kfree_const() in the clk-next tree so please resend based
on clk-next, not linux-next.

> +
> +	kfree(core->parent_names);
> +	kfree_const(core->name);
> +	kfree(core);
>  
> -	kfree(clk->parent_names);
> -	kfree_const(clk->name);
>  	kfree(clk);
>  }
>  
> @@ -2671,8 +2674,6 @@ void __clk_put(struct clk *clk)
>  	clk_prepare_unlock();
>  
>  	module_put(owner);
> -
> -	kfree(clk);
>  }
>  
>  /***        clk rate change notifiers        ***/

I'm still confused. Care to send the actual backtrace and describe which
hardware you're running on (perhaps some dts file to look at)?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 2/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.

[Stephen Boyd]

On 02/05/15 10:24, Alban Browaeys wrote:
> Put is called more than once, thus hlist_del ends up on a poisoned
> list.

Why is clk_put() called more than once on the same clk pointer? Where
does this happe

>
> Move hlist_del to the __clk_release handler managed by kref instead
> of calling it in _clk_put.
>
> Fixes: 1c8e600440c7 ("clk: Add rate constraints to clocks")
> Signed-off-by: Alban Browaeys <prahal@yahoo.com>
> ---
>  drivers/clk/clk.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 8f33722..f1d4b33 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2489,6 +2489,9 @@ static void __clk_release(struct kref *ref)
>  	struct clk *clk = container_of(&core, struct clk, core);
>  	int i = core->num_parents;
>  
> +	hlist_del(&clk->child_node);
> +	clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
> +
>  	kfree(core->parents);
>  	while (--i >= 0)
>  		kfree_const(core->parent_names[i]);
> @@ -2666,8 +2669,6 @@ void __clk_put(struct clk *clk)
>  
>  	clk_prepare_lock();
>  
> -	hlist_del(&clk->child_node);

Ugh we should call this clks_node or something so that we don't confuse
it with child_node in clk_core.

> -	clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
>  	owner = clk->core->owner;
>  	kref_put(&clk->core->ref, __clk_release);
>  


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.

[Alban Browaeys]

Le jeudi 05 février 2015 à 11:30 -0800, Stephen Boyd a écrit :

> > Signed-off-by: Alban Browaeys <prahal@yahoo.com>
> > ---
> >  drivers/clk/clk.c | 17 +++++++++--------
> >  1 file changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index df94668..8f33722 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -2485,15 +2485,18 @@ EXPORT_SYMBOL_GPL(clk_register);
> >   */
> >  static void __clk_release(struct kref *ref)
> >  {
> > -	struct clk_core *clk = container_of(ref, struct clk_core, ref);
> > -	int i = clk->num_parents;
> > +	struct clk_core *core = container_of(ref, struct clk_core, ref);
> > +	struct clk *clk = container_of(&core, struct clk, core);
> 
> How does this work? struct clk_core doesn't have a struct clk inside it.
> 

Seems I am confused. The aim is  to get the clk struct from its core
field. If I cannot do that from within __clk_release , this fix is
doomed.

> > +	int i = core->num_parents;
> >  
> > -	kfree(clk->parents);
> > +	kfree(core->parents);
> >  	while (--i >= 0)
> > -		kfree_const(clk->parent_names[i]);
> > +		kfree_const(core->parent_names[i]);
> 
> We don't have kfree_const() in the clk-next tree so please resend based
> on clk-next, not linux-next.
> 

I will do after we confirmed there is a way to get to clk from clk_core.
Otherwise the fix makes no sense. 

> I'm still confused. Care to send the actual backtrace and describe which
> hardware you're running on (perhaps some dts file to look at)?
> 

This is the initial oops before any change (based on linux-next
20150204). 

[    7.264186] Unable to handle kernel paging request at virtual address 6b6b6b77
[    7.270206] pgd = eb0b4000
[    7.272809] [6b6b6b77] *pgd=00000000
[    7.276466] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[    7.281667] Modules linked in: exynosdrm(+) drm_kms_helper phy_exynos_usb2 fuse
[    7.288950] CPU: 1 PID: 98 Comm: systemd-modules Not tainted 3.19.0-rc7-next-20150204-00052-g1059e6a #91
[    7.298424] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    7.304488] task: ebae3c00 ti: eb0bc000 task.ti: eb0bc000
[    7.309888] PC is at __clk_get+0x30/0xa0
[    7.313781] LR is at of_clk_get_by_clkspec+0x38/0x54
[    7.318722] pc : [<c0556098>]    lr : [<c055231c>]    psr: 200d0053
[    7.318722] sp : eb0bdbb0  ip : eb0bdbc8  fp : eb0bdbc4
[    7.330187] r10: 00000006  r9 : 00000001  r8 : 00000000
[    7.335398] r7 : eb0bdbf8  r6 : 00000000  r5 : ee5c7d80  r4 : 6b6b6b6b
[    7.341913] r3 : 00000001  r2 : 00000011  r1 : ee0b7004  r0 : ee0ff600
[    7.341923] Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment user
[    7.341927] Control: 10c5387d  Table: 6b0b404a  DAC: 00000015
[    7.341931] Process systemd-modules (pid: 98, stack limit = 0xeb0bc218)
[    7.341934] Stack: (0xeb0bdbb0 to 0xeb0be000)
[    7.341939] dba0:                                     00000001 ee0ff600 eb0bdbdc eb0bdbc8
[    7.341945] dbc0: c055231c c0556074 00000001 ed834850 eb0bdc6c eb0bdbe0 c0558528 c05522f0
[    7.341950] dbe0: eb0bdbf8 c01cc560 eb3e4710 ee2b4200 eb0bdc14 c01ced00 ee5e0d3c 00000001
[    7.341956] dc00: 00000011 ee2b4200 eb0bdc34 eb3e4900 00000000 c08c5790 ee2b4200 eb3e4700
[    7.341962] dc20: 00000001 00000006 eb0bdc5c eb0bdc38 c01ced00 c01cb2d0 ed834850 00000000
[    7.341968] dc40: ed834858 ed834850 ed834850 bf06c0b4 c0aa82b8 00000000 bf06c0b4 00000006
[    7.341974] dc60: eb0bdc8c eb0bdc70 c044213c c0558474 ed834850 c0b61248 c0b61254 c0aa82b8
[    7.341979] dc80: eb0bdcc4 eb0bdc90 c043ff34 c0442120 00000000 bf0631f0 e9c3b700 ed834850
[    7.341985] dca0: bf06c0b4 ed834884 00000000 bf0631f0 e9c3b700 c0a4f40c eb0bdce4 eb0bdcc8
[    7.341991] dcc0: c044020c c043fdc8 00000000 00000000 bf06c0b4 c0440194 eb0bdd0c eb0bdce8
[    7.341997] dce0: c043ded8 c04401a0 ee284e38 ed830900 c06f5728 bf06c0b4 eb1477c0 c0a87448
[    7.342003] dd00: eb0bdd1c eb0bdd10 c043fa14 c043de88 eb0bdd44 eb0bdd20 c043f460 c043f9f4
[    7.342009] dd20: bf069280 eb0bdd30 bf06c0b4 00000000 bf0631e8 bf06c388 eb0bdd5c eb0bdd48
[    7.342014] dd40: c0440bfc c043f370 00000001 00000000 eb0bdd6c eb0bdd60 c0442094 c0440b50
[    7.342020] dd60: eb0bddbc eb0bdd70 bf04ca08 c044203c 00000000 bf065090 ffffffff 00000000
[    7.342026] dd80: 00000000 00000000 00000000 00000000 00000000 00000000 c0a53b20 bf06c208
[    7.342031] dda0: c0a53b20 bf04c950 00000000 c0a53b20 eb0bde4c eb0bddc0 c0008b28 bf04c95c
[    7.342037] ddc0: 0010000f 00000000 eb0bddec eb0bddd8 c00504f4 c006dde0 eb0bc000 00000000
[    7.342043] dde0: ee002140 000000d0 c06ed170 0000000c c0a50600 00000000 eb0bde4c eb0bde08
[    7.342049] de00: c01541d8 c0153934 eb0bc008 eb0bde08 00000000 eb0bc008 ee002140 dc8cb100
[    7.342055] de20: 00000001 bf06c208 00000001 e9c3b600 e9c3bb00 00000001 163c451c e9c3bb08
[    7.342060] de40: eb0bde74 eb0bde50 c06ed1ac c00089ec eb0bde74 eb0bde60 c014496c eb0bdf48
[    7.342066] de60: 00000001 bf06c208 eb0bdf3c eb0bde78 c00af61c c06ed148 bf06c214 00007fff
[    7.342072] de80: c00ac6a8 eb0bdf48 eb0bdeb4 f0473db8 00000780 00000777 f0473e84 bf06c214
[    7.342078] dea0: bf06c378 b6e609f8 bf06c250 c0a4f40c c00ad024 c0169924 00000000 00000000
[    7.342084] dec0: bf063194 00000009 00000000 00000000 6e72656b 00006c65 00000000 00000000
[    7.342089] dee0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    7.342095] df00: 00000000 00000000 00000000 dc8cb100 eb0bdf2c 00000000 00000006 b6e609f8
[    7.342100] df20: 0000017b c000fb64 eb0bc000 00000000 eb0bdfa4 eb0bdf40 c00afdec c00ada4c
[    7.342106] df40: c0180738 00000000 f0454000 002bb414 f070ec6c f066643f f066dd80 00020390
[    7.342112] df60: 00026450 bf06c1f0 00000001 00000000 0000002f 00000030 0000001a 00000000
[    7.342118] df80: 00000008 00000000 00000000 b6e617d4 00028948 39980800 00000000 eb0bdfa8
[    7.342124] dfa0: c000f9c0 c00afd54 b6e617d4 00028948 00000006 b6e609f8 00000000 b6e6131c
[    7.342129] dfc0: b6e617d4 00028948 39980800 0000017b 00020000 00015964 00015f34 0002e640
[    7.342135] dfe0: beb38268 beb38258 b6e5ac4b b6f03d42 600d0070 00000006 5a5a5a5a 5a5a5a5a
[    7.342150] [<c0556098>] (__clk_get) from [<c055231c>] (of_clk_get_by_clkspec+0x38/0x54)
[    7.342162] [<c055231c>] (of_clk_get_by_clkspec) from [<c0558528>] (of_clk_set_defaults+0xc0/0x2ec)
[    7.342171] [<c0558528>] (of_clk_set_defaults) from [<c044213c>] (platform_drv_probe+0x28/0xb0)
[    7.342185] [<c044213c>] (platform_drv_probe) from [<c043ff34>] (driver_probe_device+0x178/0x384)
[    7.342193] [<c043ff34>] (driver_probe_device) from [<c044020c>] (__driver_attach+0x78/0x9c)
[    7.342201] [<c044020c>] (__driver_attach) from [<c043ded8>] (bus_for_each_dev+0x5c/0xb4)
[    7.342208] [<c043ded8>] (bus_for_each_dev) from [<c043fa14>] (driver_attach+0x2c/0x30)
[    7.342215] [<c043fa14>] (driver_attach) from [<c043f460>] (bus_add_driver+0xfc/0x228)
[    7.342222] [<c043f460>] (bus_add_driver) from [<c0440bfc>] (driver_register+0xb8/0xf8)
[    7.342231] [<c0440bfc>] (driver_register) from [<c0442094>] (__platform_driver_register+0x64/0x6c)
[    7.342326] [<c0442094>] (__platform_driver_register) from [<bf04ca08>] (exynos_drm_init+0xb8/0x1d0 [exynosdrm])
[    7.342363] [<bf04ca08>] (exynos_drm_init [exynosdrm]) from [<c0008b28>] (do_one_initcall+0x148/0x224)
[    7.342376] [<c0008b28>] (do_one_initcall) from [<c06ed1ac>] (do_init_module+0x70/0x1bc)
[    7.342390] [<c06ed1ac>] (do_init_module) from [<c00af61c>] (load_module+0x1bdc/0x21f0)
[    7.342399] [<c00af61c>] (load_module) from [<c00afdec>] (SyS_finit_module+0xa4/0xb4)
[    7.342409] [<c00afdec>] (SyS_finit_module) from [<c000f9c0>] (ret_fast_syscall+0x0/0x34)
[    7.342415] Code: e89da818 e5904000 e3540000 0afffffa (e594000c) 
[    7.342464] ---[ end trace d90d42eb4fbac408 ]---



dts with the offending fimc nodes arch/arm/boot/dts/exynos4412-odroid-common.dtsi in linus master and linux-next.



another version of the oops with added pr_warn before calls to of_clk_get_by_clkspec :
- __set_clk_parents: Assigned clk parents clk: try to get parent clock
this before the assigned-clock-parents is sent to of_clk_get_by_clkspec in __set_clk_parents
- __set_clk_parents: Assigned clocks clk: try to get parent clock
this before the assigned-clocks is sent to of_clk_get_by_clkspec in __set_clk_parents

- __set_clk_rates Assigned clocks clk: couldn't clock
this before the assigned-clocks is sent to of_clk_get_by_clkspec in __set_clk_rates



[    7.212207] s5p-g2d 10800000.g2d: The exynos g2d(ver 4.1) successfully probed
[    7.215130] __set_clk_parents: Assigned clk parents clk: try to get parent clock 0 for /camera/fimc@11800000
[    7.224297] __set_clk_parents: Assigned clocks clk: try to get parent clock 0 for /camera/fimc@11800000
[    7.233505] __set_clk_rates Assigned clocks clk: couldn't get clock 1 for /camera/fimc@11800000
[    7.241902] cam-power-domain: Power-on latency exceeded, new value 369958 ns
[    7.249084] __set_clk_parents: Assigned clk parents clk: try to get parent clock 0 for /camera/fimc@11810000
[    7.258246] Unable to handle kernel paging request at virtual address 6b6b6b77
[    7.265425] pgd = eb114000
[    7.268067] [6b6b6b77] *pgd=00000000
[    7.271625] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[    7.276922] Modules linked in: exynosdrm(+) drm_kms_helper phy_exynos_usb2 fuse
[    7.284209] CPU: 1 PID: 100 Comm: systemd-modules Not tainted 3.19.0-rc7-next-20150204-00056-g37e6c6e-dirty #106
[    7.294382] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    7.300447] task: ebad4b00 ti: eb094000 task.ti: eb094000
[    7.300462] PC is at __clk_get+0x30/0xa0
[    7.300467] LR is at of_clk_get_by_clkspec+0x38/0x54
[    7.300471] pc : [<c0556088>]    lr : [<c055230c>]    psr: 200e0053
[    7.300471] sp : eb095bb0  ip : eb095bc8  fp : eb095bc4
[    7.300474] r10: 00000007  r9 : bf06c0a8  r8 : 00000000
[    7.300477] r7 : 00000001  r6 : 00000000  r5 : ed850850  r4 : 6b6b6b6b
[    7.300480] r3 : 00000000  r2 : 00000011  r1 : ee0b7004  r0 : ee0ff600
[    7.300484] Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment user
[    7.300487] Control: 10c5387d  Table: 6b11404a  DAC: 00000015
[    7.300491] Process systemd-modules (pid: 100, stack limit = 0xeb094218)
[    7.300494] Stack: (0xeb095bb0 to 0xeb096000)
[    7.300499] 5ba0:                                     00000000 ee0ff600 eb095bdc eb095bc8
[    7.300505] 5bc0: c055230c c0556064 00000000 ee5c7d80 eb095c6c eb095be0 c055852c c05522e0
[    7.300511] 5be0: eb095bf8 c01cc560 eb2a4310 ed847200 eb095c14 c01ced00 ee5e0d3c 00000001
[    7.300516] 5c00: 00000011 ed847200 eb095c34 eb2a4100 00000000 c08c6798 ed847200 eb2a4300
[    7.300522] 5c20: 00000001 00000007 eb095c5c eb095c38 c01ced00 c01cb2d0 ed850850 00000000
[    7.300527] 5c40: ed850858 ed850850 ed850850 bf06c0a8 c0aa82b8 00000000 bf06c0a8 00000007
[    7.300533] 5c60: eb095c8c eb095c70 c044212c c0558464 ed850850 c0b61248 c0b61254 c0aa82b8
[    7.300539] 5c80: eb095cc4 eb095c90 c043ff24 c0442110 00000000 bf0631e4 eb389700 ed850850
[    7.300545] 5ca0: bf06c0a8 ed850884 00000000 bf0631e4 eb389700 c0a4f40c eb095ce4 eb095cc8
[    7.300550] 5cc0: c04401fc c043fdb8 00000000 00000000 bf06c0a8 c0440184 eb095d0c eb095ce8
[    7.300556] 5ce0: c043dec8 c0440190 ee284e38 ed84e900 c06f5750 bf06c0a8 eb08dcc0 c0a87448
[    7.300562] 5d00: eb095d1c eb095d10 c043fa04 c043de78 eb095d44 eb095d20 c043f450 c043f9e4
[    7.300568] 5d20: bf069274 eb095d30 bf06c0a8 00000000 bf0631dc bf06c378 eb095d5c eb095d48
[    7.300574] 5d40: c0440bec c043f360 00000001 00000000 eb095d6c eb095d60 c0442084 c0440b40
[    7.300580] 5d60: eb095dbc eb095d70 bf04ca08 c044202c 00000000 bf065084 ffffffff 00000000
[    7.300585] 5d80: 00000000 00000000 00000000 00000000 00000000 00000000 c0a53b20 bf06c1f8
[    7.300591] 5da0: c0a53b20 bf04c950 00000000 c0a53b20 eb095e4c eb095dc0 c0008b28 bf04c95c
[    7.300597] 5dc0: 0010000f 00000000 eb095dec eb095dd8 c00504f4 c006dde0 eb094000 00000000
[    7.300602] 5de0: ee002140 000000d0 c06ed198 0000000c c0a50600 00000002 eb095e4c eb095e08
[    7.300608] 5e00: c01541d8 c0153934 eb094008 eb095e08 00000002 eb094008 ee002140 dc8cb100
[    7.300614] 5e20: 00000001 bf06c1f8 00000001 ebb3cf00 eb389300 00000001 14c76d1c eb389308
[    7.300620] 5e40: eb095e74 eb095e50 c06ed1d4 c00089ec eb095e74 eb095e60 c014496c eb095f48
[    7.300626] 5e60: 00000001 bf06c1f8 eb095f3c eb095e78 c00af61c c06ed170 bf06c204 00007fff
[    7.300632] 5e80: c00ac6a8 eb095f48 eb095eb4 f0473db8 00000780 00000777 f0473e84 bf06c204
[    7.300637] 5ea0: bf06c368 b6df59f8 bf06c240 c0a4f40c c00ad024 c0169924 00000000 00000000
[    7.300643] 5ec0: bf063188 00000009 00000000 00000000 6e72656b 00006c65 00000000 00000000
[    7.300648] 5ee0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    7.300654] 5f00: 00000000 00000000 00000000 dc8cb100 eb095f2c 00000000 00000006 b6df59f8
[    7.300660] 5f20: 0000017b c000fb64 eb094000 00000000 eb095fa4 eb095f40 c00afdec c00ada4c
[    7.300666] 5f40: c0180738 00000000 f0454000 002bb374 f070ebcc f06663c5 f066dd08 00020380
[    7.300672] 5f60: 00026440 bf06c1e0 00000001 00000000 0000002f 00000030 0000001a 00000000
[    7.300677] 5f80: 00000008 00000000 00000000 b6df67d4 00028948 1747b100 00000000 eb095fa8
[    7.300683] 5fa0: c000f9c0 c00afd54 b6df67d4 00028948 00000006 b6df59f8 00000000 b6df631c
[    7.300689] 5fc0: b6df67d4 00028948 1747b100 0000017b 00020000 00015964 00015f34 0002e640
[    7.300695] 5fe0: bebf0268 bebf0258 b6defc4b b6e98d42 600d0070 00000006 5a5a5a5a 5a5a5a5a
[    7.300706] [<c0556088>] (__clk_get) from [<c055230c>] (of_clk_get_by_clkspec+0x38/0x54)
[    7.300717] [<c055230c>] (of_clk_get_by_clkspec) from [<c055852c>] (of_clk_set_defaults+0xd4/0x324)
[    7.300726] [<c055852c>] (of_clk_set_defaults) from [<c044212c>] (platform_drv_probe+0x28/0xb0)
[    7.300739] [<c044212c>] (platform_drv_probe) from [<c043ff24>] (driver_probe_device+0x178/0x384)
[    7.300747] [<c043ff24>] (driver_probe_device) from [<c04401fc>] (__driver_attach+0x78/0x9c)
[    7.300755] [<c04401fc>] (__driver_attach) from [<c043dec8>] (bus_for_each_dev+0x5c/0xb4)
[    7.300762] [<c043dec8>] (bus_for_each_dev) from [<c043fa04>] (driver_attach+0x2c/0x30)
[    7.300769] [<c043fa04>] (driver_attach) from [<c043f450>] (bus_add_driver+0xfc/0x228)
[    7.300776] [<c043f450>] (bus_add_driver) from [<c0440bec>] (driver_register+0xb8/0xf8)
[    7.300785] [<c0440bec>] (driver_register) from [<c0442084>] (__platform_driver_register+0x64/0x6c)
[    7.300872] [<c0442084>] (__platform_driver_register) from [<bf04ca08>] (exynos_drm_init+0xb8/0x1d0 [exynosdrm])
[    7.300905] [<bf04ca08>] (exynos_drm_init [exynosdrm]) from [<c0008b28>] (do_one_initcall+0x148/0x224)
[    7.300918] [<c0008b28>] (do_one_initcall) from [<c06ed1d4>] (do_init_module+0x70/0x1bc)
[    7.300931] [<c06ed1d4>] (do_init_module) from [<c00af61c>] (load_module+0x1bdc/0x21f0)
[    7.300939] [<c00af61c>] (load_module) from [<c00afdec>] (SyS_finit_module+0xa4/0xb4)
[    7.300950] [<c00afdec>] (SyS_finit_module) from [<c000f9c0>] (ret_fast_syscall+0x0/0x34)
[    7.300956] Code: e89da818 e5904000 e3540000 0afffffa (e594000�


Best regards,
Alban

Re: [PATCH 2/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.

[Alban Browaeys]

Le jeudi 05 février 2015 à 11:33 -0800, Stephen Boyd a écrit :
> On 02/05/15 10:24, Alban Browaeys wrote:
> > Put is called more than once, thus hlist_del ends up on a poisoned
> > list.
> 
> Why is clk_put() called more than once on the same clk pointer? Where
> does this happe
> 

With only patch 1 from this set applied I get another oops: 


[    7.346612] Unable to handle kernel paging request at virtual address 00200200
[    7.353686] pgd = ebbf4000
[    7.355939] [00200200] *pgd=00000000
[    7.359444] Internal error: Oops: 805 [#1] PREEMPT SMP ARM
[    7.364914] Modules linked in: exynosdrm(+) drm_kms_helper phy_exynos_usb2 fuse
[    7.372201] CPU: 3 PID: 102 Comm: systemd-modules Not tainted 3.19.0-rc7-next-20150204-00053-g17e4521 #93
[    7.381764] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    7.387832] task: eb0a8f00 ti: ebbfe000 task.ti: ebbfe000
[    7.393213] PC is at __clk_put+0x64/0xdc
[    7.397111] LR is at clk_prepare_lock+0x20/0x100
[    7.401712] pc : [<c055616c>]    lr : [<c0552bfc>]    psr: 200f0053
[    7.401712] sp : ebbffbb8  ip : ebbffba0  fp : ebbffbcc
[    7.413185] r10: ee0ff600  r9 : 00000001  r8 : 00000000
[    7.413188] r7 : ebbffbf8  r6 : 00000000  r5 : ee5c7d80  r4 : ee0ff600
[    7.413195] r3 : 00100100  r2 : 00200200  r1 : 080007ff  r0 : 00000001
[    7.413200] Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment user
[    7.413204] Control: 10c5387d  Table: 6bbf404a  DAC: 00000015
[    7.413208] Process systemd-modules (pid: 102, stack limit = 0xebbfe218)
[    7.413211] Stack: (0xebbffbb8 to 0xebc00000)
[    7.413215] fba0:                                                       00000000 ee106400
[    7.413222] fbc0: ebbffbdc ebbffbd0 c055222c c0556114 ebbffc6c ebbffbe0 c0558604 c0552220
[    7.413228] fbe0: ebbffbf8 c01cc560 ebbbc310 ee2b5200 ebbffc14 c01ced00 ee5e0d3c 00000001
[    7.413234] fc00: 00000181 ee2b5200 ebbffc34 ebbbc100 00000000 c08c5790 ee2b5200 ebbbc300
[    7.413239] fc20: 00000001 00000006 ebbffc5c ebbffc38 c01ced00 c01cb2d0 ed834850 00000000
[    7.413245] fc40: ed834858 ed834850 ed834850 bf06c0b4 c0aa82b8 00000000 bf06c0b4 00000006
[    7.413251] fc60: ebbffc8c ebbffc70 c044213c c055846c ed834850 c0b61248 c0b61254 c0aa82b8
[    7.413257] fc80: ebbffcc4 ebbffc90 c043ff34 c0442120 ed834850 bf06c0b4 ed834884 ed834850
[    7.413263] fca0: bf06c0b4 ed834884 00000000 bf0631f0 eb3cfc00 c0a4f40c ebbffce4 ebbffcc8
[    7.413269] fcc0: c044020c c043fdc8 00000000 00000000 bf06c0b4 c0440194 ebbffd0c ebbffce8
[    7.413275] fce0: c043ded8 c04401a0 ee284e38 ed830900 c06f5720 bf06c0b4 eb329cc0 c0a87448
[    7.413280] fd00: ebbffd1c ebbffd10 c043fa14 c043de88 ebbffd44 ebbffd20 c043f460 c043f9f4
[    7.413286] fd20: bf069280 ebbffd30 bf06c0b4 00000000 bf0631e8 bf06c388 ebbffd5c ebbffd48
[    7.413292] fd40: c0440bfc c043f370 00000001 00000000 ebbffd6c ebbffd60 c0442094 c0440b50
[    7.413298] fd60: ebbffdbc ebbffd70 bf04ca08 c044203c 00000000 bf065090 ffffffff 00000000
[    7.413303] fd80: 00000000 00000000 00000000 00000000 00000000 00000000 c0a53b20 bf06c208
[    7.413309] fda0: c0a53b20 bf04c950 00000000 c0a53b20 ebbffe4c ebbffdc0 c0008b28 bf04c95c
[    7.413315] fdc0: 0010000f 00000000 ebbffdec ebbffdd8 c00504f4 c006dde0 ebbfe000 00000000
[    7.413321] fde0: ee002140 000000d0 c06ed168 0000000c c0a50600 00000001 ebbffe4c ebbffe08
[    7.413327] fe00: c01541d8 c0153934 ebbfe008 ebbffe08 00000001 ebbfe008 ee002140 dc8cb100
[    7.413333] fe20: 00000001 bf06c208 00000001 eb3cfd00 eb3cf000 00000001 14c3101c eb3cf008
[    7.413339] fe40: ebbffe74 ebbffe50 c06ed1a4 c00089ec ebbffe74 ebbffe60 c014496c ebbfff48
[    7.413345] fe60: 00000001 bf06c208 ebbfff3c ebbffe78 c00af61c c06ed140 bf06c214 00007fff
[    7.413350] fe80: c00ac6a8 ebbfff48 ebbffeb4 f0473db8 00000780 00000777 f0473e84 bf06c214
[    7.413356] fea0: bf06c378 b6dc99f8 bf06c250 c0a4f40c c00ad024 c0169924 00000000 00000000
[    7.413362] fec0: bf063194 00000009 00000000 00000000 6e72656b 00006c65 00000000 00000000
[    7.413367] fee0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    7.413373] ff00: 00000000 00000000 00000000 dc8cb100 ebbfff2c 00000000 00000006 b6dc99f8
[    7.413379] ff20: 0000017b c000fb64 ebbfe000 00000000 ebbfffa4 ebbfff40 c00afdec c00ada4c
[    7.413385] ff40: c0180738 00000000 f0454000 002bb414 f070ec6c f066643f f066dd80 00020390
[    7.413390] ff60: 00026450 bf06c1f0 00000001 00000000 0000002f 00000030 0000001a 00000000
[    7.413396] ff80: 00000008 00000000 00000000 b6dca7d4 00028948 e1c5e100 00000000 ebbfffa8
[    7.413402] ffa0: c000f9c0 c00afd54 b6dca7d4 00028948 00000006 b6dc99f8 00000000 b6dca31c
[    7.413408] ffc0: b6dca7d4 00028948 e1c5e100 0000017b 00020000 00015964 00015f34 0002e640
[    7.413414] ffe0: bedc0268 bedc0258 b6dc3c4b b6e6cd42 600d0070 00000006 6f7fd821 6f7fdc21
[    7.413427] [<c055616c>] (__clk_put) from [<c055222c>] (clk_put+0x18/0x1c)
[    7.413438] [<c055222c>] (clk_put) from [<c0558604>] (of_clk_set_defaults+0x1a4/0x2ec)
[    7.413448] [<c0558604>] (of_clk_set_defaults) from [<c044213c>] (platform_drv_probe+0x28/0xb0)
[    7.413462] [<c044213c>] (platform_drv_probe) from [<c043ff34>] (driver_probe_device+0x178/0x384)
[    7.413470] [<c043ff34>] (driver_probe_device) from [<c044020c>] (__driver_attach+0x78/0x9c)
[    7.413478] [<c044020c>] (__driver_attach) from [<c043ded8>] (bus_for_each_dev+0x5c/0xb4)
[    7.413485] [<c043ded8>] (bus_for_each_dev) from [<c043fa14>] (driver_attach+0x2c/0x30)
[    7.413492] [<c043fa14>] (driver_attach) from [<c043f460>] (bus_add_driver+0xfc/0x228)
[    7.413500] [<c043f460>] (bus_add_driver) from [<c0440bfc>] (driver_register+0xb8/0xf8)
[    7.413509] [<c0440bfc>] (driver_register) from [<c0442094>] (__platform_driver_register+0x64/0x6c)
[    7.413597] [<c0442094>] (__platform_driver_register) from [<bf04ca08>] (exynos_drm_init+0xb8/0x1d0 [exynosdrm])
[    7.413631] [<bf04ca08>] (exynos_drm_init [exynosdrm]) from [<c0008b28>] (do_one_initcall+0x148/0x224)
[    7.413644] [<c0008b28>] (do_one_initcall) from [<c06ed1a4>] (do_init_module+0x70/0x1bc)
[    7.413657] [<c06ed1a4>] (do_init_module) from [<c00af61c>] (load_module+0x1bdc/0x21f0)
[    7.413665] [<c00af61c>] (load_module) from [<c00afdec>] (SyS_finit_module+0xa4/0xb4)
[    7.413675] [<c00afdec>] (SyS_finit_module) from [<c000f9c0>] (ret_fast_syscall+0x0/0x34)
[    7.413681] Code: ebfff29e e5943014 e5942018 e3530000 (e582300


from the patch 1 debugging I concluded that in __set_clk_parents, the parent clk of
 the fimc devices was the same thus clk_put was called for each of the four of them 
(but oops at the second one).

This indeed would deserve a debug run of itself. I only deduced the second oops behaviour
from the analysis of the first oops as both happens in the same loop . This is weak. Will do
the detailed testing for this one too .

Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.

[Stephen Boyd]

On 02/05/15 12:43, Alban Browaeys wrote:
> Le jeudi 05 février 2015 à 11:30 -0800, Stephen Boyd a écrit :
>
>>> Signed-off-by: Alban Browaeys <prahal@yahoo.com>
>>> ---
>>>  drivers/clk/clk.c | 17 +++++++++--------
>>>  1 file changed, 9 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>>> index df94668..8f33722 100644
>>> --- a/drivers/clk/clk.c
>>> +++ b/drivers/clk/clk.c
>>> @@ -2485,15 +2485,18 @@ EXPORT_SYMBOL_GPL(clk_register);
>>>   */
>>>  static void __clk_release(struct kref *ref)
>>>  {
>>> -	struct clk_core *clk = container_of(ref, struct clk_core, ref);
>>> -	int i = clk->num_parents;
>>> +	struct clk_core *core = container_of(ref, struct clk_core, ref);
>>> +	struct clk *clk = container_of(&core, struct clk, core);
>> How does this work? struct clk_core doesn't have a struct clk inside it.
>>
> Seems I am confused. The aim is  to get the clk struct from its core
> field. If I cannot do that from within __clk_release , this fix is
> doomed.
>

Thanks for the information. Can you please try the patch in this other
thread[1]? I think you're seeing the same problem.

[1] https://lkml.org/lkml/2015/2/5/595

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.

[Alban Browaeys]

> 
> Thanks for the information. Can you please try the patch in this other
> thread[1]? I think you're seeing the same problem.
> 
> [1] https://lkml.org/lkml/2015/2/5/595


This fixes both oops : the one from __clk_get and the next from
hlist_del.

Thanks
Alban

Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.

[Stephen Boyd]

On 02/05/15 13:30, Alban Browaeys wrote:
>> Thanks for the information. Can you please try the patch in this other
>> thread[1]? I think you're seeing the same problem.
>>
>> [1] https://lkml.org/lkml/2015/2/5/595
>
> This fixes both oops : the one from __clk_get and the next from
> hlist_del.
>

Great! Can I take this as your Tested-by?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.

[Alban Browaeys]

Le jeudi 05 février 2015 à 13:45 -0800, Stephen Boyd a écrit :
> On 02/05/15 13:30, Alban Browaeys wrote:
> >> Thanks for the information. Can you please try the patch in this other
> >> thread[1]? I think you're seeing the same problem.
> >>
> >> [1] https://lkml.org/lkml/2015/2/5/595
> >
> > This fixes both oops : the one from __clk_get and the next from
> > hlist_del.
> >
> 
> Great! Can I take this as your Tested-by?


Tested-by: Alban Browaeys <prahal@yahoo.com>

Well done!