linux-kernel - Re: [PATCH v2 3/3] clk: Replace explicit clk assignment with __clk_hw_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <54DD051D.8020701@codeaurora.org>
Date:	Thu, 12 Feb 2015 11:55:09 -0800
From:	Stephen Boyd <sboyd@...eaurora.org>
To:	Javier Martinez Canillas <javier.martinez@...labora.co.uk>,
	Mike Turquette <mturquette@...aro.org>
CC:	Tomeu Vizoso <tomeu.vizoso@...labora.com>,
	Emilio López 
	<emilio@...pez.com.ar>,
	Peter De Schrijver <pdeschrijver@...dia.com>,
	Robert Jarzmik <robert.jarzmik@...e.fr>,
	Giuseppe Cavallaro <peppe.cavallaro@...com>,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] clk: Replace explicit clk assignment with __clk_hw_set_clk

On 02/12/15 05:58, Javier Martinez Canillas wrote:
> The change in the clk API to return a per-user clock instance, moved
> the clock state to struct clk_core so now the struct clk_hw .core field
> is used instead of .clk for most operations.
>
> So for hardware clocks that needs to share the same clock state, both
> the .core and .clk pointers have to be assigned but currently only the
> .clk is set. This leads to NULL pointer dereference when the operations
> try to access the hw clock .core. For example, the composite clock rate
> and mux components didn't have a .core set which leads to this error:
>
> Unable to handle kernel NULL pointer dereference at virtual address 00000034
> pgd = c0004000
> [00000034] *pgd=00000000
> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-next-20150211-00002-g1fb7f0e1150d #423
> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> task: ee480000 ti: ee488000 task.ti: ee488000
> PC is at clk_mux_determine_rate_flags+0x14/0x19c
> LR is at __clk_mux_determine_rate+0x24/0x2c
> pc : [<c03a355c>]    lr : [<c03a3734>]    psr: a0000113
> sp : ee489ce8  ip : ee489d84  fp : ee489d84
> r10: 0000005c  r9 : 00000001  r8 : 016e3600
> r7 : 00000000  r6 : 00000000  r5 : ee442200  r4 : ee440c98
> r3 : ffffffff  r2 : 00000000  r1 : 016e3600  r0 : ee440c98
> Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> Control: 10c5387d  Table: 4000406a  DAC: 00000015
> Process swapper/0 (pid: 1, stack limit = 0xee488210)
> Stack: (0xee489ce8 to 0xee48a000)
> 9ce0:                   00000000 ffffffff 60000113 ee440c98 ee442200 00000000
> 9d00: 016e3600 ffffffff 00000001 0000005c ee489d84 c03a3734 ee489d80 ee489d84
> 9d20: 00000000 c048b130 00000400 c03a5798 ee489d80 ee489d84 c0607f60 ffffffea
> 9d40: 00000001 00000001 ee489d5c c003f844 c06e3340 ee402680 ee440d0c ed935000
> 9d60: 016e3600 00000003 00000001 0000005c eded3700 c03a11a0 ee489d80 ee489d84
> 9d80: 016e3600 ee402680 c05b413a eddc9900 016e3600 c03a1228 00000000 ffffffff
> 9da0: ffffffff eddc9900 016e3600 c03a1c1c ffffffff 016e3600 ed8c6710 c03d6ce4
> 9dc0: eded3400 00000000 00000000 c03c797c 00000001 0000005c eded3700 eded3700
> 9de0: 000005e0 00000001 0000005c c03db8ac c06e7e54 c03c8f08 00000000 c06e7e64
> 9e00: c06b6e74 c06e7f64 000005e0 c06e7df8 c06e5100 00000000 c06e7e6c c06e7f54
> 9e20: 00000000 00000000 eebd9550 00000000 c06e7da0 c06e7e54 ee7b5010 c06e7da0
> 9e40: eddc9690 c06e7db4 c06b6e74 00000097 00000000 c03d4398 00000000 ee7b5010
> 9e60: eebd9550 c06e7da0 00000000 c03db824 ee7b5010 fffffffe c06e7db4 c0299c7c
> 9e80: ee7b5010 c072a05c 00000000 c0298858 ee7b5010 c06e7db4 ee7b5044 00000000
> 9ea0: eddc9580 c0298a04 c06e7db4 00000000 c0298978 c02971d4 ee405c78 ee732b40
> 9ec0: c06e7db4 eded3800 c06d6738 c0298044 c0608300 c06e7db4 00000000 c06e7db4
> 9ee0: 00000000 c06beb58 c06beb58 c0299024 00000000 c068dd00 00000000 c0008944
> 9f00: 00000038 c049013c ee462200 c0711920 ee480000 60000113 c06c2cb0 00000000
> 9f20: 00000000 c06c2cb0 60000113 00000000 ef7fcafc 00000000 c0640194 c00389ec
> 9f40: c05ec3a8 c063f824 00000006 00000006 c06c2c50 c0696444 00000006 c0696424
> 9f60: c06ee1c0 c066b588 c06b6e74 00000097 00000000 c066bd44 00000006 00000006
> 9f80: c066b588 c003d684 00000000 c0481938 00000000 00000000 00000000 00000000
> 9fa0: 00000000 c0481940 00000000 c000e680 00000000 00000000 00000000 00000000
> 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
> [<c03a355c>] (clk_mux_determine_rate_flags) from [<c03a3734>] (__clk_mux_determine_rate+0x24/0x2c)
> [<c03a3734>] (__clk_mux_determine_rate) from [<c03a5798>] (clk_composite_determine_rate+0xbc/0x238)
> [<c03a5798>] (clk_composite_determine_rate) from [<c03a11a0>] (clk_core_round_rate_nolock+0x5c/0x9c)
> [<c03a11a0>] (clk_core_round_rate_nolock) from [<c03a1228>] (__clk_round_rate+0x38/0x40)
> [<c03a1228>] (__clk_round_rate) from [<c03a1c1c>] (clk_round_rate+0x20/0x38)
> [<c03a1c1c>] (clk_round_rate) from [<c03d6ce4>] (max98090_dai_set_sysclk+0x34/0x118)
> [<c03d6ce4>] (max98090_dai_set_sysclk) from [<c03c797c>] (snd_soc_dai_set_sysclk+0x38/0x80)
> [<c03c797c>] (snd_soc_dai_set_sysclk) from [<c03db8ac>] (snow_late_probe+0x24/0x48)
> [<c03db8ac>] (snow_late_probe) from [<c03c8f08>] (snd_soc_register_card+0xf04/0x1070)
> [<c03c8f08>] (snd_soc_register_card) from [<c03d4398>] (devm_snd_soc_register_card+0x30/0x64)
> [<c03d4398>] (devm_snd_soc_register_card) from [<c03db824>] (snow_probe+0x68/0xcc)
> [<c03db824>] (snow_probe) from [<c0299c7c>] (platform_drv_probe+0x48/0x98)
> [<c0299c7c>] (platform_drv_probe) from [<c0298858>] (driver_probe_device+0x114/0x234)
> [<c0298858>] (driver_probe_device) from [<c0298a04>] (__driver_attach+0x8c/0x90)
> [<c0298a04>] (__driver_attach) from [<c02971d4>] (bus_for_each_dev+0x54/0x88)
> [<c02971d4>] (bus_for_each_dev) from [<c0298044>] (bus_add_driver+0xd8/0x1cc)
> [<c0298044>] (bus_add_driver) from [<c0299024>] (driver_register+0x78/0xf4)
> [<c0299024>] (driver_register) from [<c0008944>] (do_one_initcall+0x80/0x1d0)
> [<c0008944>] (do_one_initcall) from [<c066bd44>] (kernel_init_freeable+0x10c/0x1d8)
> [<c066bd44>] (kernel_init_freeable) from [<c0481940>] (kernel_init+0x8/0xe4)
> [<c0481940>] (kernel_init) from [<c000e680>] (ret_from_fork+0x14/0x34)
> Code: e24dd00c e5907000 e1a08001 e88d000c (e5970034)
>
> The changes were made using the following cocinelle semantic patch:
>
> @i@
> @@
>
> @depends on i@
> identifier dst;
> @@
>
> - dst->clk = hw->clk;
> + __clk_hw_set_clk(dst, hw);
>
> @depends on i@
> identifier dst;
> @@
>
> - dst->hw.clk = hw->clk;
> + __clk_hw_set_clk(&dst->hw, hw);
>
> Fixes: 035a61c314eb3 ("clk: Make clk API return per-user struct clk instances")
> Signed-off-by: Javier Martinez Canillas <javier.martinez@...labora.co.uk>

Reviewed-by: Stephen Boyd <sboyd@...eaurora.org>

Did you run this on all files that include clk-provider.h? I hope there
aren't similar situations in arch/arm/ for example.

-- 
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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/