On Wed, Jun 07, 2023 at 08:31:58PM +0200, Christian Marangi wrote:
On Wed, Jun 07, 2023 at 02:19:51PM -0700, Kees Cook wrote:
On Wed, Jun 07, 2023 at 04:42:27PM +0200, Christian Marangi wrote:
Dynamically allocate note.data in parse_elf_properties to fix compilation warning on some arch.
I'd rather avoid dynamic allocation as much as possible in the exec path, but we can balance it against how much it may happen.
I guess there isn't a good way to handle this other than static global variables and kmalloc. But check the arch question for additional info on the case.
On some arch note.data exceed the stack limit for a single function and this cause the following compilation warning: fs/binfmt_elf.c: In function 'parse_elf_properties.isra': fs/binfmt_elf.c:821:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] 821 | } | ^ cc1: all warnings being treated as errors
Which architectures see this warning?
This is funny. On OpenWRT we are enforcing WERROR and we had FRAME_WARN hardcoded to 1024. (the option is set to 2048 on 64bit arch)
Ah-ha. Okay, I was wondering how you got that. :)
ARCH_USE_GNU_PROPERTY is set only on arm64 that have a FRAME_WARN set to 2048.
So this was triggered by building arm64 with FRAME_WARN set to 1024.
Now with the configuration of 2048 the stack warn is not triggered, but I wonder if it may happen to have a 32bit system with ARCH_USE_GNU_PROPERTY. That would effectively trigger the warning.
So this is effectively a patch that fix a currently not possible configuration, since:
!IS_ENABLED(CONFIG_ARCH_USE_GNU_PROPERTY) will result in node.data effectively never allocated by the compiler are the function will return 0 on everything that doesn't have CONFIG_ARCH_USE_GNU_PROPERTY.
Fix this by dynamically allocating the array. Update the sizeof of the union to the biggest element allocated.
How common are these notes? I assume they're very common; I see them even in /bin/true:
$ readelf -lW /bin/true | grep PROP GNU_PROPERTY 0x000338 0x0000000000000338 0x0000000000000338 0x000030 0x000030 R 0x8
--
Is there a way to check if this kmalloc actually cause perf regression?
I don't have a good benchmark besides just an exec loop. But since this isn't reachable in a regular config, I'd rather keep things how there already are.
-Kees