linux-kernel - Re: [PATCH] clk: Fix debugfs clk removal before inited

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20150127194701.22722.72853@quantum>
Date:	Tue, 27 Jan 2015 11:47:01 -0800
From:	Mike Turquette <mturquette@...aro.org>
To:	Stephen Boyd <sboyd@...eaurora.org>,
	"Srinivas Kandagatla" <srinivas.kandagatla@...aro.org>
Cc:	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] clk: Fix debugfs clk removal before inited

Quoting Stephen Boyd (2015-01-19 11:44:35)
> On 01/19/2015 01:57 AM, Srinivas Kandagatla wrote:
> > Some of the clks can be registered & unregistered before the clk related debugfs
> > entries are initialized at late_initcall. In the unregister path checking for only
> > dentry before clk_debug_init() would lead dangling pointers in the debug clk list,
> > because the list is already populated in register path and the clk pointer freed in
> > unregister path.
> > The side effect of not removing it from the list is either a null pointer
> > dereference or if lucky to boot the system, the number of clk entries in
> > debugfs disappear.
> >
> > We could add more checks like if (inited && !clk->dentry) but just removing
> > the check for dentry made more sense as debugfs_remove_recursive() seems to be
> > safe with null pointers. This will ensure that the unregistering clk would be
> > removed from the debug list in all the code paths.
> >
> > Without this patch kernel would crash with log:
> > Unable to handle kernel NULL pointer dereference at virtual address 00000000
> > pgd = c0204000
> > [00000000] *pgd=00000000
> > Internal error: Oops: 5 [#1] SMP ARM
> > Modules linked in:
> > CPU: 1 PID: 1 Comm: swapper/0 Tainted: G    B          3.19.0-rc3-00007-g412f9ba-dirty #840
> > Hardware name: Qualcomm (Flattened Device Tree)
> > task: ed948000 ti: ed944000 task.ti: ed944000
> > PC is at strlen+0xc/0x40
> > LR is at __create_file+0x64/0x1dc
> > pc : [<c04ee604>]    lr : [<c049f1c4>]    psr: 60000013
> > sp : ed945e40  ip : ed945e50  fp : ed945e4c
> > r10: 00000000  r9 : c1006094  r8 : 00000000
> > r7 : 000041ed  r6 : 00000000  r5 : ed4af998  r4 : c11b5e28
> > r3 : 00000000  r2 : ed945e38  r1 : a0000013  r0 : 00000000
> > Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> > Control: 10c5787d  Table: 8020406a  DAC: 00000015
> > Process swapper/0 (pid: 1, stack limit = 0xed944248)
> > Stack: (0xed945e40 to 0xed946000)
> > 5e40: ed945e7c ed945e50 c049f1c4 c04ee604 c0fc2fa4 00000000 ecb748c0 c11c2b80
> > 5e60: c0beec04 0000011c c0fc2fa4 00000000 ed945e94 ed945e80 c049f3e0 c049f16c
> > 5e80: 00000000 00000000 ed945eac ed945e98 c08cbc50 c049f3c0 ecb748c0 c11c2b80
> > 5ea0: ed945ed4 ed945eb0 c0fc3080 c08cbc30 c0beec04 c107e1d8 ecdf0600 c107e1d8
> > 5ec0: c107e1d8 ecdf0600 ed945f54 ed945ed8 c0208ed4 c0fc2fb0 c026a784 c04ee628
> > 5ee0: ed945f0c ed945ef0 c0f5d600 c04ee604 c0f5d5ec ef7fcc7d c0b40ecc 0000011c
> > 5f00: ed945f54 ed945f10 c026a994 c0f5d5f8 c04ecc00 00000007 ef7fcc95 00000007
> > 5f20: c0e90744 c0dd0884 ed945f54 c106cde0 00000007 c117f8c0 0000011c c0f5d5ec
> > 5f40: c1006094 c100609c ed945f94 ed945f58 c0f5de34 c0208e50 00000007 00000007
> > 5f60: c0f5d5ec be9b5ae0 00000000 c117f8c0 c0af1680 00000000 00000000 00000000
> > 5f80: 00000000 00000000 ed945fac ed945f98 c0af169c c0f5dd2c ed944000 00000000
> > 5fa0: 00000000 ed945fb0 c020f298 c0af168c 00000000 00000000 00000000 00000000
> > 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ebcc6d33 bfffca73
> > [<c04ee604>] (strlen) from [<c049f1c4>] (__create_file+0x64/0x1dc)
> > [<c049f1c4>] (__create_file) from [<c049f3e0>] (debugfs_create_dir+0x2c/0x34)
> > [<c049f3e0>] (debugfs_create_dir) from [<c08cbc50>] (clk_debug_create_one+0x2c/0x16c)
> > [<c08cbc50>] (clk_debug_create_one) from [<c0fc3080>] (clk_debug_init+0xdc/0x144)
> > [<c0fc3080>] (clk_debug_init) from [<c0208ed4>] (do_one_initcall+0x90/0x1e0)
> > [<c0208ed4>] (do_one_initcall) from [<c0f5de34>] (kernel_init_freeable+0x114/0x1e0)
> > [<c0f5de34>] (kernel_init_freeable) from [<c0af169c>] (kernel_init+0x1c/0xfc)
> > [<c0af169c>] (kernel_init) from [<c020f298>] (ret_from_fork+0x14/0x3c)
> > Code: c0b40ecc e1a0c00d e92dd800 e24cb004 (e5d02000)
> > ---[ end trace b940e45b5e25c1e7 ]---
> >
> > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> >
> 
> Reviewed-by: Stephen Boyd <sboyd@...eaurora.org>
> Fixes: 6314b6796e3c "clk: Don't hold prepare_lock across debugfs creation"

Applied to clk-next.

Regards,
Mike

> 
> -- 
> 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/