Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
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 Well done! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
> > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
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
>>> ---
>>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
Le jeudi 05 février 2015 à 11:30 -0800, Stephen Boyd a écrit :
> > Signed-off-by: Alban Browaeys
> > ---
> > 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=
[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 : []lr : []psr: 200d0053
[7.318722] sp : eb0bdbb0 ip : eb0bdbc8 fp : eb0bdbc4
[7.330187] r10: 0006 r9 : 0001 r8 :
[7.335398] r7 : eb0bdbf8 r6 : r5 : ee5c7d80 r4 : 6b6b6b6b
[7.341913] r3 : 0001 r2 : 0011 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: 0015
[7.341931] Process systemd-modules (pid: 98, stack limit = 0xeb0bc218)
[7.341934] Stack: (0xeb0bdbb0 to 0xeb0be000)
[7.341939] dba0: 0001 ee0ff600
eb0bdbdc eb0bdbc8
[7.341945] dbc0: c055231c c0556074 0001 ed834850 eb0bdc6c eb0bdbe0
c0558528 c05522f0
[7.341950] dbe0: eb0bdbf8 c01cc560 eb3e4710 ee2b4200 eb0bdc14 c01ced00
ee5e0d3c 0001
[7.341956] dc00: 0011 ee2b4200 eb0bdc34 eb3e4900 c08c5790
ee2b4200 eb3e4700
[7.341962] dc20: 0001 0006 eb0bdc5c eb0bdc38 c01ced00 c01cb2d0
ed834850
[7.341968] dc40: ed834858 ed834850 ed834850 bf06c0b4 c0aa82b8
bf06c0b4 0006
[7.341974] dc60: eb0bdc8c eb0bdc70 c044213c c0558474 ed834850 c0b61248
c0b61254 c0aa82b8
[7.341979] dc80: eb0bdcc4 eb0bdc90 c043ff34 c0442120 bf0631f0
e9c3b700 ed834850
[7.341985] dca0: bf06c0b4 ed834884 bf0631f0 e9c3b700 c0a4f40c
eb0bdce4 eb0bdcc8
[7.341991] dcc0: c044020c c043fdc8 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 bf0631e8 bf06c388
eb0bdd5c eb0bdd48
[7.342014] dd40: c0440bfc c043f370 0001 eb0bdd6c eb0bdd60
c0442094 c0440b50
[7.342020] dd60: eb0bddbc eb0bdd70 bf04ca08 c044203c bf065090
[7.342026] dd80:
c0a53b20 bf06c208
[7.342031] dda0: c0a53b20 bf04c950 c0a53b20 eb0bde4c eb0bddc0
c0008b28 bf04c95c
[7.342037] ddc0: 001f eb0bddec eb0bddd8 c00504f4 c006dde0
eb0bc000
[7.342043] dde0: ee002140 00d0 c06ed170 000c c0a50600
eb0bde4c eb0bde08
[7.342049] de00: c01541d8 c0153934 eb0bc008 eb0bde08 eb0bc008
ee002140 dc8cb100
[7.342055] de20: 0001 bf06c208 0001 e9c3b600 e9c3bb00 0001
163c451c e9c3bb08
[7.342060] de40: eb0bde74 eb0bde50 c06ed1ac c00089ec eb0bde74 eb0bde60
c014496c eb0bdf48
[7.342066] de60: 0001 bf06c208 eb0bdf3c eb0bde78 c00af61c c06ed148
bf06c214
Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
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
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
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
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
