Re: Regression with arm in next with stack protector

[Rich Felker]

On Tue, Mar 27, 2018 at 06:20:27PM +0100, Russell King - ARM Linux wrote:
> On Tue, Mar 27, 2018 at 11:35:25AM -0400, Rich Felker wrote:
> > On Tue, Mar 27, 2018 at 10:04:10AM +0100, Russell King - ARM Linux wrote:
> > > On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> > > > Hi,
> > > > 
> > > > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > > > compressed boot phase") breaks booting on arm.
> > > > 
> > > > This is all I get from the bootloader on omap3:
> > > > 
> > > > Starting kernel ...
> > > > 
> > > > data abort
> > > > pc : [<810002d0>]  lr : [<100110a8>]
> > > > reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> > > > sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> > > > r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> > > > r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> > > > r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> > > > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > > > Resetting CPU ...
> > > > 
> > > > resetting ...
> > > 
> > > The reason for this is the following code that was introduced by the
> > > referenced patch:
> > > 
> > > +   ldr r0, =__stack_chk_guard
> > > +   ldr r1, =0x000a0dff
> > > +   str r1, [r0]
> > > 
> > > This uses the absolute address of __stack_chk_guard in the decompressor,
> > > which is a self-relocatable image.  As with all constructs like the
> > > above, this absolute address doesn't get fixed up, and so it ends up
> > > pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
> > > and the decompressor looks to be around 0x8100.
> > > 
> > > Such constructs can not be used in the decompressor for exactly this
> > > reason - they need to use PC-relative addressing instead just like
> > > everything else does in head.S.
> > 
> > Can someone please answer why this is even needed to begin with? I
> > don't see any compelling reason __stack_chk_guard needs a particular
> > value in the decompressor, which is not dealing with any non-constant
> > input.
> 
> Untrue - it can do some parsing of the DT and updating/appending
> information from ATAGs.  However, all that should be coming from
> a trusted environment, so I don't see much of a "trust" issue here.
> (If the parent environment is not trusted, then the environment we're
> running in is not trusted.)

OK, I was considering DT constant, but it doesn't really matter as you
say since the input comes from a trusted environment and could subvert
the system in much more direct ways than blowing away the
decompressor's stack buffers if it wanted to.

> > Just putting __stack_chk_guard in its bss should be fine and
> > would eliminate all the risks of wrong code to load a value into it.
> > Alternatively put it in initialized data with the desired value.
> 
> I'm no expert with this, so I can't comment.  I build my kernels
> with gcc 4.7.4, which I don't think supports this feature.

By "this feature" do you mean stack protector? I still have a 4.7.3
for x86 around and -fstack-protector-all works fine on it. Not sure if
there are issues using stack protector with kernel, or on ARM, for
older GCCs. In any case defining __stack_chk_guard as initialized data
should work on any gcc version regardless of whether stack protector
is actually used; it doesn't require any compiler features just basic
C.

Rich

Re: Regression with arm in next with stack protector

[Rich Felker]

On Tue, Mar 27, 2018 at 06:20:27PM +0100, Russell King - ARM Linux wrote:
> On Tue, Mar 27, 2018 at 11:35:25AM -0400, Rich Felker wrote:
> > On Tue, Mar 27, 2018 at 10:04:10AM +0100, Russell King - ARM Linux wrote:
> > > On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> > > > Hi,
> > > > 
> > > > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > > > compressed boot phase") breaks booting on arm.
> > > > 
> > > > This is all I get from the bootloader on omap3:
> > > > 
> > > > Starting kernel ...
> > > > 
> > > > data abort
> > > > pc : [<810002d0>]  lr : [<100110a8>]
> > > > reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> > > > sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> > > > r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> > > > r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> > > > r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> > > > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > > > Resetting CPU ...
> > > > 
> > > > resetting ...
> > > 
> > > The reason for this is the following code that was introduced by the
> > > referenced patch:
> > > 
> > > +   ldr r0, =__stack_chk_guard
> > > +   ldr r1, =0x000a0dff
> > > +   str r1, [r0]
> > > 
> > > This uses the absolute address of __stack_chk_guard in the decompressor,
> > > which is a self-relocatable image.  As with all constructs like the
> > > above, this absolute address doesn't get fixed up, and so it ends up
> > > pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
> > > and the decompressor looks to be around 0x8100.
> > > 
> > > Such constructs can not be used in the decompressor for exactly this
> > > reason - they need to use PC-relative addressing instead just like
> > > everything else does in head.S.
> > 
> > Can someone please answer why this is even needed to begin with? I
> > don't see any compelling reason __stack_chk_guard needs a particular
> > value in the decompressor, which is not dealing with any non-constant
> > input.
> 
> Untrue - it can do some parsing of the DT and updating/appending
> information from ATAGs.  However, all that should be coming from
> a trusted environment, so I don't see much of a "trust" issue here.
> (If the parent environment is not trusted, then the environment we're
> running in is not trusted.)

OK, I was considering DT constant, but it doesn't really matter as you
say since the input comes from a trusted environment and could subvert
the system in much more direct ways than blowing away the
decompressor's stack buffers if it wanted to.

> > Just putting __stack_chk_guard in its bss should be fine and
> > would eliminate all the risks of wrong code to load a value into it.
> > Alternatively put it in initialized data with the desired value.
> 
> I'm no expert with this, so I can't comment.  I build my kernels
> with gcc 4.7.4, which I don't think supports this feature.

By "this feature" do you mean stack protector? I still have a 4.7.3
for x86 around and -fstack-protector-all works fine on it. Not sure if
there are issues using stack protector with kernel, or on ARM, for
older GCCs. In any case defining __stack_chk_guard as initialized data
should work on any gcc version regardless of whether stack protector
is actually used; it doesn't require any compiler features just basic
C.

Rich

Re: Regression with arm in next with stack protector

[Russell King - ARM Linux]

On Tue, Mar 27, 2018 at 11:35:25AM -0400, Rich Felker wrote:
> On Tue, Mar 27, 2018 at 10:04:10AM +0100, Russell King - ARM Linux wrote:
> > On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> > > Hi,
> > > 
> > > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > > compressed boot phase") breaks booting on arm.
> > > 
> > > This is all I get from the bootloader on omap3:
> > > 
> > > Starting kernel ...
> > > 
> > > data abort
> > > pc : [<810002d0>]  lr : [<100110a8>]
> > > reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> > > sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> > > r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> > > r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> > > r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> > > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > > Resetting CPU ...
> > > 
> > > resetting ...
> > 
> > The reason for this is the following code that was introduced by the
> > referenced patch:
> > 
> > +   ldr r0, =__stack_chk_guard
> > +   ldr r1, =0x000a0dff
> > +   str r1, [r0]
> > 
> > This uses the absolute address of __stack_chk_guard in the decompressor,
> > which is a self-relocatable image.  As with all constructs like the
> > above, this absolute address doesn't get fixed up, and so it ends up
> > pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
> > and the decompressor looks to be around 0x8100.
> > 
> > Such constructs can not be used in the decompressor for exactly this
> > reason - they need to use PC-relative addressing instead just like
> > everything else does in head.S.
> 
> Can someone please answer why this is even needed to begin with? I
> don't see any compelling reason __stack_chk_guard needs a particular
> value in the decompressor, which is not dealing with any non-constant
> input.

Untrue - it can do some parsing of the DT and updating/appending
information from ATAGs.  However, all that should be coming from
a trusted environment, so I don't see much of a "trust" issue here.
(If the parent environment is not trusted, then the environment we're
running in is not trusted.)

> Just putting __stack_chk_guard in its bss should be fine and
> would eliminate all the risks of wrong code to load a value into it.
> Alternatively put it in initialized data with the desired value.

I'm no expert with this, so I can't comment.  I build my kernels
with gcc 4.7.4, which I don't think supports this feature.

-- 
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

Re: Regression with arm in next with stack protector

[Russell King - ARM Linux]

On Tue, Mar 27, 2018 at 11:35:25AM -0400, Rich Felker wrote:
> On Tue, Mar 27, 2018 at 10:04:10AM +0100, Russell King - ARM Linux wrote:
> > On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> > > Hi,
> > > 
> > > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > > compressed boot phase") breaks booting on arm.
> > > 
> > > This is all I get from the bootloader on omap3:
> > > 
> > > Starting kernel ...
> > > 
> > > data abort
> > > pc : [<810002d0>]  lr : [<100110a8>]
> > > reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> > > sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> > > r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> > > r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> > > r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> > > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > > Resetting CPU ...
> > > 
> > > resetting ...
> > 
> > The reason for this is the following code that was introduced by the
> > referenced patch:
> > 
> > +   ldr r0, =__stack_chk_guard
> > +   ldr r1, =0x000a0dff
> > +   str r1, [r0]
> > 
> > This uses the absolute address of __stack_chk_guard in the decompressor,
> > which is a self-relocatable image.  As with all constructs like the
> > above, this absolute address doesn't get fixed up, and so it ends up
> > pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
> > and the decompressor looks to be around 0x8100.
> > 
> > Such constructs can not be used in the decompressor for exactly this
> > reason - they need to use PC-relative addressing instead just like
> > everything else does in head.S.
> 
> Can someone please answer why this is even needed to begin with? I
> don't see any compelling reason __stack_chk_guard needs a particular
> value in the decompressor, which is not dealing with any non-constant
> input.

Untrue - it can do some parsing of the DT and updating/appending
information from ATAGs.  However, all that should be coming from
a trusted environment, so I don't see much of a "trust" issue here.
(If the parent environment is not trusted, then the environment we're
running in is not trusted.)

> Just putting __stack_chk_guard in its bss should be fine and
> would eliminate all the risks of wrong code to load a value into it.
> Alternatively put it in initialized data with the desired value.

I'm no expert with this, so I can't comment.  I build my kernels
with gcc 4.7.4, which I don't think supports this feature.

-- 
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

Re: Regression with arm in next with stack protector

[Rich Felker]

On Tue, Mar 27, 2018 at 12:34:53PM +0100, Russell King - ARM Linux wrote:
> On Tue, Mar 27, 2018 at 10:04:10AM +0100, Russell King - ARM Linux wrote:
> > On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> > > Hi,
> > > 
> > > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > > compressed boot phase") breaks booting on arm.
> > > 
> > > This is all I get from the bootloader on omap3:
> > > 
> > > Starting kernel ...
> > > 
> > > data abort
> > > pc : [<810002d0>]  lr : [<100110a8>]
> > > reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> > > sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> > > r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> > > r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> > > r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> > > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > > Resetting CPU ...
> > > 
> > > resetting ...
> > 
> > The reason for this is the following code that was introduced by the
> > referenced patch:
> > 
> > +   ldr r0, =__stack_chk_guard
> > +   ldr r1, =0x000a0dff
> > +   str r1, [r0]
> > 
> > This uses the absolute address of __stack_chk_guard in the decompressor,
> > which is a self-relocatable image.  As with all constructs like the
> > above, this absolute address doesn't get fixed up, and so it ends up
> > pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
> > and the decompressor looks to be around 0x8100.
> > 
> > Such constructs can not be used in the decompressor for exactly this
> > reason - they need to use PC-relative addressing instead just like
> > everything else does in head.S.
> 
> Is there any reason we can't do this in misc.c:
> 
> const unsigned int __stack_chk_guard = 0x000a0dff;
> 
> ?  That would save having runtime code to set that value up, and after
> all, it is constant.  Arguments about it potentially ending up at a
> fixed offset into the image need not be said - that's already the case
> with placing it in the early assembly code, and as has been established,
> it needs to be initialised prior to any C code being called.

I've asked this multiple times in this thread and as far as I know
nobody has answered.

Rich

Re: Regression with arm in next with stack protector

[Rich Felker]

On Tue, Mar 27, 2018 at 12:34:53PM +0100, Russell King - ARM Linux wrote:
> On Tue, Mar 27, 2018 at 10:04:10AM +0100, Russell King - ARM Linux wrote:
> > On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> > > Hi,
> > > 
> > > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > > compressed boot phase") breaks booting on arm.
> > > 
> > > This is all I get from the bootloader on omap3:
> > > 
> > > Starting kernel ...
> > > 
> > > data abort
> > > pc : [<810002d0>]  lr : [<100110a8>]
> > > reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> > > sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> > > r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> > > r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> > > r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> > > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > > Resetting CPU ...
> > > 
> > > resetting ...
> > 
> > The reason for this is the following code that was introduced by the
> > referenced patch:
> > 
> > +   ldr r0, =__stack_chk_guard
> > +   ldr r1, =0x000a0dff
> > +   str r1, [r0]
> > 
> > This uses the absolute address of __stack_chk_guard in the decompressor,
> > which is a self-relocatable image.  As with all constructs like the
> > above, this absolute address doesn't get fixed up, and so it ends up
> > pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
> > and the decompressor looks to be around 0x8100.
> > 
> > Such constructs can not be used in the decompressor for exactly this
> > reason - they need to use PC-relative addressing instead just like
> > everything else does in head.S.
> 
> Is there any reason we can't do this in misc.c:
> 
> const unsigned int __stack_chk_guard = 0x000a0dff;
> 
> ?  That would save having runtime code to set that value up, and after
> all, it is constant.  Arguments about it potentially ending up at a
> fixed offset into the image need not be said - that's already the case
> with placing it in the early assembly code, and as has been established,
> it needs to be initialised prior to any C code being called.

I've asked this multiple times in this thread and as far as I know
nobody has answered.

Rich

Re: Regression with arm in next with stack protector

[Rich Felker]

On Tue, Mar 27, 2018 at 10:04:10AM +0100, Russell King - ARM Linux wrote:
> On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> > Hi,
> > 
> > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > compressed boot phase") breaks booting on arm.
> > 
> > This is all I get from the bootloader on omap3:
> > 
> > Starting kernel ...
> > 
> > data abort
> > pc : [<810002d0>]  lr : [<100110a8>]
> > reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> > sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> > r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> > r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> > r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > Resetting CPU ...
> > 
> > resetting ...
> 
> The reason for this is the following code that was introduced by the
> referenced patch:
> 
> +   ldr r0, =__stack_chk_guard
> +   ldr r1, =0x000a0dff
> +   str r1, [r0]
> 
> This uses the absolute address of __stack_chk_guard in the decompressor,
> which is a self-relocatable image.  As with all constructs like the
> above, this absolute address doesn't get fixed up, and so it ends up
> pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
> and the decompressor looks to be around 0x8100.
> 
> Such constructs can not be used in the decompressor for exactly this
> reason - they need to use PC-relative addressing instead just like
> everything else does in head.S.

Can someone please answer why this is even needed to begin with? I
don't see any compelling reason __stack_chk_guard needs a particular
value in the decompressor, which is not dealing with any non-constant
input. Just putting __stack_chk_guard in its bss should be fine and
would eliminate all the risks of wrong code to load a value into it.
Alternatively put it in initialized data with the desired value.

Rich

Re: Regression with arm in next with stack protector

[Rich Felker]

On Tue, Mar 27, 2018 at 10:04:10AM +0100, Russell King - ARM Linux wrote:
> On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> > Hi,
> > 
> > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > compressed boot phase") breaks booting on arm.
> > 
> > This is all I get from the bootloader on omap3:
> > 
> > Starting kernel ...
> > 
> > data abort
> > pc : [<810002d0>]  lr : [<100110a8>]
> > reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> > sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> > r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> > r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> > r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > Resetting CPU ...
> > 
> > resetting ...
> 
> The reason for this is the following code that was introduced by the
> referenced patch:
> 
> +   ldr r0, =__stack_chk_guard
> +   ldr r1, =0x000a0dff
> +   str r1, [r0]
> 
> This uses the absolute address of __stack_chk_guard in the decompressor,
> which is a self-relocatable image.  As with all constructs like the
> above, this absolute address doesn't get fixed up, and so it ends up
> pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
> and the decompressor looks to be around 0x8100.
> 
> Such constructs can not be used in the decompressor for exactly this
> reason - they need to use PC-relative addressing instead just like
> everything else does in head.S.

Can someone please answer why this is even needed to begin with? I
don't see any compelling reason __stack_chk_guard needs a particular
value in the decompressor, which is not dealing with any non-constant
input. Just putting __stack_chk_guard in its bss should be fine and
would eliminate all the risks of wrong code to load a value into it.
Alternatively put it in initialized data with the desired value.

Rich

Re: Regression with arm in next with stack protector

[Russell King - ARM Linux]

On Tue, Mar 27, 2018 at 10:04:10AM +0100, Russell King - ARM Linux wrote:
> On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> > Hi,
> > 
> > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > compressed boot phase") breaks booting on arm.
> > 
> > This is all I get from the bootloader on omap3:
> > 
> > Starting kernel ...
> > 
> > data abort
> > pc : [<810002d0>]  lr : [<100110a8>]
> > reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> > sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> > r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> > r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> > r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > Resetting CPU ...
> > 
> > resetting ...
> 
> The reason for this is the following code that was introduced by the
> referenced patch:
> 
> +   ldr r0, =__stack_chk_guard
> +   ldr r1, =0x000a0dff
> +   str r1, [r0]
> 
> This uses the absolute address of __stack_chk_guard in the decompressor,
> which is a self-relocatable image.  As with all constructs like the
> above, this absolute address doesn't get fixed up, and so it ends up
> pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
> and the decompressor looks to be around 0x8100.
> 
> Such constructs can not be used in the decompressor for exactly this
> reason - they need to use PC-relative addressing instead just like
> everything else does in head.S.

Is there any reason we can't do this in misc.c:

const unsigned int __stack_chk_guard = 0x000a0dff;

?  That would save having runtime code to set that value up, and after
all, it is constant.  Arguments about it potentially ending up at a
fixed offset into the image need not be said - that's already the case
with placing it in the early assembly code, and as has been established,
it needs to be initialised prior to any C code being called.

-- 
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

Re: Regression with arm in next with stack protector

[Russell King - ARM Linux]

On Tue, Mar 27, 2018 at 10:04:10AM +0100, Russell King - ARM Linux wrote:
> On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> > Hi,
> > 
> > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > compressed boot phase") breaks booting on arm.
> > 
> > This is all I get from the bootloader on omap3:
> > 
> > Starting kernel ...
> > 
> > data abort
> > pc : [<810002d0>]  lr : [<100110a8>]
> > reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> > sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> > r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> > r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> > r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > Resetting CPU ...
> > 
> > resetting ...
> 
> The reason for this is the following code that was introduced by the
> referenced patch:
> 
> +   ldr r0, =__stack_chk_guard
> +   ldr r1, =0x000a0dff
> +   str r1, [r0]
> 
> This uses the absolute address of __stack_chk_guard in the decompressor,
> which is a self-relocatable image.  As with all constructs like the
> above, this absolute address doesn't get fixed up, and so it ends up
> pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
> and the decompressor looks to be around 0x8100.
> 
> Such constructs can not be used in the decompressor for exactly this
> reason - they need to use PC-relative addressing instead just like
> everything else does in head.S.

Is there any reason we can't do this in misc.c:

const unsigned int __stack_chk_guard = 0x000a0dff;

?  That would save having runtime code to set that value up, and after
all, it is constant.  Arguments about it potentially ending up at a
fixed offset into the image need not be said - that's already the case
with placing it in the early assembly code, and as has been established,
it needs to be initialised prior to any C code being called.

-- 
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

Re: Regression with arm in next with stack protector

[Russell King - ARM Linux]

On Tue, Mar 27, 2018 at 10:04:10AM +0100, Russell King - ARM Linux wrote:
> On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> > Hi,
> > 
> > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > compressed boot phase") breaks booting on arm.
> > 
> > This is all I get from the bootloader on omap3:
> > 
> > Starting kernel ...
> > 
> > data abort
> > pc : [<810002d0>]  lr : [<100110a8>]
> > reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> > sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> > r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> > r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> > r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > Resetting CPU ...
> > 
> > resetting ...
> 
> The reason for this is the following code that was introduced by the
> referenced patch:
> 
> +   ldr r0, =__stack_chk_guard
> +   ldr r1, =0x000a0dff
> +   str r1, [r0]
> 
> This uses the absolute address of __stack_chk_guard in the decompressor,
> which is a self-relocatable image.  As with all constructs like the
> above, this absolute address doesn't get fixed up, and so it ends up
> pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
> and the decompressor looks to be around 0x8100.
> 
> Such constructs can not be used in the decompressor for exactly this
> reason - they need to use PC-relative addressing instead just like
> everything else does in head.S.

I guess someone's not going to see my message to correct their patch:

  che...@lemote.com
SMTP error from remote mail server after end of data:
host mxbiz1.qq.com [184.105.206.88]: 550 Mail content denied.
http://service.exmail.qq.com/cgi-bin/help?subtype=1&=20022&=1000726

-- 
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

Re: Regression with arm in next with stack protector

[Russell King - ARM Linux]

On Tue, Mar 27, 2018 at 10:04:10AM +0100, Russell King - ARM Linux wrote:
> On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> > Hi,
> > 
> > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > compressed boot phase") breaks booting on arm.
> > 
> > This is all I get from the bootloader on omap3:
> > 
> > Starting kernel ...
> > 
> > data abort
> > pc : [<810002d0>]  lr : [<100110a8>]
> > reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> > sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> > r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> > r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> > r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > Resetting CPU ...
> > 
> > resetting ...
> 
> The reason for this is the following code that was introduced by the
> referenced patch:
> 
> +   ldr r0, =__stack_chk_guard
> +   ldr r1, =0x000a0dff
> +   str r1, [r0]
> 
> This uses the absolute address of __stack_chk_guard in the decompressor,
> which is a self-relocatable image.  As with all constructs like the
> above, this absolute address doesn't get fixed up, and so it ends up
> pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
> and the decompressor looks to be around 0x8100.
> 
> Such constructs can not be used in the decompressor for exactly this
> reason - they need to use PC-relative addressing instead just like
> everything else does in head.S.

I guess someone's not going to see my message to correct their patch:

  che...@lemote.com
SMTP error from remote mail server after end of data:
host mxbiz1.qq.com [184.105.206.88]: 550 Mail content denied.
http://service.exmail.qq.com/cgi-bin/help?subtype=1&=20022&=1000726

-- 
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

Re: Regression with arm in next with stack protector

[Russell King - ARM Linux]

On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> Hi,
> 
> Looks like commit 5638790dadae ("zboot: fix stack protector in
> compressed boot phase") breaks booting on arm.
> 
> This is all I get from the bootloader on omap3:
> 
> Starting kernel ...
> 
> data abort
> pc : [<810002d0>]  lr : [<100110a8>]
> reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> Resetting CPU ...
> 
> resetting ...

The reason for this is the following code that was introduced by the
referenced patch:

+   ldr r0, =__stack_chk_guard
+   ldr r1, =0x000a0dff
+   str r1, [r0]

This uses the absolute address of __stack_chk_guard in the decompressor,
which is a self-relocatable image.  As with all constructs like the
above, this absolute address doesn't get fixed up, and so it ends up
pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
and the decompressor looks to be around 0x8100.

Such constructs can not be used in the decompressor for exactly this
reason - they need to use PC-relative addressing instead just like
everything else does in head.S.

-- 
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

Re: Regression with arm in next with stack protector

[Russell King - ARM Linux]

On Fri, Mar 23, 2018 at 11:14:53AM -0700, Tony Lindgren wrote:
> Hi,
> 
> Looks like commit 5638790dadae ("zboot: fix stack protector in
> compressed boot phase") breaks booting on arm.
> 
> This is all I get from the bootloader on omap3:
> 
> Starting kernel ...
> 
> data abort
> pc : [<810002d0>]  lr : [<100110a8>]
> reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
> sp : 81467c18  ip : 81466bf0 fp : 81466bf0
> r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
> r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
> r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> Resetting CPU ...
> 
> resetting ...

The reason for this is the following code that was introduced by the
referenced patch:

+   ldr r0, =__stack_chk_guard
+   ldr r1, =0x000a0dff
+   str r1, [r0]

This uses the absolute address of __stack_chk_guard in the decompressor,
which is a self-relocatable image.  As with all constructs like the
above, this absolute address doesn't get fixed up, and so it ends up
pointing at invalid memory (in this case 0x466bf8) vs RAM at 0x8000,
and the decompressor looks to be around 0x8100.

Such constructs can not be used in the decompressor for exactly this
reason - they need to use PC-relative addressing instead just like
everything else does in head.S.

-- 
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

Re: Regression with arm in next with stack protector

[陈华才]

Allwinner devices are OK, Exynos devices (except 4210) are OK.
 
-- Original --
From:  "Tony Lindgren";
Date:  Mon, Mar 26, 2018 11:37 PM
To:  "陈华才";
Cc:  "Andrew Morton"; "Fabio 
Estevam"; "Stephen Rothwell"; "Rich 
Felker"; "Russell King"; "Yoshinori 
Sato"; 
"linux-kernel"; "Ralf 
Baechle"; "linux-omap"; "James 
Hogan"; "moderated list:ARM/FREESCALE IMX / MXC ARM 
ARCHITECTURE";
Subject:  Re: Regression with arm in next with stack protector
 
* 陈华才  [180326 06:59]:
> Hi, Tony and Fabio,
> 
> Could you please upload your kernel binary to somewhere for me? I don't 
> understand why some ARM boards is OK while others are broken.

Well the kernel I'm testing is just current Linux next cross
compiled omap2plus_defconfig kernel. I do have few more config
options enabled like LOCKDEP and DEBUG_ATOMIC_SLEEP, but I
doubt they matter here :)

Then I'm using gcc-7.3.0 and binutils-2.30 built with the
buildall.git scripts:

git://git.infradead.org/users/segher/buildall.git

If you still need binaries, let me know.

Do you know which arm devices are working with your patch?

Regards,

Tony

Re: Regression with arm in next with stack protector

[陈华才]

Allwinner devices are OK, Exynos devices (except 4210) are OK.
 
-- Original --
From:  "Tony Lindgren";
Date:  Mon, Mar 26, 2018 11:37 PM
To:  "陈华才";
Cc:  "Andrew Morton"; "Fabio 
Estevam"; "Stephen Rothwell"; "Rich 
Felker"; "Russell King"; "Yoshinori 
Sato"; 
"linux-kernel"; "Ralf 
Baechle"; "linux-omap"; "James 
Hogan"; "moderated list:ARM/FREESCALE IMX / MXC ARM 
ARCHITECTURE";
Subject:  Re: Regression with arm in next with stack protector
 
* 陈华才  [180326 06:59]:
> Hi, Tony and Fabio,
> 
> Could you please upload your kernel binary to somewhere for me? I don't 
> understand why some ARM boards is OK while others are broken.

Well the kernel I'm testing is just current Linux next cross
compiled omap2plus_defconfig kernel. I do have few more config
options enabled like LOCKDEP and DEBUG_ATOMIC_SLEEP, but I
doubt they matter here :)

Then I'm using gcc-7.3.0 and binutils-2.30 built with the
buildall.git scripts:

git://git.infradead.org/users/segher/buildall.git

If you still need binaries, let me know.

Do you know which arm devices are working with your patch?

Regards,

Tony

Re: Regression with arm in next with stack protector

[Tony Lindgren]

* 陈华才  [180326 06:59]:
> Hi, Tony and Fabio,
> 
> Could you please upload your kernel binary to somewhere for me? I don't 
> understand why some ARM boards is OK while others are broken.

Well the kernel I'm testing is just current Linux next cross
compiled omap2plus_defconfig kernel. I do have few more config
options enabled like LOCKDEP and DEBUG_ATOMIC_SLEEP, but I
doubt they matter here :)

Then I'm using gcc-7.3.0 and binutils-2.30 built with the
buildall.git scripts:

git://git.infradead.org/users/segher/buildall.git

If you still need binaries, let me know.

Do you know which arm devices are working with your patch?

Regards,

Tony

Re: Regression with arm in next with stack protector

[Tony Lindgren]

* 陈华才  [180326 06:59]:
> Hi, Tony and Fabio,
> 
> Could you please upload your kernel binary to somewhere for me? I don't 
> understand why some ARM boards is OK while others are broken.

Well the kernel I'm testing is just current Linux next cross
compiled omap2plus_defconfig kernel. I do have few more config
options enabled like LOCKDEP and DEBUG_ATOMIC_SLEEP, but I
doubt they matter here :)

Then I'm using gcc-7.3.0 and binutils-2.30 built with the
buildall.git scripts:

git://git.infradead.org/users/segher/buildall.git

If you still need binaries, let me know.

Do you know which arm devices are working with your patch?

Regards,

Tony

Re: Regression with arm in next with stack protector

[陈华才]

Hi, Tony and Fabio,

Could you please upload your kernel binary to somewhere for me? I don't 
understand why some ARM boards is OK while others are broken.

Huacai
 
-- Original --
From:  "Andrew Morton";
Date:  Sat, Mar 24, 2018 03:15 AM
To:  "Fabio Estevam";
Cc:  "Tony Lindgren"; "Huacai Chen"; 
"Stephen Rothwell"; "Rich Felker"; 
"Russell King"; "Yoshinori 
Sato"; 
"linux-kernel"; "Ralf 
Baechle"; "linux-omap"; "James 
Hogan"; "moderated list:ARM/FREESCALE IMX / MXC ARM 
ARCHITECTURE";
Subject:  Re: Regression with arm in next with stack protector
 
On Fri, 23 Mar 2018 15:45:27 -0300 Fabio Estevam  wrote:

> On Fri, Mar 23, 2018 at 3:14 PM, Tony Lindgren  wrote:
> > Hi,
> >
> > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > compressed boot phase") breaks booting on arm.
> 
> Yes, almost all imx are broken in linux-next due to this commit:
> https://kernelci.org/soc/imx/job/next/kernel/next-20180323/

Thanks, I dropped it and I expect Stephen will also do that.

Re: Regression with arm in next with stack protector

[陈华才]

Hi, Tony and Fabio,

Could you please upload your kernel binary to somewhere for me? I don't 
understand why some ARM boards is OK while others are broken.

Huacai
 
-- Original --
From:  "Andrew Morton";
Date:  Sat, Mar 24, 2018 03:15 AM
To:  "Fabio Estevam";
Cc:  "Tony Lindgren"; "Huacai Chen"; 
"Stephen Rothwell"; "Rich Felker"; 
"Russell King"; "Yoshinori 
Sato"; 
"linux-kernel"; "Ralf 
Baechle"; "linux-omap"; "James 
Hogan"; "moderated list:ARM/FREESCALE IMX / MXC ARM 
ARCHITECTURE";
Subject:  Re: Regression with arm in next with stack protector
 
On Fri, 23 Mar 2018 15:45:27 -0300 Fabio Estevam  wrote:

> On Fri, Mar 23, 2018 at 3:14 PM, Tony Lindgren  wrote:
> > Hi,
> >
> > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > compressed boot phase") breaks booting on arm.
> 
> Yes, almost all imx are broken in linux-next due to this commit:
> https://kernelci.org/soc/imx/job/next/kernel/next-20180323/

Thanks, I dropped it and I expect Stephen will also do that.

Re: Regression with arm in next with stack protector

[Stephen Rothwell]

Hi Andrew,

On Fri, 23 Mar 2018 12:15:30 -0700 Andrew Morton  
wrote:
>
> On Fri, 23 Mar 2018 15:45:27 -0300 Fabio Estevam  wrote:
> 
> > On Fri, Mar 23, 2018 at 3:14 PM, Tony Lindgren  wrote:  
> > > Hi,
> > >
> > > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > > compressed boot phase") breaks booting on arm.  
> > 
> > Yes, almost all imx are broken in linux-next due to this commit:
> > https://kernelci.org/soc/imx/job/next/kernel/next-20180323/  
> 
> Thanks, I dropped it and I expect Stephen will also do that.

OK, dropped.

-- 
Cheers,
Stephen Rothwell


pgpcHpQf7Ic6c.pgp
Description: OpenPGP digital signature

Re: Regression with arm in next with stack protector

[Stephen Rothwell]

Hi Andrew,

On Fri, 23 Mar 2018 12:15:30 -0700 Andrew Morton  
wrote:
>
> On Fri, 23 Mar 2018 15:45:27 -0300 Fabio Estevam  wrote:
> 
> > On Fri, Mar 23, 2018 at 3:14 PM, Tony Lindgren  wrote:  
> > > Hi,
> > >
> > > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > > compressed boot phase") breaks booting on arm.  
> > 
> > Yes, almost all imx are broken in linux-next due to this commit:
> > https://kernelci.org/soc/imx/job/next/kernel/next-20180323/  
> 
> Thanks, I dropped it and I expect Stephen will also do that.

OK, dropped.

-- 
Cheers,
Stephen Rothwell


pgpcHpQf7Ic6c.pgp
Description: OpenPGP digital signature

Re: Regression with arm in next with stack protector

[Andrew Morton]

On Fri, 23 Mar 2018 15:45:27 -0300 Fabio Estevam  wrote:

> On Fri, Mar 23, 2018 at 3:14 PM, Tony Lindgren  wrote:
> > Hi,
> >
> > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > compressed boot phase") breaks booting on arm.
> 
> Yes, almost all imx are broken in linux-next due to this commit:
> https://kernelci.org/soc/imx/job/next/kernel/next-20180323/

Thanks, I dropped it and I expect Stephen will also do that.

Re: Regression with arm in next with stack protector

[Andrew Morton]

On Fri, 23 Mar 2018 15:45:27 -0300 Fabio Estevam  wrote:

> On Fri, Mar 23, 2018 at 3:14 PM, Tony Lindgren  wrote:
> > Hi,
> >
> > Looks like commit 5638790dadae ("zboot: fix stack protector in
> > compressed boot phase") breaks booting on arm.
> 
> Yes, almost all imx are broken in linux-next due to this commit:
> https://kernelci.org/soc/imx/job/next/kernel/next-20180323/

Thanks, I dropped it and I expect Stephen will also do that.

Re: Regression with arm in next with stack protector

[Fabio Estevam]

On Fri, Mar 23, 2018 at 3:14 PM, Tony Lindgren  wrote:
> Hi,
>
> Looks like commit 5638790dadae ("zboot: fix stack protector in
> compressed boot phase") breaks booting on arm.

Yes, almost all imx are broken in linux-next due to this commit:
https://kernelci.org/soc/imx/job/next/kernel/next-20180323/

Re: Regression with arm in next with stack protector

[Fabio Estevam]

On Fri, Mar 23, 2018 at 3:14 PM, Tony Lindgren  wrote:
> Hi,
>
> Looks like commit 5638790dadae ("zboot: fix stack protector in
> compressed boot phase") breaks booting on arm.

Yes, almost all imx are broken in linux-next due to this commit:
https://kernelci.org/soc/imx/job/next/kernel/next-20180323/

Regression with arm in next with stack protector

[Tony Lindgren]

Hi,

Looks like commit 5638790dadae ("zboot: fix stack protector in
compressed boot phase") breaks booting on arm.

This is all I get from the bootloader on omap3:

Starting kernel ...

data abort
pc : [<810002d0>]  lr : [<100110a8>]
reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
sp : 81467c18  ip : 81466bf0 fp : 81466bf0
r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
Resetting CPU ...

resetting ...

Regards,

Tony

Regression with arm in next with stack protector

[Tony Lindgren]

Hi,

Looks like commit 5638790dadae ("zboot: fix stack protector in
compressed boot phase") breaks booting on arm.

This is all I get from the bootloader on omap3:

Starting kernel ...

data abort
pc : [<810002d0>]  lr : [<100110a8>]
reloc pc : [<9d6002d0>]lr : [<2c6110a8>]
sp : 81467c18  ip : 81466bf0 fp : 81466bf0
r10: 80fc2c40  r9 : 81000258 r8 : 86fec000
r7 :   r6 : 81466bf8 r5 :   r4 : 80008000
r3 : 81466c14  r2 : 81466c18 r1 : 000a0dff  r0 : 00466bf8
Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
Resetting CPU ...

resetting ...

Regards,

Tony