sudo-project / sudo

Utility to execute a command as another user
https://www.sudo.ws
Other
1.17k stars 210 forks source link

`-fstack-clash-protection` detection false-positive on ARMv5 with Thumb-1 #191

Closed rossburton closed 1 year ago

rossburton commented 1 year ago

If I build sudo for armv5 with thumb-1 enabled:

libtool: compile:  arm-poky-linux-gnueabi-gcc -march=armv5te -mthumb -fstack-protector-strong -O2 -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Werror=format-security --sysroot=/builds/engineering/yocto/meta-arm/work/build/tmp/work/armv5te-poky-linux-gnueabi/sudo/1.9.11p3-r0/recipe-sysroot -c -I../../../sudo-1.9.11p3/include -I../.. -I. -I../../../sudo-1.9.11p3/lib/util -D_PATH_SUDO_CONF=\"/etc/sudo.conf\" -DZLIB_CONST -D_FORTIFY_SOURCE=2 -DDEFAULT_TEXT_DOMAIN=\"sudo\" -O2 -pipe -g -feliminate-unused-debug-types -fvisibility=hidden -fstack-protector-strong -fstack-clash-protection ../../../sudo-1.9.11p3/lib/util/gidlist.c  -fPIC -DPIC -o .libs/gidlist.o
../../../sudo-1.9.11p3/lib/util/fatal.c: In function 'do_cleanup':
../../../sudo-1.9.11p3/lib/util/fatal.c:73:1: sorry, unimplemented: '-fstack-check=specific' for Thumb-1

It looks like the configure check for -fstack-clash-protection isn't quite being comprehensive enough? Is it passing all of the CFLAGS that are used in the build?

millert commented 1 year ago

Currently the configure check only verifies that -fstack-clash-protection is accepted by the linker. It should probably test that it is accepted when compiling a trivial program.

millert commented 1 year ago

After a bit of fiddling I was able to trigger this on an arm VM. The difficulty is that the gcc front-end may accept -fstack-clash-protection even when the machine-dependent code does not support it. Using a test program with a large stack allocation causes the compiler to emit the protection code, or fail as in your case.

Can you verify that a checkout that includes 3662175 works for you?

millert commented 1 year ago

This was fixed in sudo 1.9.12p1.

rossburton commented 1 year ago

Sorry for not getting around to testing your fix. It looks right, I'll get back to you shortly if it breaks as the upgrade has been queued.