All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] s390/decompressor: add fortify_panic as x86 has.
@ 2017-12-07 10:37 Michal Suchanek
  2017-12-07 12:55 ` Martin Schwidefsky
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Suchanek @ 2017-12-07 10:37 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens, Michal Suchanek,
	Marcelo Henrique Cerri, Greg Kroah-Hartman, linux-s390,
	linux-kernel

Fix following error:

  LD      arch/s390/boot/compressed/vmlinux
drivers/s390/char/sclp_early_core.o: In function `memcpy':
../include/linux/string.h:340: undefined reference to `fortify_panic'
make[4]: *** [../arch/s390/boot/compressed/Makefile:29: arch/s390/boot/compressed/vmlinux] Error 1

Fixes: 79962038dffa ("s390: add support for FORTIFY_SOURCE")
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/s390/boot/compressed/misc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/s390/boot/compressed/misc.c b/arch/s390/boot/compressed/misc.c
index cecf38b9ec82..e79c4499c548 100644
--- a/arch/s390/boot/compressed/misc.c
+++ b/arch/s390/boot/compressed/misc.c
@@ -174,3 +174,7 @@ unsigned long decompress_kernel(void)
 	return (unsigned long) output;
 }
 
+void fortify_panic(const char *name)
+{
+	error("detected buffer overflow");
+}
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] s390/decompressor: add fortify_panic as x86 has.
  2017-12-07 10:37 [PATCH] s390/decompressor: add fortify_panic as x86 has Michal Suchanek
@ 2017-12-07 12:55 ` Martin Schwidefsky
  2017-12-11 13:09   ` Michal Suchánek
  0 siblings, 1 reply; 6+ messages in thread
From: Martin Schwidefsky @ 2017-12-07 12:55 UTC (permalink / raw)
  To: Michal Suchanek
  Cc: Heiko Carstens, Marcelo Henrique Cerri, Greg Kroah-Hartman,
	linux-s390, linux-kernel

On Thu,  7 Dec 2017 11:37:27 +0100
Michal Suchanek <msuchanek@suse.de> wrote:

> Fix following error:
> 
>   LD      arch/s390/boot/compressed/vmlinux
> drivers/s390/char/sclp_early_core.o: In function `memcpy':
> ../include/linux/string.h:340: undefined reference to `fortify_panic'
> make[4]: *** [../arch/s390/boot/compressed/Makefile:29: arch/s390/boot/compressed/vmlinux] Error 1
> 
> Fixes: 79962038dffa ("s390: add support for FORTIFY_SOURCE")
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  arch/s390/boot/compressed/misc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/s390/boot/compressed/misc.c b/arch/s390/boot/compressed/misc.c
> index cecf38b9ec82..e79c4499c548 100644
> --- a/arch/s390/boot/compressed/misc.c
> +++ b/arch/s390/boot/compressed/misc.c
> @@ -174,3 +174,7 @@ unsigned long decompress_kernel(void)
>  	return (unsigned long) output;
>  }
> 
> +void fortify_panic(const char *name)
> +{
> +	error("detected buffer overflow");
> +}

Odd, the current linux master tree builds just fine with CONFIG_FORTIFY_SOURCE=y.
There *is* a reference to fortify_panic in drivers/s390/char/sclp_early.o.
This object is included in the link for the compressed vmlinux, but the
function  that contains the call to fortify_panic is not included in the
compressed image. I wonder what causes this difference in behavior.

The patch makes sense though and I will add it to the queue.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] s390/decompressor: add fortify_panic as x86 has.
  2017-12-07 12:55 ` Martin Schwidefsky
@ 2017-12-11 13:09   ` Michal Suchánek
  2017-12-13  8:33       ` Heiko Carstens
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Suchánek @ 2017-12-11 13:09 UTC (permalink / raw)
  To: Martin Schwidefsky
  Cc: Heiko Carstens, Marcelo Henrique Cerri, Greg Kroah-Hartman,
	linux-s390, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1712 bytes --]

Hello,

On Thu, 7 Dec 2017 13:55:07 +0100
Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:

> On Thu,  7 Dec 2017 11:37:27 +0100
> Michal Suchanek <msuchanek@suse.de> wrote:
> 
> > Fix following error:
> > 
> >   LD      arch/s390/boot/compressed/vmlinux
> > drivers/s390/char/sclp_early_core.o: In function `memcpy':
> > ../include/linux/string.h:340: undefined reference to
> > `fortify_panic' make[4]: ***
> > [../arch/s390/boot/compressed/Makefile:29:
> > arch/s390/boot/compressed/vmlinux] Error 1
> > 
> > Fixes: 79962038dffa ("s390: add support for FORTIFY_SOURCE")
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  arch/s390/boot/compressed/misc.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/s390/boot/compressed/misc.c
> > b/arch/s390/boot/compressed/misc.c index cecf38b9ec82..e79c4499c548
> > 100644 --- a/arch/s390/boot/compressed/misc.c
> > +++ b/arch/s390/boot/compressed/misc.c
> > @@ -174,3 +174,7 @@ unsigned long decompress_kernel(void)
> >  	return (unsigned long) output;
> >  }
> > 
> > +void fortify_panic(const char *name)
> > +{
> > +	error("detected buffer overflow");
> > +}  
> 
> Odd, the current linux master tree builds just fine with
> CONFIG_FORTIFY_SOURCE=y. There *is* a reference to fortify_panic in
> drivers/s390/char/sclp_early.o. This object is included in the link
> for the compressed vmlinux, but the function  that contains the call
> to fortify_panic is not included in the compressed image. I wonder
> what causes this difference in behavior.
> 
> The patch makes sense though and I will add it to the queue.
> 

It probably depends on the config.

FWIW attaching the config that fails to build for me.

Thanks

Michal

[-- Attachment #2: zfcpdump.xz --]
[-- Type: application/x-xz, Size: 7668 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] s390/decompressor: add fortify_panic as x86 has.
  2017-12-11 13:09   ` Michal Suchánek
@ 2017-12-13  8:33       ` Heiko Carstens
  0 siblings, 0 replies; 6+ messages in thread
From: Heiko Carstens @ 2017-12-13  8:33 UTC (permalink / raw)
  To: Michal Suchánek
  Cc: Martin Schwidefsky, Marcelo Henrique Cerri, Greg Kroah-Hartman,
	linux-s390, linux-kernel

On Mon, Dec 11, 2017 at 02:09:04PM +0100, Michal Suchánek wrote:
> Hello,
> 
> On Thu, 7 Dec 2017 13:55:07 +0100
> Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> 
> > On Thu,  7 Dec 2017 11:37:27 +0100
> > Michal Suchanek <msuchanek@suse.de> wrote:
> > 
> > > Fix following error:
> > > 
> > >   LD      arch/s390/boot/compressed/vmlinux
> > > drivers/s390/char/sclp_early_core.o: In function `memcpy':
> > > ../include/linux/string.h:340: undefined reference to
> > > `fortify_panic' make[4]: ***
> > > [../arch/s390/boot/compressed/Makefile:29:
> > > arch/s390/boot/compressed/vmlinux] Error 1
> > > 
> > > Fixes: 79962038dffa ("s390: add support for FORTIFY_SOURCE")
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > >  arch/s390/boot/compressed/misc.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/arch/s390/boot/compressed/misc.c
> > > b/arch/s390/boot/compressed/misc.c index cecf38b9ec82..e79c4499c548
> > > 100644 --- a/arch/s390/boot/compressed/misc.c
> > > +++ b/arch/s390/boot/compressed/misc.c
> > > @@ -174,3 +174,7 @@ unsigned long decompress_kernel(void)
> > >  	return (unsigned long) output;
> > >  }
> > > 
> > > +void fortify_panic(const char *name)
> > > +{
> > > +	error("detected buffer overflow");
> > > +}  
> > 
> > Odd, the current linux master tree builds just fine with
> > CONFIG_FORTIFY_SOURCE=y. There *is* a reference to fortify_panic in
> > drivers/s390/char/sclp_early.o. This object is included in the link
> > for the compressed vmlinux, but the function  that contains the call
> > to fortify_panic is not included in the compressed image. I wonder
> > what causes this difference in behavior.
> > 
> > The patch makes sense though and I will add it to the queue.
> > 
> 
> It probably depends on the config.
> 
> FWIW attaching the config that fails to build for me.

Yes, with that configuration it is reproducable. However I disagree with
the fix. We have a call to fortify_panic() within sclp_early_core.c which
is our console driver. If that would ever trigger the result would be an
endless loop (error() would call the console driver again). This would
repeat until we hit an addressing exception or code gets overwritten due to
the ever increasing stack. At some point we simply have to trust the
code. That's also why I disabled FORTIFY_SOURCE for a couple of other files
which contain early code.

Therefore the simple solution would be to just disable FORTIFY_SOURCE for
the early sclp code as well.

The patch below will do that:

>From 4ec2a3fd66bb5b1da35807bc2e382f9b8d9eebb8 Mon Sep 17 00:00:00 2001
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Wed, 13 Dec 2017 09:21:59 +0100
Subject: [PATCH] s390/sclp: disable FORTIFY_SOURCE for early sclp code

Michal Suchanek reported the following compile error with
FORTIFY_SOURCE enabled:

drivers/s390/char/sclp_early_core.o: In function `memcpy':
include/linux/string.h:340: undefined reference to `fortify_panic'

To fix this simply disable FORTIFY_SOURCE on the early sclp code as
well, which I forgot on the initial commit.

Fixes: 79962038dffa ("s390: add support for FORTIFY_SOURCE")
Reported-by: Michal Suchanek <msuchanek@suse.de>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 drivers/s390/char/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/s390/char/Makefile b/drivers/s390/char/Makefile
index 05ac6ba15a53..614b44e70a28 100644
--- a/drivers/s390/char/Makefile
+++ b/drivers/s390/char/Makefile
@@ -17,6 +17,8 @@ CFLAGS_REMOVE_sclp_early_core.o	+= $(CC_FLAGS_MARCH)
 CFLAGS_sclp_early_core.o		+= -march=z900
 endif
 
+CFLAGS_sclp_early_core.o		+= -D__NO_FORTIFY
+
 obj-y += ctrlchar.o keyboard.o defkeymap.o sclp.o sclp_rw.o sclp_quiesce.o \
 	 sclp_cmd.o sclp_config.o sclp_cpi_sys.o sclp_ocf.o sclp_ctl.o \
 	 sclp_early.o sclp_early_core.o
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] s390/decompressor: add fortify_panic as x86 has.
@ 2017-12-13  8:33       ` Heiko Carstens
  0 siblings, 0 replies; 6+ messages in thread
From: Heiko Carstens @ 2017-12-13  8:33 UTC (permalink / raw)
  To: Michal Suchánek
  Cc: Martin Schwidefsky, Marcelo Henrique Cerri, Greg Kroah-Hartman,
	linux-s390, linux-kernel

On Mon, Dec 11, 2017 at 02:09:04PM +0100, Michal Suchánek wrote:
> Hello,
> 
> On Thu, 7 Dec 2017 13:55:07 +0100
> Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> 
> > On Thu,  7 Dec 2017 11:37:27 +0100
> > Michal Suchanek <msuchanek@suse.de> wrote:
> > 
> > > Fix following error:
> > > 
> > >   LD      arch/s390/boot/compressed/vmlinux
> > > drivers/s390/char/sclp_early_core.o: In function `memcpy':
> > > ../include/linux/string.h:340: undefined reference to
> > > `fortify_panic' make[4]: ***
> > > [../arch/s390/boot/compressed/Makefile:29:
> > > arch/s390/boot/compressed/vmlinux] Error 1
> > > 
> > > Fixes: 79962038dffa ("s390: add support for FORTIFY_SOURCE")
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > >  arch/s390/boot/compressed/misc.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/arch/s390/boot/compressed/misc.c
> > > b/arch/s390/boot/compressed/misc.c index cecf38b9ec82..e79c4499c548
> > > 100644 --- a/arch/s390/boot/compressed/misc.c
> > > +++ b/arch/s390/boot/compressed/misc.c
> > > @@ -174,3 +174,7 @@ unsigned long decompress_kernel(void)
> > >  	return (unsigned long) output;
> > >  }
> > > 
> > > +void fortify_panic(const char *name)
> > > +{
> > > +	error("detected buffer overflow");
> > > +}  
> > 
> > Odd, the current linux master tree builds just fine with
> > CONFIG_FORTIFY_SOURCE=y. There *is* a reference to fortify_panic in
> > drivers/s390/char/sclp_early.o. This object is included in the link
> > for the compressed vmlinux, but the function  that contains the call
> > to fortify_panic is not included in the compressed image. I wonder
> > what causes this difference in behavior.
> > 
> > The patch makes sense though and I will add it to the queue.
> > 
> 
> It probably depends on the config.
> 
> FWIW attaching the config that fails to build for me.

Yes, with that configuration it is reproducable. However I disagree with
the fix. We have a call to fortify_panic() within sclp_early_core.c which
is our console driver. If that would ever trigger the result would be an
endless loop (error() would call the console driver again). This would
repeat until we hit an addressing exception or code gets overwritten due to
the ever increasing stack. At some point we simply have to trust the
code. That's also why I disabled FORTIFY_SOURCE for a couple of other files
which contain early code.

Therefore the simple solution would be to just disable FORTIFY_SOURCE for
the early sclp code as well.

The patch below will do that:

From 4ec2a3fd66bb5b1da35807bc2e382f9b8d9eebb8 Mon Sep 17 00:00:00 2001
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Wed, 13 Dec 2017 09:21:59 +0100
Subject: [PATCH] s390/sclp: disable FORTIFY_SOURCE for early sclp code

Michal Suchanek reported the following compile error with
FORTIFY_SOURCE enabled:

drivers/s390/char/sclp_early_core.o: In function `memcpy':
include/linux/string.h:340: undefined reference to `fortify_panic'

To fix this simply disable FORTIFY_SOURCE on the early sclp code as
well, which I forgot on the initial commit.

Fixes: 79962038dffa ("s390: add support for FORTIFY_SOURCE")
Reported-by: Michal Suchanek <msuchanek@suse.de>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 drivers/s390/char/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/s390/char/Makefile b/drivers/s390/char/Makefile
index 05ac6ba15a53..614b44e70a28 100644
--- a/drivers/s390/char/Makefile
+++ b/drivers/s390/char/Makefile
@@ -17,6 +17,8 @@ CFLAGS_REMOVE_sclp_early_core.o	+= $(CC_FLAGS_MARCH)
 CFLAGS_sclp_early_core.o		+= -march=z900
 endif
 
+CFLAGS_sclp_early_core.o		+= -D__NO_FORTIFY
+
 obj-y += ctrlchar.o keyboard.o defkeymap.o sclp.o sclp_rw.o sclp_quiesce.o \
 	 sclp_cmd.o sclp_config.o sclp_cpi_sys.o sclp_ocf.o sclp_ctl.o \
 	 sclp_early.o sclp_early_core.o
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] s390/decompressor: add fortify_panic as x86 has.
  2017-12-13  8:33       ` Heiko Carstens
  (?)
@ 2017-12-20 14:09       ` Michal Suchánek
  -1 siblings, 0 replies; 6+ messages in thread
From: Michal Suchánek @ 2017-12-20 14:09 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Martin Schwidefsky, Marcelo Henrique Cerri, Greg Kroah-Hartman,
	linux-s390, linux-kernel

On Wed, 13 Dec 2017 09:33:17 +0100
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> From 4ec2a3fd66bb5b1da35807bc2e382f9b8d9eebb8 Mon Sep 17 00:00:00 2001
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
> Date: Wed, 13 Dec 2017 09:21:59 +0100
> Subject: [PATCH] s390/sclp: disable FORTIFY_SOURCE for early sclp code
> 
> Michal Suchanek reported the following compile error with
> FORTIFY_SOURCE enabled:
> 
> drivers/s390/char/sclp_early_core.o: In function `memcpy':
> include/linux/string.h:340: undefined reference to `fortify_panic'
> 
> To fix this simply disable FORTIFY_SOURCE on the early sclp code as
> well, which I forgot on the initial commit.
> 
> Fixes: 79962038dffa ("s390: add support for FORTIFY_SOURCE")
> Reported-by: Michal Suchanek <msuchanek@suse.de>
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> ---
>  drivers/s390/char/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/s390/char/Makefile b/drivers/s390/char/Makefile
> index 05ac6ba15a53..614b44e70a28 100644
> --- a/drivers/s390/char/Makefile
> +++ b/drivers/s390/char/Makefile
> @@ -17,6 +17,8 @@ CFLAGS_REMOVE_sclp_early_core.o	+=
> $(CC_FLAGS_MARCH) CFLAGS_sclp_early_core.o		+=
> -march=z900 endif
>  
> +CFLAGS_sclp_early_core.o		+= -D__NO_FORTIFY
> +
>  obj-y += ctrlchar.o keyboard.o defkeymap.o sclp.o sclp_rw.o
> sclp_quiesce.o \ sclp_cmd.o sclp_config.o sclp_cpi_sys.o sclp_ocf.o
> sclp_ctl.o \ sclp_early.o sclp_early_core.o

That works for me as well.

Tested-by: Michal Suchanek <msuchanek@suse.de>

Thanks

Michal

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-20 14:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-07 10:37 [PATCH] s390/decompressor: add fortify_panic as x86 has Michal Suchanek
2017-12-07 12:55 ` Martin Schwidefsky
2017-12-11 13:09   ` Michal Suchánek
2017-12-13  8:33     ` Heiko Carstens
2017-12-13  8:33       ` Heiko Carstens
2017-12-20 14:09       ` Michal Suchánek

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.