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

[Heiko Carstens]

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@xxxxxxxxxx> wrote:
> 
> > On Thu,  7 Dec 2017 11:37:27 +0100
> > Michal Suchanek <msuchanek@xxxxxxx> 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@xxxxxxx>
> > > ---
> > >  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: