Messages in this thread | | | Date | Tue, 5 Dec 2017 15:28:44 -0800 | From | Stephen Boyd <> | Subject | Re: [PATCH] clk: fix a panic error caused by accessing NULL pointer |
| |
On 11/21, Chunyan Zhang wrote: > On 21 November 2017 at 16:57, Chunyan Zhang <zhang.lyra@gmail.com> wrote: > > On 21 November 2017 at 03:12, Stephen Boyd <sboyd@codeaurora.org> wrote: > >> On 11/20, Chunyan Zhang wrote: > >>> From: Cai Li <cai.li@spreadtrum.com> > >>> > >>> In some cases the clock parent would be set NULL when doing re-parent, > >>> it will cause a NULL pointer accessing if clk_set trace event is enabled, > >>> since the trace event function would not check the input parameter. > >>> > >>> Signed-off-by: Cai Li <cai.li@spreadtrum.com> > >>> Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com> > >> > >> Fixes: tag? > >> > >>> --- > >>> drivers/clk/clk.c | 9 ++++----- > >>> 1 file changed, 4 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > >>> index c8d83ac..64efaf0 100644 > >>> --- a/drivers/clk/clk.c > >>> +++ b/drivers/clk/clk.c > >>> @@ -1242,13 +1242,12 @@ static int __clk_set_parent(struct clk_core *core, struct clk_core *parent, > >>> > >>> old_parent = __clk_set_parent_before(core, parent); > >>> > >>> - trace_clk_set_parent(core, parent); > >>> - > >>> /* change clock input source */ > >>> - if (parent && core->ops->set_parent) > >>> + if (parent && core->ops->set_parent) { > >>> + trace_clk_set_parent(core, parent); > >>> ret = core->ops->set_parent(core->hw, p_index); > >>> - > >>> - trace_clk_set_parent_complete(core, parent); > >>> + trace_clk_set_parent_complete(core, parent); > >>> + } > >> > >> Is the problem that parent may be NULL and the tracepoint > >> dereferences it? > > > > Yes, I think that probably is uncommon usage though, it revealed that > > the tracepoint could be stronger :) > > The reason we need to set the parent as NULL is to disable the clk for > the purpose of saving power.
Do you have drivers calling set_parent with NULL to save power? Seems sort of odd. Why not do something when disabling the clk in clk_disable() path instead? Either way, I'll apply the patch to clk-fixes.
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
| |