Bug#886506: busybox sh broken on i386 with glibc 2.26, leads to kernel panic

[Denys Vlasenko]

On Sun, Jan 21, 2018 at 12:18 PM, Aurelien Jarno  wrote:
> On 2018-01-21 00:47, Ben Hutchings wrote:
>> On Wed, 17 Jan 2018 12:31:06 +0100 Aurelien Jarno  wrote:
>> [...]
>> > busybox is compiled with -mpreferred-stack-boundary=2 on i386 which has
>> > the same effect of reducing the default stack alignment from 16 bytes to
>> > 2 bytes. This comes from arch/i386/Makefile:
>>
>> The argument is the log2 of the alignment, so this sets alignment to 4
>> bytes - which is compliant with the System V psABI for i386.
>
> This is correct, but it is not compliant with the i386 GCC ABI which
> assumes the stack is 16-byte aligned (not just 4-byte aligned) when the
> call instruction in the caller was executed.

gcc people simply broke ABI just because they could. There were other ways
to accomodate SSE alignment restrictions, such as aligning stack
only in functions which spill vector temporaries to stack,
but gcc chose a disruptive one.

The breakage appears *now* because SSE vector usage inside glibc is increasing.
If your libc is careful to not use SSE needlessly, it would work fine.

As to how to fix this for busybox, let's have a config option to disable it.

Bug#886506: busybox sh broken on i386 with glibc 2.26, leads to kernel panic

[Ben Hutchings]

On Sun, 2018-01-21 at 12:18 +0100, Aurelien Jarno wrote:
> On 2018-01-21 00:47, Ben Hutchings wrote:
> > On Wed, 17 Jan 2018 12:31:06 +0100 Aurelien Jarno  
> > wrote:
> > [...]
> > > busybox is compiled with -mpreferred-stack-boundary=2 on i386 which has
> > > the same effect of reducing the default stack alignment from 16 bytes to
> > > 2 bytes. This comes from arch/i386/Makefile:
> > 
> > The argument is the log2 of the alignment, so this sets alignment to 4
> > bytes - which is compliant with the System V psABI for i386.
> 
> This is correct, but it is not compliant with the i386 GCC ABI which
> assumes the stack is 16-byte aligned (not just 4-byte aligned) when the
> call instruction in the caller was executed.

Yes, that's what I think the bug is - gcc's default ABI for
i386-linux-gnu was quietly changed.

> > Any assumption of 16-byte stack alignment in glibc on i386 will break
> > not only busybox but most binaries built with old versions of gcc
> > (before 4.2, if the comment in busybox is correct).  So this really
> > ought to be fixed there.
> 
> The 16-byte stack alignment in glibc on i386 comes from a GCC 7
> regression, reported as bug #887327. It has been fixed in the upstream
> gcc-7 branch in the mean time.
> 
> > I think that any libraries that need to maintain backward binary
> > compatibility will need to be compiled with the option
> > -mincoming-stack-boundary=2.  gcc will then fix up the stack alignment
> > in functions that need greater alignment for local variables.
> 
> If we allow any binary to be built with -mpreferred-stack-boundary=2,
> we need to build *all* libraries with -mincoming-stack-boundary=2, not
> only the ones that need to maintain backward binary compatibility.

That's true, but we could also specify that if you mess with stack
alignment you can only use libc6 and other libraries whose ABI pre-
dates the change in gcc.  (I don't know where we would do that; I
haven't seen any formal specifications of Debian architectures.)

> In that case we should make it the default in GCC.

I think there are quite few libraries where this would be necessary.  I
could be wrong.

Ben.

-- 
Ben Hutchings
Reality is just a crutch for people who can't handle science fiction.



signature.asc
Description: This is a digitally signed message part

Bug#886506: busybox sh broken on i386 with glibc 2.26, leads to kernel panic

[Aurelien Jarno]

On 2018-01-21 00:47, Ben Hutchings wrote:
> On Wed, 17 Jan 2018 12:31:06 +0100 Aurelien Jarno  wrote:
> [...]
> > busybox is compiled with -mpreferred-stack-boundary=2 on i386 which has
> > the same effect of reducing the default stack alignment from 16 bytes to
> > 2 bytes. This comes from arch/i386/Makefile:
> 
> The argument is the log2 of the alignment, so this sets alignment to 4
> bytes - which is compliant with the System V psABI for i386.

This is correct, but it is not compliant with the i386 GCC ABI which
assumes the stack is 16-byte aligned (not just 4-byte aligned) when the
call instruction in the caller was executed.

> Any assumption of 16-byte stack alignment in glibc on i386 will break
> not only busybox but most binaries built with old versions of gcc
> (before 4.2, if the comment in busybox is correct).  So this really
> ought to be fixed there.

The 16-byte stack alignment in glibc on i386 comes from a GCC 7
regression, reported as bug #887327. It has been fixed in the upstream
gcc-7 branch in the mean time.

> I think that any libraries that need to maintain backward binary
> compatibility will need to be compiled with the option
> -mincoming-stack-boundary=2.  gcc will then fix up the stack alignment
> in functions that need greater alignment for local variables.

If we allow any binary to be built with -mpreferred-stack-boundary=2,
we need to build *all* libraries with -mincoming-stack-boundary=2, not
only the ones that need to maintain backward binary compatibility. In
that case we should make it the default in GCC.

Aurelien

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net


signature.asc
Description: PGP signature

Bug#886506: busybox sh broken on i386 with glibc 2.26, leads to kernel panic

[Ben Hutchings]

On Wed, 17 Jan 2018 12:31:06 +0100 Aurelien Jarno  wrote:
[...]
> busybox is compiled with -mpreferred-stack-boundary=2 on i386 which has
> the same effect of reducing the default stack alignment from 16 bytes to
> 2 bytes. This comes from arch/i386/Makefile:

The argument is the log2 of the alignment, so this sets alignment to 4
bytes - which is compliant with the System V psABI for i386.

Any assumption of 16-byte stack alignment in glibc on i386 will break
not only busybox but most binaries built with old versions of gcc
(before 4.2, if the comment in busybox is correct).  So this really
ought to be fixed there.

I think that any libraries that need to maintain backward binary
compatibility will need to be compiled with the option
-mincoming-stack-boundary=2.  gcc will then fix up the stack alignment
in functions that need greater alignment for local variables.

Ben.

-- 
Ben Hutchings
Reality is just a crutch for people who can't handle science fiction.


signature.asc
Description: This is a digitally signed message part

Bug#886506: busybox sh broken on i386 with glibc 2.26, leads to kernel panic

[Denys Vlasenko]

On Fri, Jan 19, 2018 at 10:58 AM, Chris Boot  wrote:
> On 18/01/18 23:44, Cyril Brulebois wrote:
>> Raphael Hertzog  (2018-01-17):
>>> On Wed, 17 Jan 2018, Aurelien Jarno wrote:
 busybox is compiled with -mpreferred-stack-boundary=2 on i386 which has
 the same effect of reducing the default stack alignment from 16 bytes to
 2 bytes. This comes from arch/i386/Makefile:

 |  # -mpreferred-stack-boundary=2 is essential in preventing gcc 4.2.x
 |  # from aligning stack to 16 bytes. (Which is gcc's way of supporting 
 SSE).
 |  CFLAGS += $(call cc-option,-march=i386 -mpreferred-stack-boundary=2,)

 I don't really get why it is essential to prevent gcc from aligning
 stack to 16 bytes, anyway this is a bad idea. Removing this option just
 fixes the issue.
>
> Hi all,
>
> I've added the busybox mailing list and Denys Vlasenko. Denys: can you
> explain in more detail why this is/was necessary and whether it's
> sensible to remove this these days, given the issues it seems to cause?
>
>> Oh wow, that's an old commit:
>> | commit 65b8cfb2a0b6854271dbd8baf5203896223cd4ce
>> | Author: Denis Vlasenko 
>> | Date:   Mon Jul 23 21:05:06 2007 +
>> |
>> | add comment why preferred stack boundary is 4 on i386
>>
>>> I confirm that rebuilding busybox with this option dropped fixed the issue
>>> for me. I uploaded a fixed busybox to Kali. I'm happy to do the same for
>>> Debian if it can help the current maintainers.
>>
>> Fixing it ASAP would look good to me (unless you here differently from
>> Chris or Christoph); it would be great to ping upstream to propagate
>> Aurélien's comments too.
>
> I'd like to get a response from Denys about this first but I don't have
> any particular objection to patching this for Debian; I just want to
> understand better why this was done upstream before simply reverting it.

It adds stack alignment code to almost every function, with ~3.5% size growth.
If function already has locals, this takes the form of allocating more
local space (to make stack aligned to 16 bytes), which in turn makes
some instruction longer. Example:

 :
 53 push   %ebx
-83 ec 04   sub$0x4,%esp
 89 c3  mov%eax,%ebx
+83 ec 18   sub$0x18,%esp
 e8 fc ff ff ff call   7  R_386_PC32
.text.machtime
-89 04 24   mov%eax,(%esp)
-89 e2  mov%esp,%edx
+8d 54 24 14lea0x14(%esp),%edx
 b9 04 00 00 00 mov$0x4,%ecx
+89 44 24 14mov%eax,0x14(%esp)
 89 d8  mov%ebx,%eax
-e8 fc ff ff ff call   18 
+e8 fc ff ff ff call   1b   R_386_PC32
   full_write
-58 pop%eax
+83 c4 18   add$0x18,%esp
 5b pop%ebx
 c3 ret

"make bloatcheck":

function old new   delta
hdparm_main 45824948+366
identify43864682+296
wget_main   26012831+230
bootchartd_main 17981974+176
make_device 21832339+156
handle_incoming_and_exit28603008+148
common_traceroute_main  37123858+146
redirect11211264+143
evaluate34873625+138
udhcpc_main 27062843+137
getty_main  15491680+131
mkfs_ext2_main  25992727+128
do_cmd  44664593+127
process_files   22392365+126
ife_print   12871413+126
syslogd_main19462070+124
unzip_main  27092832+123
sv  13091432+123
parse_stream26832800+117
fdisk_main  28442961+117
exec_builtin14881603+115
dpkg_main   29353046+111
setpriv_main 9541064+110
update_passwd   14351542+107
read_line_input 24782582+104
mount_main  12341337+103
copy_file  

Bug#886506: busybox sh broken on i386 with glibc 2.26, leads to kernel panic

[Chris Boot]

On 18/01/18 23:44, Cyril Brulebois wrote:
> Raphael Hertzog  (2018-01-17):
>> On Wed, 17 Jan 2018, Aurelien Jarno wrote:
>>> busybox is compiled with -mpreferred-stack-boundary=2 on i386 which has
>>> the same effect of reducing the default stack alignment from 16 bytes to
>>> 2 bytes. This comes from arch/i386/Makefile:
>>>
>>> |  # -mpreferred-stack-boundary=2 is essential in preventing gcc 4.2.x
>>> |  # from aligning stack to 16 bytes. (Which is gcc's way of supporting 
>>> SSE).
>>> |  CFLAGS += $(call cc-option,-march=i386 -mpreferred-stack-boundary=2,)
>>>
>>> I don't really get why it is essential to prevent gcc from aligning
>>> stack to 16 bytes, anyway this is a bad idea. Removing this option just
>>> fixes the issue.

Hi all,

I've added the busybox mailing list and Denys Vlasenko. Denys: can you
explain in more detail why this is/was necessary and whether it's
sensible to remove this these days, given the issues it seems to cause?

> Oh wow, that's an old commit:
> | commit 65b8cfb2a0b6854271dbd8baf5203896223cd4ce
> | Author: Denis Vlasenko 
> | Date:   Mon Jul 23 21:05:06 2007 +
> | 
> | add comment why preferred stack boundary is 4 on i386
> 
>> I confirm that rebuilding busybox with this option dropped fixed the issue
>> for me. I uploaded a fixed busybox to Kali. I'm happy to do the same for
>> Debian if it can help the current maintainers.
> 
> Fixing it ASAP would look good to me (unless you here differently from
> Chris or Christoph); it would be great to ping upstream to propagate
> Aurélien's comments too.

I'd like to get a response from Denys about this first but I don't have
any particular objection to patching this for Debian; I just want to
understand better why this was done upstream before simply reverting it.

We also have a new upstream release of busybox to push into unstable, so
it's tempting to roll this tweak in with that.

Cheers,
Chris

-- 
Chris Boot
bo...@debian.org



signature.asc
Description: OpenPGP digital signature

Bug#886506: busybox sh broken on i386 with glibc 2.26, leads to kernel panic

[Cyril Brulebois]

Raphael Hertzog  (2018-01-17):
> On Wed, 17 Jan 2018, Aurelien Jarno wrote:
> > busybox is compiled with -mpreferred-stack-boundary=2 on i386 which has
> > the same effect of reducing the default stack alignment from 16 bytes to
> > 2 bytes. This comes from arch/i386/Makefile:
> > 
> > |  # -mpreferred-stack-boundary=2 is essential in preventing gcc 4.2.x
> > |  # from aligning stack to 16 bytes. (Which is gcc's way of supporting 
> > SSE).
> > |  CFLAGS += $(call cc-option,-march=i386 -mpreferred-stack-boundary=2,)
> > 
> > I don't really get why it is essential to prevent gcc from aligning
> > stack to 16 bytes, anyway this is a bad idea. Removing this option just
> > fixes the issue.

Oh wow, that's an old commit:
| commit 65b8cfb2a0b6854271dbd8baf5203896223cd4ce
| Author: Denis Vlasenko 
| Date:   Mon Jul 23 21:05:06 2007 +
| 
| add comment why preferred stack boundary is 4 on i386

> I confirm that rebuilding busybox with this option dropped fixed the issue
> for me. I uploaded a fixed busybox to Kali. I'm happy to do the same for
> Debian if it can help the current maintainers.

Fixing it ASAP would look good to me (unless you here differently from
Chris or Christoph); it would be great to ping upstream to propagate
Aurélien's comments too.


Cheers,
-- 
Cyril Brulebois (k...@debian.org)
D-I release manager -- Release team member -- Freelance Consultant


signature.asc
Description: PGP signature

Bug#886506: busybox sh broken on i386 with glibc 2.26, leads to kernel panic

[Raphael Hertzog]

Hello,

On Wed, 17 Jan 2018, Aurelien Jarno wrote:
> busybox is compiled with -mpreferred-stack-boundary=2 on i386 which has
> the same effect of reducing the default stack alignment from 16 bytes to
> 2 bytes. This comes from arch/i386/Makefile:
> 
> |  # -mpreferred-stack-boundary=2 is essential in preventing gcc 4.2.x
> |  # from aligning stack to 16 bytes. (Which is gcc's way of supporting SSE).
> |  CFLAGS += $(call cc-option,-march=i386 -mpreferred-stack-boundary=2,)
> 
> I don't really get why it is essential to prevent gcc from aligning
> stack to 16 bytes, anyway this is a bad idea. Removing this option just
> fixes the issue.

I confirm that rebuilding busybox with this option dropped fixed the issue
for me. I uploaded a fixed busybox to Kali. I'm happy to do the same for
Debian if it can help the current maintainers.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/

Bug#886506: busybox sh broken on i386 with glibc 2.26, leads to kernel panic

[Aurelien Jarno]

control: reassign -1 busybox
control: retitle -1 busybox: wrongly compiled with -mpreferred-stack-boundary=2 
on i386

On 2018-01-17 12:08, Raphael Hertzog wrote:
> Control: reassign -1 src:glibc 2.26-1
> Control: retitle -1 busybox sh broken on i386 with glibc 2.26, leads to 
> kernel panic
> Control: severity -1 serious
> Control: affects -1 + busybox src:linux
> 
> Hello,
> 
> on i386 with glibc 2.26-4, busybox sh is broken:
> 
> $ busybox sh
> [...]
> BusyBox v1.27.2 (Debian 1:1.27.2-2) built-in shell (ash)
> Enter 'help' for a list of built-in commands.
> 
> Segmentation fault
> 
> In the kernel messages, you see this:
> [1097712.640730] traps: busybox[3288] general protection ip:f7e9a51d 
> sp:ff8da68c error:0 in libc-2.26.so[f7d48000+1cd000]
> 
> There's a work-around (the same as the one described in
> #887169):
> 
> $ GLIBC_TUNABLES=glibc.tune.hwcaps=-SSE4_2 busybox sh
> [...]
> BusyBox v1.27.2 (Debian 1:1.27.2-2) built-in shell (ash)
> Enter 'help' for a list of built-in commands.
> 
> ~ $
> 
> Given that busybox's sh is used in the initrd and that the init
> command is a shell script, this will lead to the kernel panic
> shown earlier in this bug report.
> 
> Possible work-arounds in the mean time:
> - disable busybox in the initrd by setting BUSYBOX=n in
>   /etc/initramfs-tools/initramfs.conf (but this is not
>   possible if you use cryptsetup)
> - you can add the "GLIBC_TUNABLES=glibc.tune.hwcaps=-SSE4_2" to the kernel
>   command line so that it's set in the environment of the init script
>   (this will at least let you boot once to fix it permanently)
> - install busybox-static instead of busybox (since it was built with
>   an earlier version of glibc) and rebuild your initrd.
> 
> Aurélien Jaron commented on IRC that this was strange that busybox
> was affected by this bug since the analysis made in #887169 lead to
> believe that only binaries compiled with -mstack-align=4 would be
> affected.

busybox is compiled with -mpreferred-stack-boundary=2 on i386 which has
the same effect of reducing the default stack alignment from 16 bytes to
2 bytes. This comes from arch/i386/Makefile:

|  # -mpreferred-stack-boundary=2 is essential in preventing gcc 4.2.x
|  # from aligning stack to 16 bytes. (Which is gcc's way of supporting SSE).
|  CFLAGS += $(call cc-option,-march=i386 -mpreferred-stack-boundary=2,)

I don't really get why it is essential to prevent gcc from aligning
stack to 16 bytes, anyway this is a bad idea. Removing this option just
fixes the issue.

I am therefore reassigning the bug to busybox.

Aurelien

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net