Re: [U-Boot] [PATCH] watchdog: Move watchdog_dev to data section (BSS may not be cleared)
On 02.04.19 12:30, Michal Simek wrote: On 02. 04. 19 11:57, Stefan Roese wrote: This patch moves all instances of static "watchdog_dev" declarations to the "data" section. This may be needed, as the BSS may not be cleared in the early U-Boot phase, where watchdog_reset() is already beeing called. This may result in incorrect pointer access, as the check to "!watchdog_dev" in watchdog_reset() may not be true and the function may continue to run. Signed-off-by: Stefan Roese Cc: Heiko Schocher Cc: Tom Rini Cc: Michal Simek Cc: "Marek Behún" Cc: Daniel Schwierzeck --- arch/mips/mach-mt7620/cpu.c | 2 +- board/CZ.NIC/turris_mox/turris_mox.c | 2 +- board/CZ.NIC/turris_omnia/turris_omnia.c | 2 +- board/xilinx/microblaze-generic/microblaze-generic.c | 2 +- board/xilinx/zynq/board.c| 2 +- board/xilinx/zynqmp/zynqmp.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/mips/mach-mt7620/cpu.c b/arch/mips/mach-mt7620/cpu.c index 9e0ca716f7..fe74f26a54 100644 --- a/arch/mips/mach-mt7620/cpu.c +++ b/arch/mips/mach-mt7620/cpu.c @@ -70,7 +70,7 @@ int print_cpuinfo(void) } #ifdef CONFIG_WATCHDOG -static struct udevice *watchdog_dev; +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; /* Called by macro WATCHDOG_RESET */ void watchdog_reset(void) diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index 65d50a92dd..96cb9c7e5c 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -120,7 +120,7 @@ int board_fix_fdt(void *blob) #endif #ifdef CONFIG_WDT_ARMADA_37XX -static struct udevice *watchdog_dev; +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; void watchdog_reset(void) { diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index 1f7650cb36..c7f6479a0c 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -365,7 +365,7 @@ static bool disable_mcu_watchdog(void) #endif #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT_ORION) -static struct udevice *watchdog_dev = NULL; +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; #endif int board_init(void) diff --git a/board/xilinx/microblaze-generic/microblaze-generic.c b/board/xilinx/microblaze-generic/microblaze-generic.c index c946ec3208..28c9efa3a2 100644 --- a/board/xilinx/microblaze-generic/microblaze-generic.c +++ b/board/xilinx/microblaze-generic/microblaze-generic.c @@ -25,7 +25,7 @@ DECLARE_GLOBAL_DATA_PTR; #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) -static struct udevice *watchdog_dev; +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; #endif /* !CONFIG_SPL_BUILD && CONFIG_WDT */ ulong ram_base; diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c index b4b84df576..7c8c32a1c7 100644 --- a/board/xilinx/zynq/board.c +++ b/board/xilinx/zynq/board.c @@ -19,7 +19,7 @@ DECLARE_GLOBAL_DATA_PTR; #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) -static struct udevice *watchdog_dev; +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; #endif #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_BOARD_EARLY_INIT_F) diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 5e1d2116bc..1c2aa99b97 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -25,7 +25,7 @@ DECLARE_GLOBAL_DATA_PTR; #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) -static struct udevice *watchdog_dev; +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; #endif #if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_ZYNQMPPL) && \ If you want to do it for all then please remove the same from this file http://git.denx.de/?p=u-boot.git;a=commitdiff;h=4490e013ee4f2a4b9b6ca9224221fed2788b6940 +#if defined(CONFIG_WDT) && !defined(CONFIG_SPL_BUILD) + /* bss is not cleared at time when watchdog_reset() is called */ + watchdog_dev = NULL; +#endif And also from zynq file. board/xilinx/zynq/board.c Thanks for spotting. I'll remove it in v2. Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] watchdog: Move watchdog_dev to data section (BSS may not be cleared)
On 02. 04. 19 11:57, Stefan Roese wrote: > This patch moves all instances of static "watchdog_dev" declarations to > the "data" section. This may be needed, as the BSS may not be cleared > in the early U-Boot phase, where watchdog_reset() is already beeing > called. This may result in incorrect pointer access, as the check to > "!watchdog_dev" in watchdog_reset() may not be true and the function > may continue to run. > > Signed-off-by: Stefan Roese > Cc: Heiko Schocher > Cc: Tom Rini > Cc: Michal Simek > Cc: "Marek Behún" > Cc: Daniel Schwierzeck > --- > arch/mips/mach-mt7620/cpu.c | 2 +- > board/CZ.NIC/turris_mox/turris_mox.c | 2 +- > board/CZ.NIC/turris_omnia/turris_omnia.c | 2 +- > board/xilinx/microblaze-generic/microblaze-generic.c | 2 +- > board/xilinx/zynq/board.c| 2 +- > board/xilinx/zynqmp/zynqmp.c | 2 +- > 6 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/arch/mips/mach-mt7620/cpu.c b/arch/mips/mach-mt7620/cpu.c > index 9e0ca716f7..fe74f26a54 100644 > --- a/arch/mips/mach-mt7620/cpu.c > +++ b/arch/mips/mach-mt7620/cpu.c > @@ -70,7 +70,7 @@ int print_cpuinfo(void) > } > > #ifdef CONFIG_WATCHDOG > -static struct udevice *watchdog_dev; > +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; > > /* Called by macro WATCHDOG_RESET */ > void watchdog_reset(void) > diff --git a/board/CZ.NIC/turris_mox/turris_mox.c > b/board/CZ.NIC/turris_mox/turris_mox.c > index 65d50a92dd..96cb9c7e5c 100644 > --- a/board/CZ.NIC/turris_mox/turris_mox.c > +++ b/board/CZ.NIC/turris_mox/turris_mox.c > @@ -120,7 +120,7 @@ int board_fix_fdt(void *blob) > #endif > > #ifdef CONFIG_WDT_ARMADA_37XX > -static struct udevice *watchdog_dev; > +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; > > void watchdog_reset(void) > { > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c > b/board/CZ.NIC/turris_omnia/turris_omnia.c > index 1f7650cb36..c7f6479a0c 100644 > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c > @@ -365,7 +365,7 @@ static bool disable_mcu_watchdog(void) > #endif > > #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT_ORION) > -static struct udevice *watchdog_dev = NULL; > +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; > #endif > > int board_init(void) > diff --git a/board/xilinx/microblaze-generic/microblaze-generic.c > b/board/xilinx/microblaze-generic/microblaze-generic.c > index c946ec3208..28c9efa3a2 100644 > --- a/board/xilinx/microblaze-generic/microblaze-generic.c > +++ b/board/xilinx/microblaze-generic/microblaze-generic.c > @@ -25,7 +25,7 @@ > DECLARE_GLOBAL_DATA_PTR; > > #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) > -static struct udevice *watchdog_dev; > +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; > #endif /* !CONFIG_SPL_BUILD && CONFIG_WDT */ > > ulong ram_base; > diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c > index b4b84df576..7c8c32a1c7 100644 > --- a/board/xilinx/zynq/board.c > +++ b/board/xilinx/zynq/board.c > @@ -19,7 +19,7 @@ > DECLARE_GLOBAL_DATA_PTR; > > #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) > -static struct udevice *watchdog_dev; > +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; > #endif > > #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_BOARD_EARLY_INIT_F) > diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c > index 5e1d2116bc..1c2aa99b97 100644 > --- a/board/xilinx/zynqmp/zynqmp.c > +++ b/board/xilinx/zynqmp/zynqmp.c > @@ -25,7 +25,7 @@ > DECLARE_GLOBAL_DATA_PTR; > > #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) > -static struct udevice *watchdog_dev; > +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; > #endif > > #if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_ZYNQMPPL) && \ > If you want to do it for all then please remove the same from this file http://git.denx.de/?p=u-boot.git;a=commitdiff;h=4490e013ee4f2a4b9b6ca9224221fed2788b6940 +#if defined(CONFIG_WDT) && !defined(CONFIG_SPL_BUILD) + /* bss is not cleared at time when watchdog_reset() is called */ + watchdog_dev = NULL; +#endif And also from zynq file. board/xilinx/zynq/board.c Thanks, Michal ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] watchdog: Move watchdog_dev to data section (BSS may not be cleared)
This patch moves all instances of static "watchdog_dev" declarations to the "data" section. This may be needed, as the BSS may not be cleared in the early U-Boot phase, where watchdog_reset() is already beeing called. This may result in incorrect pointer access, as the check to "!watchdog_dev" in watchdog_reset() may not be true and the function may continue to run. Signed-off-by: Stefan Roese Cc: Heiko Schocher Cc: Tom Rini Cc: Michal Simek Cc: "Marek Behún" Cc: Daniel Schwierzeck --- arch/mips/mach-mt7620/cpu.c | 2 +- board/CZ.NIC/turris_mox/turris_mox.c | 2 +- board/CZ.NIC/turris_omnia/turris_omnia.c | 2 +- board/xilinx/microblaze-generic/microblaze-generic.c | 2 +- board/xilinx/zynq/board.c| 2 +- board/xilinx/zynqmp/zynqmp.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/mips/mach-mt7620/cpu.c b/arch/mips/mach-mt7620/cpu.c index 9e0ca716f7..fe74f26a54 100644 --- a/arch/mips/mach-mt7620/cpu.c +++ b/arch/mips/mach-mt7620/cpu.c @@ -70,7 +70,7 @@ int print_cpuinfo(void) } #ifdef CONFIG_WATCHDOG -static struct udevice *watchdog_dev; +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; /* Called by macro WATCHDOG_RESET */ void watchdog_reset(void) diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index 65d50a92dd..96cb9c7e5c 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -120,7 +120,7 @@ int board_fix_fdt(void *blob) #endif #ifdef CONFIG_WDT_ARMADA_37XX -static struct udevice *watchdog_dev; +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; void watchdog_reset(void) { diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index 1f7650cb36..c7f6479a0c 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -365,7 +365,7 @@ static bool disable_mcu_watchdog(void) #endif #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT_ORION) -static struct udevice *watchdog_dev = NULL; +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; #endif int board_init(void) diff --git a/board/xilinx/microblaze-generic/microblaze-generic.c b/board/xilinx/microblaze-generic/microblaze-generic.c index c946ec3208..28c9efa3a2 100644 --- a/board/xilinx/microblaze-generic/microblaze-generic.c +++ b/board/xilinx/microblaze-generic/microblaze-generic.c @@ -25,7 +25,7 @@ DECLARE_GLOBAL_DATA_PTR; #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) -static struct udevice *watchdog_dev; +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; #endif /* !CONFIG_SPL_BUILD && CONFIG_WDT */ ulong ram_base; diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c index b4b84df576..7c8c32a1c7 100644 --- a/board/xilinx/zynq/board.c +++ b/board/xilinx/zynq/board.c @@ -19,7 +19,7 @@ DECLARE_GLOBAL_DATA_PTR; #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) -static struct udevice *watchdog_dev; +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; #endif #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_BOARD_EARLY_INIT_F) diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 5e1d2116bc..1c2aa99b97 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -25,7 +25,7 @@ DECLARE_GLOBAL_DATA_PTR; #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) -static struct udevice *watchdog_dev; +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; #endif #if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_ZYNQMPPL) && \ -- 2.21.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot