kconfig.h: Include compiler types to avoid missed struct attributes

PATCH Superseded Kees Cook 2 participants 4 messages Feb 22, 2018 – Feb 23, 2018 1 message not in the archive MIPS

Share & tools

Kees Cook

author

2018-02-22 00:28 UTC #0 lore raw

PATCH v2 1/1 kconfig.h: Include compiler types to avoid missed struct attributes

The header files for some structures could get included in such a way that struct attributes (specifically __randomize_layout from path.h) would be parsed as variable names instead of attributes. This could lead to some instances of a structure being unrandomized, causing nasty GPFs, etc.

This patch makes sure the compiler_types.h header is included in kconfig.h so that we've always got types and struct attributes defined, since kconfig.h is included from the compiler command line.

Reported-by Patrick McLean <[email protected]>

Root-caused-by Maciej S. Szmigiero <[email protected]>

Fixes 3859a271a003 ("randstruct: Mark various structs for randomization")

Signed-off-by Kees Cook <[email protected]>

---
Updated Maciej's tag.

---
 include/linux/kconfig.h | 3 +++
 1 file changed, 3 insertions(+)

include/linux/kconfig.h +3 -0

index fec5076eda91..c5fd4ee776ba 100644
@@ -64,4 +64,7 @@
  */
 #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
 
+/* Make sure we always have all types and struct attributes defined. */
+#include <linux/compiler_types.h>
+
 #endif /* __LINUX_KCONFIG_H */

Signature · 6 lines

2.7.4


-- 
Kees Cook
Pixel Security

Maciej S. Szmigiero

2018-02-22 15:57 UTC #1 lore raw

Quoted 9 lines · Kees Cook

The header files for some structures could get included in such a way that struct attributes (specifically __randomize_layout from path.h) would be parsed as variable names instead of attributes. This could lead to some instances of a structure being unrandomized, causing nasty GPFs, etc.
This patch makes sure the compiler_types.h header is included in kconfig.h so that we've always got types and struct attributes defined, since kconfig.h is included from the compiler command line.

Quoted 5 lines

Reported-by Patrick McLean <[email protected]>
Root-caused-by Maciej S. Szmigiero <[email protected]>
Fixes 3859a271a003 ("randstruct: Mark various structs for randomization")
Suggested-by Linus Torvalds <[email protected]>
Signed-off-by Kees Cook <[email protected]>

I can confirm that this patch fixes the original nfsd GPF issue. Also, struct path members offsets are consistent now between nfs4xdr.s and other files.

Quoted 2 lines

---
Updated Maciej's tag.

Thanks.

Maciej

A message that is not in the archive (sent off-list or not yet received): 201802230613.8bqOOUnw%[email protected]

Kees Cook

author

2018-02-23 00:03 UTC #3 lore raw

Quoted 17 lines · kbuild test robot

Hi Kees,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc2 next-20180222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Kees-Cook/kconfig-h-Include-compiler-types-to-avoid-missed-struct-attributes/20180223-040717
config: mips-jz4740 (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips

Quoted 6 lines

All errors (new ones prefixed by >>):
Quoted 1 line
Quoted 1 line
Error: arch/mips/boot/vmlinux.gz.its:201.1-2 syntax error
FATAL ERROR: Unable to parse input tree
/usr/bin/mkimage: Can't read arch/mips/boot/vmlinux.gz.itb.tmp: Invalid argument /usr/bin/mkimage Can't add hashes to FIT blob

Ugh, the compiler_types.h commit breaks MIPS. I've reproduced this failure; debugging now....

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Kees Cook

author

2018-02-23 00:19 UTC #4 lore raw

Quoted 20 lines · Kees Cook

Quoted 19 lines · kbuild test robot
Hi Kees,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc2 next-20180222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Kees-Cook/kconfig-h-Include-compiler-types-to-avoid-missed-struct-attributes/20180223-040717
config: mips-jz4740 (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips
All errors (new ones prefixed by >>):

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Quoted 1 line
Error: arch/mips/boot/vmlinux.gz.its:201.1-2 syntax error
FATAL ERROR: Unable to parse input tree
/usr/bin/mkimage: Can't read arch/mips/boot/vmlinux.gz.itb.tmp: Invalid argument /usr/bin/mkimage Can't add hashes to FIT blob
Ugh, the compiler_types.h commit breaks MIPS. I've reproduced this failure; debugging now....

For the MIPS folks, this is about 28128c61e08e ("kconfig.h: Include compiler types to avoid missed struct attributes").

This hack fixes it ("struct" definitions were appearing in the resulting its output files, which broke parsing):

arch/mips/boot/Makefile +0 -0

index 1bd5c4f00d19..c22da16d67b8 100644
@@ -126,6 +126,7 @@ $(obj)/vmlinux.its.S: $(addprefix

$(srctree)/arch/mips/$(PLATFORM)/,$(ITS_INPUTS

quiet_cmd_cpp_its_S = ITS $@

       cmd_cpp_its_S = $(CPP) $(cpp_flags) -P -C -o $@ $< \
+                       -D__ASSEMBLY__ \
                        -DKERNEL_NAME="\"Linux $(KERNELRELEASE)\"" \
                        -DVMLINUX_BINARY="\"$(3)\"" \
                        -DVMLINUX_COMPRESSION="\"$(2)\"" \

I assume since this is overloading a .S build rule, the __ASSEMBLY__ define went missing from cpp_flags?

I'm trying to figure out if there is a better solution; ideas welcome!

-Kees

Signature · 2 lines

Kees Cook
Pixel Security