All of lore.kernel.org
 help / color / mirror / Atom feed
From: sboyd@codeaurora.org (Stephen Boyd)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm: kgdb: Don't try to stop the machine when setting breakpoints
Date: Tue, 25 Aug 2015 15:26:26 -0700	[thread overview]
Message-ID: <20150825222626.GK19120@codeaurora.org> (raw)
In-Reply-To: <1440540165-28875-1-git-send-email-dianders@chromium.org>

On 08/25, Douglas Anderson wrote:
> In (23a4e40 arm: kgdb: Handle read-only text / modules) we moved to
> using patch_text() to set breakpoints so that we could handle the case
> when we had CONFIG_DEBUG_RODATA.  That patch used patch_text().
> Unfortunately, patch_text() assumes that we're not in atomic context
> when it runs since it needs to grab a mutex and also wait for other
> CPUs to stop (which it does with a completion).
> 
> This would result in a stack crawl if you had
> CONFIG_DEBUG_ATOMIC_SLEEP and tried to set a breakpoint in kgdb.  The
> crawl looked something like:
> 
>  BUG: scheduling while atomic: swapper/0/0/0x00010007
>  CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc7-00133-geb63b34 #1073
>  Hardware name: Rockchip (Device Tree)
>   (unwind_backtrace) from [<c00133d4>] (show_stack+0x20/0x24)
>   (show_stack) from [<c05400e8>] (dump_stack+0x84/0xb8)
>   (dump_stack) from [<c004913c>] (__schedule_bug+0x54/0x6c)
>   (__schedule_bug) from [<c054065c>] (__schedule+0x80/0x668)
>   (__schedule) from [<c0540cfc>] (schedule+0xb8/0xd4)
>   (schedule) from [<c0543a3c>] (schedule_timeout+0x2c/0x234)
>   (schedule_timeout) from [<c05417c0>] (wait_for_common+0xf4/0x188)
>   (wait_for_common) from [<c0541874>] (wait_for_completion+0x20/0x24)
>   (wait_for_completion) from [<c00a0104>] (__stop_cpus+0x58/0x70)
>   (__stop_cpus) from [<c00a0580>] (stop_cpus+0x3c/0x54)
>   (stop_cpus) from [<c00a06c4>] (__stop_machine+0xcc/0xe8)
>   (__stop_machine) from [<c00a0714>] (stop_machine+0x34/0x44)
>   (stop_machine) from [<c00173e8>] (patch_text+0x28/0x34)
>   (patch_text) from [<c001733c>] (kgdb_arch_set_breakpoint+0x40/0x4c)
>   (kgdb_arch_set_breakpoint) from [<c00a0d68>] (kgdb_validate_break_address+0x2c/0x60)
>   (kgdb_validate_break_address) from [<c00a0e90>] (dbg_set_sw_break+0x1c/0xdc)
>   (dbg_set_sw_break) from [<c00a2e88>] (gdb_serial_stub+0x9c4/0xba4)
>   (gdb_serial_stub) from [<c00a11cc>] (kgdb_cpu_enter+0x1f8/0x60c)
>   (kgdb_cpu_enter) from [<c00a18cc>] (kgdb_handle_exception+0x19c/0x1d0)
>   (kgdb_handle_exception) from [<c0016f7c>] (kgdb_compiled_brk_fn+0x30/0x3c)
>   (kgdb_compiled_brk_fn) from [<c00091a4>] (do_undefinstr+0x1a4/0x20c)
>   (do_undefinstr) from [<c001400c>] (__und_svc_finish+0x0/0x34)
> 
> It turns out that when we're in kgdb all the CPUs are stopped anyway
> so there's no reason we should be calling patch_text().  We can
> instead directly call __patch_text() which assumes that CPUs have
> already been stopped.
> 
> Fixes: 23a4e4050ba9 ("arm: kgdb: Handle read-only text / modules")
> Reported-by: Aapo Vienamo <avienamo@nvidia.com>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

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

-- 
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: Stephen Boyd <sboyd@codeaurora.org>
To: Douglas Anderson <dianders@chromium.org>
Cc: Kees Cook <keescook@chromium.org>,
	Nicolas Pitre <nico@linaro.org>,
	Aapo Vienamo <avienamo@nvidia.com>,
	linux@arm.linux.org.uk, masami.hiramatsu.pt@hitachi.com,
	wangnan0@huawei.com, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm: kgdb: Don't try to stop the machine when setting breakpoints
Date: Tue, 25 Aug 2015 15:26:26 -0700	[thread overview]
Message-ID: <20150825222626.GK19120@codeaurora.org> (raw)
In-Reply-To: <1440540165-28875-1-git-send-email-dianders@chromium.org>

On 08/25, Douglas Anderson wrote:
> In (23a4e40 arm: kgdb: Handle read-only text / modules) we moved to
> using patch_text() to set breakpoints so that we could handle the case
> when we had CONFIG_DEBUG_RODATA.  That patch used patch_text().
> Unfortunately, patch_text() assumes that we're not in atomic context
> when it runs since it needs to grab a mutex and also wait for other
> CPUs to stop (which it does with a completion).
> 
> This would result in a stack crawl if you had
> CONFIG_DEBUG_ATOMIC_SLEEP and tried to set a breakpoint in kgdb.  The
> crawl looked something like:
> 
>  BUG: scheduling while atomic: swapper/0/0/0x00010007
>  CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc7-00133-geb63b34 #1073
>  Hardware name: Rockchip (Device Tree)
>   (unwind_backtrace) from [<c00133d4>] (show_stack+0x20/0x24)
>   (show_stack) from [<c05400e8>] (dump_stack+0x84/0xb8)
>   (dump_stack) from [<c004913c>] (__schedule_bug+0x54/0x6c)
>   (__schedule_bug) from [<c054065c>] (__schedule+0x80/0x668)
>   (__schedule) from [<c0540cfc>] (schedule+0xb8/0xd4)
>   (schedule) from [<c0543a3c>] (schedule_timeout+0x2c/0x234)
>   (schedule_timeout) from [<c05417c0>] (wait_for_common+0xf4/0x188)
>   (wait_for_common) from [<c0541874>] (wait_for_completion+0x20/0x24)
>   (wait_for_completion) from [<c00a0104>] (__stop_cpus+0x58/0x70)
>   (__stop_cpus) from [<c00a0580>] (stop_cpus+0x3c/0x54)
>   (stop_cpus) from [<c00a06c4>] (__stop_machine+0xcc/0xe8)
>   (__stop_machine) from [<c00a0714>] (stop_machine+0x34/0x44)
>   (stop_machine) from [<c00173e8>] (patch_text+0x28/0x34)
>   (patch_text) from [<c001733c>] (kgdb_arch_set_breakpoint+0x40/0x4c)
>   (kgdb_arch_set_breakpoint) from [<c00a0d68>] (kgdb_validate_break_address+0x2c/0x60)
>   (kgdb_validate_break_address) from [<c00a0e90>] (dbg_set_sw_break+0x1c/0xdc)
>   (dbg_set_sw_break) from [<c00a2e88>] (gdb_serial_stub+0x9c4/0xba4)
>   (gdb_serial_stub) from [<c00a11cc>] (kgdb_cpu_enter+0x1f8/0x60c)
>   (kgdb_cpu_enter) from [<c00a18cc>] (kgdb_handle_exception+0x19c/0x1d0)
>   (kgdb_handle_exception) from [<c0016f7c>] (kgdb_compiled_brk_fn+0x30/0x3c)
>   (kgdb_compiled_brk_fn) from [<c00091a4>] (do_undefinstr+0x1a4/0x20c)
>   (do_undefinstr) from [<c001400c>] (__und_svc_finish+0x0/0x34)
> 
> It turns out that when we're in kgdb all the CPUs are stopped anyway
> so there's no reason we should be calling patch_text().  We can
> instead directly call __patch_text() which assumes that CPUs have
> already been stopped.
> 
> Fixes: 23a4e4050ba9 ("arm: kgdb: Handle read-only text / modules")
> Reported-by: Aapo Vienamo <avienamo@nvidia.com>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

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

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

  reply	other threads:[~2015-08-25 22:26 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-25 22:02 [PATCH] arm: kgdb: Don't try to stop the machine when setting breakpoints Douglas Anderson
2015-08-25 22:02 ` Douglas Anderson
2015-08-25 22:26 ` Stephen Boyd [this message]
2015-08-25 22:26   ` Stephen Boyd
2015-08-26  0:32 ` Kees Cook
2015-08-26  0:32   ` Kees Cook

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=20150825222626.GK19120@codeaurora.org \
    --to=sboyd@codeaurora.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.