Re: [PATCH] s390/decompressor: add fortify_panic as x86 has.

[Heiko Carstens <heiko.carstens@de.ibm.com>]

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