From: mturquette@linaro.org (Mike Turquette)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clk: Fix debugfs clk removal before inited
Date: Tue, 27 Jan 2015 11:47:01 -0800 [thread overview]
Message-ID: <20150127194701.22722.72853@quantum> (raw)
In-Reply-To: <54BD5EA3.6070106@codeaurora.org>
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@linaro.org>
> >
>
> Reviewed-by: Stephen Boyd <sboyd@codeaurora.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
>
WARNING: multiple messages have this Message-ID (diff)
From: Mike Turquette <mturquette@linaro.org>
To: Stephen Boyd <sboyd@codeaurora.org>,
"Srinivas Kandagatla" <srinivas.kandagatla@linaro.org>
Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] clk: Fix debugfs clk removal before inited
Date: Tue, 27 Jan 2015 11:47:01 -0800 [thread overview]
Message-ID: <20150127194701.22722.72853@quantum> (raw)
In-Reply-To: <54BD5EA3.6070106@codeaurora.org>
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@linaro.org>
> >
>
> Reviewed-by: Stephen Boyd <sboyd@codeaurora.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
>
next prev parent reply other threads:[~2015-01-27 19:47 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-19 9:57 [PATCH] clk: Fix debugfs clk removal before inited Srinivas Kandagatla
2015-01-19 9:57 ` Srinivas Kandagatla
2015-01-19 19:44 ` Stephen Boyd
2015-01-19 19:44 ` Stephen Boyd
2015-01-27 19:47 ` Mike Turquette [this message]
2015-01-27 19:47 ` Mike Turquette
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150127194701.22722.72853@quantum \
--to=mturquette@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.