Removing nested function from elfxx-aarch64.c
Checks
Commit Message
Hi Matthieu,
I hope that you do not mind, but I am checking in the attached patch to
remove the nested bfd_is_non_dynamic_elf_object function from
bfd_linear_search_one_with_gnu_property in elfxx-aarch64.c and instead
replace it with an ordinary inline static function.
Whilst nested functions are supported by gcc, they are not by others
(most notably Clang in this case, which is how I encountered this
issue). More than that however they can require the use of an executable
stack (depending upon the compiler and the architecture) and this is
a bad thing from a security point of view.
Cheers
Nick
Comments
On 2025-03-05 10:45, Nick Clifton wrote:
> Hi Matthieu,
>
> I hope that you do not mind, but I am checking in the attached patch to
> remove the nested bfd_is_non_dynamic_elf_object function from
> bfd_linear_search_one_with_gnu_property in elfxx-aarch64.c and instead
> replace it with an ordinary inline static function.
>
> Whilst nested functions are supported by gcc, they are not by others
> (most notably Clang in this case, which is how I encountered this
> issue). More than that however they can require the use of an
> executable
> stack (depending upon the compiler and the architecture) and this is
> a bad thing from a security point of view.
>
> Cheers
> Nick
>
Hi Nick,
The patch looks good to me.
I didn't know that nested functions was a problem, I was treating them
more or less as the lambdas in C++.
I have some patches I need to update then :)
Regards,
Matthieu
Hi Matthieu,
> I didn't know that nested functions was a problem, I was treating them more or less as the lambdas in C++.
> I have some patches I need to update then :)
Heh. :-)
Basically there are two problems with nested functions (in C). The
first is that they are a GNU extension and not supported by all
compilers. The second is that they can lead to the creation of an
executable stack. This second problem usually only happens if you
take the address of a nested function, but there are some older
compilers where the stack was always used.
Cheers
Nick
On Mär 05 2025, Nick Clifton wrote:
> issue). More than that however they can require the use of an executable
> stack (depending upon the compiler and the architecture) and this is
> a bad thing from a security point of view.
Only if the address of the function is passed outside the static scope
(as a callback, for example), which is not used here.
> The second is that they can lead to the creation of an
> executable stack. This second problem usually only happens if you
> take the address of a nested function, but there are some older
> compilers where the stack was always used.
Curious about the latter part of the sentence. Do you have any reference?
Hi Eric,
>> The second is that they can lead to the creation of an
>> executable stack. This second problem usually only happens if you
>> take the address of a nested function, but there are some older
>> compilers where the stack was always used.
>
> Curious about the latter part of the sentence. Do you have any reference?
Darn. I knew that someone would bring me up on this.
The short answer is "no". The longer answer is that I have in mind old
versions of gcc, probably when the support for nested functions had only
recently been added, where an executable stack was always used, even when
it was not strictly necessary. Now I may be mis-remembering here and
blaming gcc incorrectly, but I that is what I had in mind when I wrote that
sentence.
Anyway, whilst nested functions are nice, they do present a problem
when trying to compile the binutils sources with a compiler other than gcc,
and so I think that we should not be using them.
Cheers
Nick
> The short answer is "no". The longer answer is that I have in mind old
> versions of gcc, probably when the support for nested functions had only
> recently been added, where an executable stack was always used, even when
> it was not strictly necessary. Now I may be mis-remembering here and
> blaming gcc incorrectly, but I that is what I had in mind when I wrote that
> sentence.
I see, thanks. My experience is that GCC versions released over the last
couple of decades behave as expected, in other words they do *not* require an
executable stack unless a pointer to a nested function is taken (and a quick
test with GCC 3.4.6 confirms it in a simple case).
> Anyway, whilst nested functions are nice, they do present a problem
> when trying to compile the binutils sources with a compiler other than gcc,
> and so I think that we should not be using them.
No disagreement here, this feature is definitely not portable.
On 07/03/2025 20:09, Eric Botcazou wrote:
>> The short answer is "no". The longer answer is that I have in mind old
>> versions of gcc, probably when the support for nested functions had only
>> recently been added, where an executable stack was always used, even when
>> it was not strictly necessary. Now I may be mis-remembering here and
>> blaming gcc incorrectly, but I that is what I had in mind when I wrote that
>> sentence.
>
> I see, thanks. My experience is that GCC versions released over the last
> couple of decades behave as expected, in other words they do *not* require an
> executable stack unless a pointer to a nested function is taken (and a quick
> test with GCC 3.4.6 confirms it in a simple case).
It very much depends on the platform. From the gcc internals manual:
{Target Hook} int TARGET_CUSTOM_FUNCTION_DESCRIPTORS
If the target can use GCC's generic descriptor mechanism for nested
functions, define this hook to a power of 2 representing an unused bit
in function pointers which can be used to differentiate descriptors at
run time. This value gives the number of bytes by which descriptor
pointers are misaligned compared to function pointers. For example, on
targets that require functions to be aligned to a 4-byte boundary, a
value of either 1 or 2 is appropriate unless the architecture already
reserves the bit for another purpose, such as on ARM.
IIRC the function descriptor method requires some spare bits in a function pointer. If all bits are reserved by the architecture you can't use that approach, unless you manufacture some bits by over-aligning every addressable function; but that's essentially an ABI break.
R.
> It very much depends on the platform. From the gcc internals manual:
>
> {Target Hook} int TARGET_CUSTOM_FUNCTION_DESCRIPTORS
> If the target can use GCC's generic descriptor mechanism for nested
> functions, define this hook to a power of 2 representing an unused bit
> in function pointers which can be used to differentiate descriptors at
> run time. This value gives the number of bytes by which descriptor
> pointers are misaligned compared to function pointers. For example, on
> targets that require functions to be aligned to a 4-byte boundary, a
> value of either 1 or 2 is appropriate unless the architecture already
> reserves the bit for another purpose, such as on ARM.
The discussion was more about requiring an executable stack in the mere
presence of nested functions though, as opposed to in the presence of pointers
to them (for which TARGET_CUSTOM_FUNCTION_DESCRIPTORS is indeed designed).
On 19/03/2025 13:58, Eric Botcazou wrote:
>> It very much depends on the platform. From the gcc internals manual:
>>
>> {Target Hook} int TARGET_CUSTOM_FUNCTION_DESCRIPTORS
>> If the target can use GCC's generic descriptor mechanism for nested
>> functions, define this hook to a power of 2 representing an unused bit
>> in function pointers which can be used to differentiate descriptors at
>> run time. This value gives the number of bytes by which descriptor
>> pointers are misaligned compared to function pointers. For example, on
>> targets that require functions to be aligned to a 4-byte boundary, a
>> value of either 1 or 2 is appropriate unless the architecture already
>> reserves the bit for another purpose, such as on ARM.
>
> The discussion was more about requiring an executable stack in the mere
> presence of nested functions though, as opposed to in the presence of pointers
> to them (for which TARGET_CUSTOM_FUNCTION_DESCRIPTORS is indeed designed).
>
I don't think we've ever needed an executable stack for nested functions if pointers to them aren't used. It's only the trampolines that those use that need it. The compiler knows when the address of a nested function is needed because it has local binding (for C at any rate).
R.
> I don't think we've ever needed an executable stack for nested functions if
> pointers to them aren't used. It's only the trampolines that those use
> that need it. The compiler knows when the address of a nested function is
> needed because it has local binding (for C at any rate).
Yes, that's what I tried to say in my reply to Nick.
@@ -704,6 +704,18 @@ typedef struct
asection* sec;
} bfd_search_result_t;
+static inline bool
+bfd_is_non_dynamic_elf_object (bfd *abfd, const struct elf_backend_data *out_be)
+{
+ const struct elf_backend_data *in_be = get_elf_backend_data (abfd);
+
+ return bfd_get_flavour (abfd) == bfd_target_elf_flavour
+ && bfd_count_sections (abfd) != 0
+ && (abfd->flags & (DYNAMIC | BFD_PLUGIN | BFD_LINKER_CREATED)) == 0
+ && out_be->elf_machine_code == in_be->elf_machine_code
+ && out_be->s->elfclass == in_be->s->elfclass;
+}
+
/* Find the first input bfd with GNU properties.
If such an input is found, set found to true and return the relevant input.
Otherwise, return the last input of bfd inputs. */
@@ -712,22 +724,13 @@ bfd_linear_search_one_with_gnu_property (struct bfd_link_info *info)
{
const struct elf_backend_data *be = get_elf_backend_data (info->output_bfd);
- bool bfd_is_non_dynamic_elf_object (bfd *abfd)
- {
- return (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
- && (bfd_count_sections (abfd) != 0)
- && (abfd->flags & (DYNAMIC | BFD_PLUGIN | BFD_LINKER_CREATED)) == 0
- && (be->elf_machine_code == get_elf_backend_data (abfd)->elf_machine_code)
- && (be->s->elfclass == get_elf_backend_data (abfd)->s->elfclass);
- }
-
bfd_search_result_t res = {
.pbfd = NULL,
.sec = NULL,
};
for (bfd *pbfd = info->input_bfds; pbfd != NULL; pbfd = pbfd->link.next)
- if (bfd_is_non_dynamic_elf_object (pbfd))
+ if (bfd_is_non_dynamic_elf_object (pbfd, be))
{
res.pbfd = pbfd;