All of lore.kernel.org
 help / color / mirror / Atom feed
From: dave.martin@linaro.org (Dave Martin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 19/19] [INCOMPLETE] ARM: make return_address available for ARM_UNWIND
Date: Mon, 28 Jan 2013 12:50:23 +0000	[thread overview]
Message-ID: <20130128125023.GA2027@linaro.org> (raw)
In-Reply-To: <CA+KhAHYJus7+LsVhNa4RcxfpeTBtV=_vf2YDACtoMujo5c6tXA@mail.gmail.com>

On Mon, Jan 28, 2013 at 11:33:11AM +0900, Keun-O Park wrote:
> Hello guys,
> 
> Could you please review the patch of fixing bug first of returning
> wrong address when using frame pointer?
> I am wondering if the first patch is not delivered to the mailing.

I posted a similar patch to alkml a couple of months ago, but I got
no response and it looks like I forgot about it.

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129381.html

[...]

> 
> ~~~~~~~~~~~~~~~~~~~~~snip~~~~~~~~~~~~~~~~~~~~~~~~~
> From 3a60b536d22a2043d735c890a9aac9e7cb72de8f Mon Sep 17 00:00:00 2001
> From: sahara <keun-o.park@windriver.com>
> Date: Thu, 3 Jan 2013 17:12:37 +0900
> Subject: [PATCH 1/2] arm: fix returning wrong CALLER_ADDRx
> 
> This makes return_address return correct value for ftrace feature.
> unwind_frame does not update frame->lr but frame->pc for backtrace.
> And, the initialization for data.addr was missing so that wrong value
> returned when unwind_frame failed.
> 
> Signed-off-by: sahara <keun-o.park@windriver.com>
> ---
>  arch/arm/kernel/return_address.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kernel/return_address.c b/arch/arm/kernel/return_address.c
> index 8085417..fafedd8 100644
> --- a/arch/arm/kernel/return_address.c
> +++ b/arch/arm/kernel/return_address.c
> @@ -26,7 +26,7 @@ static int save_return_addr(struct stackframe *frame, void *d)
>         struct return_address_data *data = d;
> 
>         if (!data->level) {
> -               data->addr = (void *)frame->lr;
> +               data->addr = (void *)frame->pc;
> 
>                 return 1;
>         } else {
> @@ -41,7 +41,8 @@ void *return_address(unsigned int level)
>         struct stackframe frame;
>         register unsigned long current_sp asm ("sp");
> 
> -       data.level = level + 1;
> +       data.level = level + 2;
> +       data.addr = NULL;

Can you explain why this is needed?  I think I concluded it wasn't
necessary, but you may be right -- I think if walk_stackframe()
fails to unwind the next frame just after data.level reaches zero,
then data.addr can remain unset and return_address() may return
uninitialised garbage.

Initialising data.addr to NULL before we start seems a good way
to avoid that.

Cheers
---Dave

WARNING: multiple messages have this Message-ID (diff)
From: Dave Martin <dave.martin@linaro.org>
To: Keun-O Park <kpark3469@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	linux-arm-kernel@lists.infradead.org,
	Steven Rostedt <rostedt@goodmis.org>,
	sahara <keun-o.park@windriver.com>,
	Russell King <linux@arm.linux.org.uk>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 19/19] [INCOMPLETE] ARM: make return_address available for ARM_UNWIND
Date: Mon, 28 Jan 2013 12:50:23 +0000	[thread overview]
Message-ID: <20130128125023.GA2027@linaro.org> (raw)
In-Reply-To: <CA+KhAHYJus7+LsVhNa4RcxfpeTBtV=_vf2YDACtoMujo5c6tXA@mail.gmail.com>

On Mon, Jan 28, 2013 at 11:33:11AM +0900, Keun-O Park wrote:
> Hello guys,
> 
> Could you please review the patch of fixing bug first of returning
> wrong address when using frame pointer?
> I am wondering if the first patch is not delivered to the mailing.

I posted a similar patch to alkml a couple of months ago, but I got
no response and it looks like I forgot about it.

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129381.html

[...]

> 
> ~~~~~~~~~~~~~~~~~~~~~snip~~~~~~~~~~~~~~~~~~~~~~~~~
> From 3a60b536d22a2043d735c890a9aac9e7cb72de8f Mon Sep 17 00:00:00 2001
> From: sahara <keun-o.park@windriver.com>
> Date: Thu, 3 Jan 2013 17:12:37 +0900
> Subject: [PATCH 1/2] arm: fix returning wrong CALLER_ADDRx
> 
> This makes return_address return correct value for ftrace feature.
> unwind_frame does not update frame->lr but frame->pc for backtrace.
> And, the initialization for data.addr was missing so that wrong value
> returned when unwind_frame failed.
> 
> Signed-off-by: sahara <keun-o.park@windriver.com>
> ---
>  arch/arm/kernel/return_address.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kernel/return_address.c b/arch/arm/kernel/return_address.c
> index 8085417..fafedd8 100644
> --- a/arch/arm/kernel/return_address.c
> +++ b/arch/arm/kernel/return_address.c
> @@ -26,7 +26,7 @@ static int save_return_addr(struct stackframe *frame, void *d)
>         struct return_address_data *data = d;
> 
>         if (!data->level) {
> -               data->addr = (void *)frame->lr;
> +               data->addr = (void *)frame->pc;
> 
>                 return 1;
>         } else {
> @@ -41,7 +41,8 @@ void *return_address(unsigned int level)
>         struct stackframe frame;
>         register unsigned long current_sp asm ("sp");
> 
> -       data.level = level + 1;
> +       data.level = level + 2;
> +       data.addr = NULL;

Can you explain why this is needed?  I think I concluded it wasn't
necessary, but you may be right -- I think if walk_stackframe()
fails to unwind the next frame just after data.level reaches zero,
then data.addr can remain unset and return_address() may return
uninitialised garbage.

Initialising data.addr to NULL before we start seems a good way
to avoid that.

Cheers
---Dave

  reply	other threads:[~2013-01-28 12:50 UTC|newest]

Thread overview: 172+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-25 14:14 [PATCH 00/19] ARM: common warning fixes Arnd Bergmann
2013-01-25 14:14 ` [PATCH 01/19] ARM: shmobile: fix defconfig warning on CONFIG_USB Arnd Bergmann
2013-01-25 14:14 ` [PATCH 02/19] ARM: disable virt_to_bus/virt_to_bus almost everywhere Arnd Bergmann
2013-01-25 14:14 ` [PATCH 03/19] ARM: msm: proc_comm_boot_wait should not be __init Arnd Bergmann
2013-01-25 18:16   ` David Brown
2013-02-12  1:42     ` Olof Johansson
2013-02-12  1:42       ` Olof Johansson
2013-01-25 14:14 ` [PATCH 04/19] oss/dmabuf: use dma_map_single Arnd Bergmann
2013-01-25 14:14   ` Arnd Bergmann
2013-01-25 14:14 ` [PATCH 05/19] sched: warnings in kernel/sched/fair.c Arnd Bergmann
2013-01-25 16:00   ` Paul Turner
2013-01-26 12:17   ` [tip:sched/urgent] sched: Fix warning " tip-bot for Arnd Bergmann
2013-01-25 14:14 ` [PATCH 06/19] sched/debug: fix format string for 32 bit platforms Arnd Bergmann
2013-01-25 16:01   ` Paul Turner
2013-01-26 12:19   ` [tip:sched/urgent] sched/debug: Fix format string for 32-bit platforms tip-bot for Arnd Bergmann
2013-01-25 14:14 ` [PATCH 07/19] scripts/sortextable: silence script output Arnd Bergmann
2013-01-25 23:06   ` David Daney
2013-01-25 14:14 ` [PATCH 08/19] lockdep: avoid warning about unused variables Arnd Bergmann
2013-01-25 14:14 ` [PATCH 09/19] mfd/twl4030: don't warn about uninitialized return code Arnd Bergmann
2013-01-25 14:25   ` Peter Ujfalusi
2013-01-25 14:34     ` Arnd Bergmann
2013-01-25 14:34       ` Arnd Bergmann
2013-01-25 14:35   ` Amit Kucheria
2013-01-25 14:14 ` [PATCH 10/19] watchdog: at91sam9: at91_wdt_dt_ids cannot be __init Arnd Bergmann
2013-01-25 14:14 ` [PATCH 11/19] regmap: avoid undefined return from regmap_read_debugfs Arnd Bergmann
2013-01-26  4:42   ` Mark Brown
2013-01-26  4:52     ` Mark Brown
2013-01-26  4:52       ` Mark Brown
2013-01-26  9:17     ` Arnd Bergmann
2013-01-26  9:17       ` Arnd Bergmann
2013-01-26  9:49       ` Mark Brown
2013-01-26  9:49         ` Mark Brown
2013-01-26  9:59         ` Russell King - ARM Linux
2013-01-26  9:59           ` Russell King - ARM Linux
2013-01-26 10:03           ` Mark Brown
2013-01-26 10:03             ` Mark Brown
2013-01-26 10:07             ` Russell King - ARM Linux
2013-01-26 10:07               ` Russell King - ARM Linux
2013-01-26 11:45               ` [PATCH 11/19] regmap: regmap: avoid spurious warning in regmap_read_debugfs Arnd Bergmann
2013-01-26 11:45                 ` Arnd Bergmann
2013-01-27  2:51                 ` Mark Brown
2013-01-27  2:51                   ` Mark Brown
2013-01-25 14:14 ` [PATCH 12/19] pinctrl: exynos: don't mark probing functions as __init Arnd Bergmann
2013-01-25 17:51   ` Kukjin Kim
2013-01-29 22:08   ` Linus Walleij
2013-01-25 14:14 ` [PATCH 13/19] pinctrl: nomadik: nmk_prcm_gpiocr_get_mode may be unused Arnd Bergmann
2013-01-29 22:11   ` Linus Walleij
     [not found] ` <1359123276-15833-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
2013-01-25 14:14   ` [PATCH 14/19] spi/atmel: remove incorrect __exit_p() Arnd Bergmann
2013-01-25 14:14     ` Arnd Bergmann
     [not found]     ` <1359123276-15833-15-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
2013-02-05 13:34       ` Grant Likely
2013-02-05 13:34         ` Grant Likely
2013-01-25 14:14 ` [PATCH 15/19] sunrpc: don't warn for unused variable 'buf' Arnd Bergmann
2013-01-25 14:14 ` [PATCH 16/19] mac80211: avoid a build warning Arnd Bergmann
2013-01-25 14:17   ` Johannes Berg
2013-01-25 14:14 ` [PATCH 17/19] input/joystick: use get_cycles on ARM Arnd Bergmann
2013-01-25 14:14 ` [PATCH 18/19] ARM: at91: suspend both memory controllers on at91sam9263 Arnd Bergmann
2013-01-25 15:42   ` Jean-Christophe PLAGNIOL-VILLARD
2013-01-25 15:57     ` Arnd Bergmann
2013-01-25 15:57       ` Arnd Bergmann
2013-01-25 14:14 ` [PATCH 19/19] [INCOMPLETE] ARM: make return_address available for ARM_UNWIND Arnd Bergmann
2013-01-25 16:26   ` Dave Martin
2013-01-25 16:44     ` Steven Rostedt
2013-01-25 16:59       ` Dave Martin
2013-01-25 16:59         ` Dave Martin
2013-01-25 17:08         ` Steven Rostedt
2013-01-25 17:08           ` Steven Rostedt
2013-01-25 17:22           ` Dave Martin
2013-01-25 17:22             ` Dave Martin
2013-01-26  0:45         ` Arnd Bergmann
2013-01-26  0:45           ` Arnd Bergmann
2013-01-28  2:33           ` Keun-O Park
2013-01-28  2:33             ` Keun-O Park
2013-01-28 12:50             ` Dave Martin [this message]
2013-01-28 12:50               ` Dave Martin
2013-01-29  2:13               ` Keun-O Park
2013-01-29  2:13                 ` Keun-O Park
2014-01-07 14:33                 ` Arnd Bergmann
2014-01-07 14:33                   ` Arnd Bergmann
2014-01-07 14:41                   ` Russell King - ARM Linux
2014-01-07 14:41                     ` Russell King - ARM Linux
2014-01-07 15:48                     ` Arnd Bergmann
2014-01-07 15:48                       ` Arnd Bergmann
2014-01-07 16:36                       ` Dave Martin
2014-01-07 16:36                         ` Dave Martin
2014-01-07 18:31                         ` Steven Rostedt
2014-01-07 18:31                           ` Steven Rostedt
2013-01-25 22:43 ` [PATCHv2 00/19] ARM: common warning fixes Arnd Bergmann
2013-01-25 22:43   ` Arnd Bergmann
2013-01-25 22:44   ` [PATCH 01/19] ARM: shmobile: fix defconfig warning on CONFIG_USB Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-28  0:21     ` Simon Horman
2013-01-28  0:21       ` Simon Horman
2013-01-28  0:21       ` Simon Horman
2013-01-25 22:44   ` [PATCH 02/19] ARM: disable virt_to_bus/virt_to_bus almost everywhere Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44   ` [PATCH 03/19] ARM: msm: proc_comm_boot_wait should not be __init Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44   ` [PATCH 04/19] oss/dmabuf: use dma_map_single Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44   ` [PATCH 05/19] sched: warnings in kernel/sched/fair.c Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44   ` [PATCH 06/19] sched/debug: fix format string for 32 bit platforms Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44   ` [PATCH 07/19] scripts/sortextable: silence script output Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44   ` [PATCH 08/19] lockdep: avoid warning about unused variables Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44   ` [PATCH 09/19] mfd/twl4030: don't warn about uninitialized return code Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-27  0:42     ` Samuel Ortiz
2013-01-27  0:42       ` Samuel Ortiz
2013-01-25 22:44   ` [PATCH 10/19] watchdog: at91sam9: at91_wdt_dt_ids cannot be __init Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-28  8:32     ` Nicolas Ferre
2013-01-28  8:32       ` Nicolas Ferre
2013-01-28 10:19       ` Fabio Porcedda
2013-01-28 10:19         ` Fabio Porcedda
2013-01-28  9:49     ` Fabio Porcedda
2013-01-28  9:49       ` Fabio Porcedda
2013-01-30 19:31     ` Wim Van Sebroeck
2013-01-25 22:44   ` [PATCH 11/19] regmap: avoid undefined return from regmap_read_debugfs Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44   ` [PATCH 12/19] pinctrl: exynos: don't mark probing functions as __init Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44   ` [PATCH 13/19] pinctrl: nomadik: nmk_prcm_gpiocr_get_mode may be unused Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
     [not found]   ` <1359153858-31992-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
2013-01-25 22:44     ` [PATCH 14/19] spi/atmel: remove incorrect __exit_p() Arnd Bergmann
2013-01-25 22:44       ` Arnd Bergmann
2013-01-25 22:44       ` Arnd Bergmann
     [not found]       ` <1359153858-31992-15-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
2013-01-28  8:33         ` Nicolas Ferre
2013-01-28  8:33           ` Nicolas Ferre
2013-01-28  8:33           ` Nicolas Ferre
2013-01-25 22:44   ` [PATCH 15/19] sunrpc: don't warn for unused variable 'buf' Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 23:04     ` Myklebust, Trond
2013-01-25 23:04       ` Myklebust, Trond
2013-01-25 23:45       ` Arnd Bergmann
2013-01-25 23:45         ` Arnd Bergmann
2013-01-25 23:45         ` Arnd Bergmann
2013-01-26 11:03         ` Russell King - ARM Linux
2013-01-26 11:03           ` Russell King - ARM Linux
2013-01-26 13:34           ` Arnd Bergmann
2013-01-26 13:34             ` Arnd Bergmann
2013-01-26 13:34             ` Arnd Bergmann
2013-01-28 23:18             ` J. Bruce Fields
2013-01-28 23:18               ` J. Bruce Fields
2013-01-25 22:44   ` [PATCH 16/19] ARM: sa1100: don't warn about mach/ide.h Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44   ` [PATCH 17/19] input/joystick: use get_cycles on ARM Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-25 22:44   ` [PATCH 18/19] ARM: at91: suspend both memory controllers on at91sam9263 Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-04-18 13:45     ` Nicolas Ferre
2013-04-18 13:45       ` Nicolas Ferre
2013-04-18 14:15       ` Arnd Bergmann
2013-04-18 14:15         ` Arnd Bergmann
2013-04-18 14:19         ` Nicolas Ferre
2013-04-18 14:19           ` Nicolas Ferre
2013-04-18 14:20           ` Arnd Bergmann
2013-04-18 14:20             ` Arnd Bergmann
2013-04-18 14:32           ` Daniel Lezcano
2013-04-18 14:32             ` Daniel Lezcano
2013-01-25 22:44   ` [PATCH 19/19] [INCOMPLETE] ARM: make return_address available for ARM_UNWIND Arnd Bergmann
2013-01-25 22:44     ` Arnd Bergmann
2013-01-26 10:05   ` [PATCHv2 00/19] ARM: common warning fixes Russell King - ARM Linux
2013-01-26 10:05     ` Russell King - ARM Linux
2013-01-26 13:31     ` Arnd Bergmann
2013-01-26 13:31       ` Arnd Bergmann

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=20130128125023.GA2027@linaro.org \
    --to=dave.martin@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.