diff mbox series

arm: decompressor: mark error() as __noreturn

Message ID 20180404123117.3575773-1-arnd@arndb.de
State New
Headers show
Series arm: decompressor: mark error() as __noreturn | expand

Commit Message

Arnd Bergmann April 4, 2018, 12:30 p.m. UTC
gcc complains about fortify_panic() possibly returning:

arch/arm/boot/compressed/misc.c: In function 'fortify_panic':
arch/arm/boot/compressed/misc.c:167:1: error: 'noreturn' function does return [-Werror]

This annotates the error() function as __noreturn, which lets gcc
see in all configurations that it fortify_panic() cannot return either.

Fixes: ee333554fed5 ("ARM: 8749/1: Kconfig: Add ARCH_HAS_FORTIFY_SOURCE")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 arch/arm/boot/compressed/misc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.9.0

Comments

Kees Cook April 4, 2018, 10:34 p.m. UTC | #1
On Wed, Apr 4, 2018 at 5:30 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> gcc complains about fortify_panic() possibly returning:

>

> arch/arm/boot/compressed/misc.c: In function 'fortify_panic':

> arch/arm/boot/compressed/misc.c:167:1: error: 'noreturn' function does return [-Werror]

>

> This annotates the error() function as __noreturn, which lets gcc

> see in all configurations that it fortify_panic() cannot return either.

>

> Fixes: ee333554fed5 ("ARM: 8749/1: Kconfig: Add ARCH_HAS_FORTIFY_SOURCE")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


Acked-by: Kees Cook <keescook@chromium.org>


-Kees

> ---

>  arch/arm/boot/compressed/misc.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c

> index 009db7d275f8..313a587d656b 100644

> --- a/arch/arm/boot/compressed/misc.c

> +++ b/arch/arm/boot/compressed/misc.c

> @@ -24,7 +24,7 @@ unsigned int __machine_arch_type;

>  #include <linux/linkage.h>

>

>  static void putstr(const char *ptr);

> -extern void error(char *x);

> +extern void error(char *x) __noreturn;

>

>  #include CONFIG_UNCOMPRESS_INCLUDE

>

> --

> 2.9.0

>




-- 
Kees Cook
Pixel Security
Russell King (Oracle) April 4, 2018, 10:38 p.m. UTC | #2
On Wed, Apr 04, 2018 at 03:34:06PM -0700, Kees Cook wrote:
> On Wed, Apr 4, 2018 at 5:30 AM, Arnd Bergmann <arnd@arndb.de> wrote:

> > gcc complains about fortify_panic() possibly returning:

> >

> > arch/arm/boot/compressed/misc.c: In function 'fortify_panic':

> > arch/arm/boot/compressed/misc.c:167:1: error: 'noreturn' function does return [-Werror]

> >

> > This annotates the error() function as __noreturn, which lets gcc

> > see in all configurations that it fortify_panic() cannot return either.

> >

> > Fixes: ee333554fed5 ("ARM: 8749/1: Kconfig: Add ARCH_HAS_FORTIFY_SOURCE")

> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> 

> Acked-by: Kees Cook <keescook@chromium.org>


I'd actually prefer a more robust solution to this - error() is used
in different files, each with their own separate prototype, which is
error-prone.  The same goes for a few other variables.  The following
is IMHO a more complete (and thus larger) patch:

 arch/arm/boot/compressed/decompress.c |  5 +----
 arch/arm/boot/compressed/misc.c       |  2 +-
 arch/arm/boot/compressed/misc.h       | 10 ++++++++++
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/compressed/decompress.c b/arch/arm/boot/compressed/decompress.c
index a2ac3fe7dbf8..c16c1829a5e4 100644
--- a/arch/arm/boot/compressed/decompress.c
+++ b/arch/arm/boot/compressed/decompress.c
@@ -6,10 +6,7 @@
 #include <linux/stddef.h>	/* for NULL */
 #include <linux/linkage.h>
 #include <asm/string.h>
-
-extern unsigned long free_mem_ptr;
-extern unsigned long free_mem_end_ptr;
-extern void error(char *);
+#include "misc.h"
 
 #define STATIC static
 #define STATIC_RW_DATA	/* non-static please */
diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index a3ded2a64b32..e94c84dded6c 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -22,9 +22,9 @@ unsigned int __machine_arch_type;
 #include <linux/compiler.h>	/* for inline */
 #include <linux/types.h>
 #include <linux/linkage.h>
+#include "misc.h"
 
 static void putstr(const char *ptr);
-extern void error(char *x);
 
 #include CONFIG_UNCOMPRESS_INCLUDE
 
diff --git a/arch/arm/boot/compressed/misc.h b/arch/arm/boot/compressed/misc.h
new file mode 100644
index 000000000000..c958dccd1d97
--- /dev/null
+++ b/arch/arm/boot/compressed/misc.h
@@ -0,0 +1,10 @@
+#ifndef MISC_H
+#define MISC_H
+
+#include <linux/compiler.h>
+
+void error(char *x) __noreturn;
+extern unsigned long free_mem_ptr;
+extern unsigned long free_mem_end_ptr;
+
+#endif


-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
Arnd Bergmann April 5, 2018, 11:18 a.m. UTC | #3
On Thu, Apr 5, 2018 at 12:38 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Wed, Apr 04, 2018 at 03:34:06PM -0700, Kees Cook wrote:

>> On Wed, Apr 4, 2018 at 5:30 AM, Arnd Bergmann <arnd@arndb.de> wrote:

>> > gcc complains about fortify_panic() possibly returning:

>> >

>> > arch/arm/boot/compressed/misc.c: In function 'fortify_panic':

>> > arch/arm/boot/compressed/misc.c:167:1: error: 'noreturn' function does return [-Werror]

>> >

>> > This annotates the error() function as __noreturn, which lets gcc

>> > see in all configurations that it fortify_panic() cannot return either.

>> >

>> > Fixes: ee333554fed5 ("ARM: 8749/1: Kconfig: Add ARCH_HAS_FORTIFY_SOURCE")

>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

>>

>> Acked-by: Kees Cook <keescook@chromium.org>

>

> I'd actually prefer a more robust solution to this - error() is used

> in different files, each with their own separate prototype, which is

> error-prone.  The same goes for a few other variables.  The following

> is IMHO a more complete (and thus larger) patch:


Right, much better.

Acked-by: Arnd Bergmann <arnd@arndb.de>


I assume you'll just fold it into the original patch if you can, or apply it
on top yourself.
diff mbox series

Patch

diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index 009db7d275f8..313a587d656b 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -24,7 +24,7 @@  unsigned int __machine_arch_type;
 #include <linux/linkage.h>
 
 static void putstr(const char *ptr);
-extern void error(char *x);
+extern void error(char *x) __noreturn;
 
 #include CONFIG_UNCOMPRESS_INCLUDE