Messages in this thread | | | From | Martin Blumenstingl <> | Date | Fri, 16 Aug 2019 08:48:08 +0200 | Subject | Re: [PATCH RFC v1] clk: Fix potential NULL dereference in clk_fetch_parent_index() |
| |
Hi Stephen,
On Fri, Aug 16, 2019 at 1:29 AM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Martin Blumenstingl (2019-08-15 15:31:55) > > Don't compare the parent clock name with a NULL name in the > > clk_parent_map. This prevents a kernel crash when passing NULL > > core->parents[i].name to strcmp(). > > > > An example which triggered this is a mux clock with four parents when > > each of them is referenced in the clock driver using > > clk_parent_data.fw_name and then calling clk_set_parent(clk, 3rd_parent) > > on this mux. > > In this case the first parent is also the HW default so > > core->parents[i].hw is populated when the clock is registered. Calling > > clk_set_parent(clk, 3rd_parent) will then go through all parents and > > skip the first parent because it's hw pointer doesn't match. For the > > second parent no hw pointer is cached yet and clk_core_get(core, 1) > > returns a non-matching pointer (which is correct because we are comparing > > the second with the third parent). Comparing the result of > > clk_core_get(core, 2) with the requested parent gives a match. However > > we don't reach this point because right after the clk_core_get(core, 1) > > mismatch the old code tried to !strcmp(parent->name, NULL) (where the > > second argument is actually core->parents[i].name, but that was never > > populated by the clock driver). > > > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > > --- > > I have seen the original crash when I was testing an MMC driver which > > is not upstream yet on v5.3-rc4. I'm not sure whether this fix is > > "correct" (it fixes the crash for me) or where to point the Fixes tag > > to, it may be one of: > > - fc0c209c147f ("clk: Allow parents to be specified without string names") > > - 1a079560b145 ("clk: Cache core in clk_fetch_parent_index() without names") > > > > This is meant to be applied on top of v5.3-rc4. > > > > Ah ok. I thought that strcmp() would ignore NULL arguments, but > apparently not. I can apply this to clk-fixes. at least ARM [0] and the generic [1] implementations don't
I did not bisect this so do you have any suggestion for a Fixes tag? I mentioned two candidates above, but I'm not sure which one to use just let me know, then I'll resend with the fixes tag so you can take it through clk-fixes
Martin
[0] https://elixir.bootlin.com/linux/v5.2/source/arch/arm/boot/compressed/string.c#L91 [1] https://elixir.bootlin.com/linux/v5.2/source/lib/string.c#L356
| |