[v2] ld: Turn on --error-execstack/--error-rwx-segments
Checks
Commit Message
Changes in v2:
1. Update the expected error in testsuite/ld-elf/pr31299-1.error to
allow different error messages.
---
Since --fatal-warnings always turns a warning to an error, turn on
--error-execstack for --warn-execstack and --error-rwx-segments for
--warn-rwx-segments if --fatal-warnings is used, overriding
--no-error-execstack and --no-error-rwx-segments.
PR ld/31299
* lexsup.c (parse_args): Turn on --error-execstack for
--warn-execstack and --error-rwx-segments for --warn-rwx-segments
if --fatal-warnings is used.
* testsuite/ld-elf/elf.exp: Run PR ld/31299 tests.
* testsuite/ld-elf/pr31299-1.error: New file.
* testsuite/ld-elf/pr31299-2.error: Likewise.
* testsuite/ld-elf/pr31299-3.error: Likewise.
---
ld/lexsup.c | 11 +++++++++++
ld/testsuite/ld-elf/elf.exp | 26 +++++++++++++++++++++++++-
ld/testsuite/ld-elf/pr31299-1.error | 2 ++
ld/testsuite/ld-elf/pr31299-2.error | 1 +
ld/testsuite/ld-elf/pr31299-3.error | 1 +
5 files changed, 40 insertions(+), 1 deletion(-)
create mode 100644 ld/testsuite/ld-elf/pr31299-1.error
create mode 100644 ld/testsuite/ld-elf/pr31299-2.error
create mode 100644 ld/testsuite/ld-elf/pr31299-3.error
Comments
On Fri, Jan 26, 2024 at 1:45 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> Changes in v2:
>
> 1. Update the expected error in testsuite/ld-elf/pr31299-1.error to
> allow different error messages.
>
> ---
> Since --fatal-warnings always turns a warning to an error, turn on
> --error-execstack for --warn-execstack and --error-rwx-segments for
> --warn-rwx-segments if --fatal-warnings is used, overriding
> --no-error-execstack and --no-error-rwx-segments.
>
> PR ld/31299
> * lexsup.c (parse_args): Turn on --error-execstack for
> --warn-execstack and --error-rwx-segments for --warn-rwx-segments
> if --fatal-warnings is used.
> * testsuite/ld-elf/elf.exp: Run PR ld/31299 tests.
> * testsuite/ld-elf/pr31299-1.error: New file.
> * testsuite/ld-elf/pr31299-2.error: Likewise.
> * testsuite/ld-elf/pr31299-3.error: Likewise.
> ---
> ld/lexsup.c | 11 +++++++++++
> ld/testsuite/ld-elf/elf.exp | 26 +++++++++++++++++++++++++-
> ld/testsuite/ld-elf/pr31299-1.error | 2 ++
> ld/testsuite/ld-elf/pr31299-2.error | 1 +
> ld/testsuite/ld-elf/pr31299-3.error | 1 +
> 5 files changed, 40 insertions(+), 1 deletion(-)
> create mode 100644 ld/testsuite/ld-elf/pr31299-1.error
> create mode 100644 ld/testsuite/ld-elf/pr31299-2.error
> create mode 100644 ld/testsuite/ld-elf/pr31299-3.error
>
> diff --git a/ld/lexsup.c b/ld/lexsup.c
> index 099dff8ecde..787d3d02e51 100644
> --- a/ld/lexsup.c
> +++ b/ld/lexsup.c
> @@ -1947,6 +1947,17 @@ parse_args (unsigned argc, char **argv)
> && command_line.check_section_addresses < 0)
> command_line.check_section_addresses = 0;
>
> + /* Override --no-error-execstack and --no-warn-execstack and turn on
> + --error-execstack for --warn-execstack and --error-rwx-segments for
> + --warn-rwx-segments if --fatal-warnings is used. */
> + if (config.fatal_warnings)
> + {
> + if (link_info.warn_execstack)
> + link_info.error_execstack = 1;
> + if (!link_info.no_warn_rwx_segments)
> + link_info.warn_is_error_for_rwx_segments = 1;
> + }
> +
> if (export_list)
> {
> struct bfd_elf_version_expr *head = export_list->head.list;
> diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
> index 685b87588e7..6ece36b7931 100644
> --- a/ld/testsuite/ld-elf/elf.exp
> +++ b/ld/testsuite/ld-elf/elf.exp
> @@ -305,7 +305,23 @@ if { [istarget *-*-*linux*]
> {nobits-1.s} \
> {} \
> "rwx-segments-3.exe"] \
> - ]
> + [list "Ensure that an error issued when creating a segment with RWX permissions" \
> + "-e 0 -Tnobits-1.t --warn-rwx-segments \
> + --fatal-warnings --no-error-rwx-segments" \
> + "" \
> + "" \
> + {nobits-1.s} \
> + {{ld pr31299-2.error}} \
> + "pr31299-2.exe"] \
> + [list "Ensure that an error issued when creating a TLS segment with execute permission" \
> + "-e 0 -T rwx-segments-2.t --warn-rwx-segments \
> + --fatal-warnings --no-error-rwx-segments" \
> + "" \
> + "" \
> + {size-2.s} \
> + {{ld pr31299-3.error}} \
> + "pr31299-3.exe"] \
> + ]
>
> set LDFLAGS $curr_ldflags
>
> @@ -318,6 +334,14 @@ if { [istarget *-*-*linux*]
> {pr29072-b.s} \
> {{ld pr29072.b.warn}} \
> "pr29072-b.exe"] \
> + [list "PR ld/31299 (error about absent .note.GNU-stack)" \
> + "-e 0 -z stack-size=0x123400 --warn-execstack \
> + --fatal-warnings --no-error-execstack" \
> + "" \
> + "" \
> + {pr29072-b.s} \
> + {{ld pr31299-1.error}} \
> + "pr31299-1.exe"] \
> ]
> } else {
> run_ld_link_tests [list \
> diff --git a/ld/testsuite/ld-elf/pr31299-1.error b/ld/testsuite/ld-elf/pr31299-1.error
> new file mode 100644
> index 00000000000..eb2598df3e6
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/pr31299-1.error
> @@ -0,0 +1,2 @@
> +.*: error: .*\.o: is triggering the generation of an executable stack because it does not have a \.note\.GNU-stack section
> +.*: failed to set dynamic section sizes: .*
> diff --git a/ld/testsuite/ld-elf/pr31299-2.error b/ld/testsuite/ld-elf/pr31299-2.error
> new file mode 100644
> index 00000000000..e5b73657069
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/pr31299-2.error
> @@ -0,0 +1 @@
> +.*: error: .* has a LOAD segment with RWX permissions
> diff --git a/ld/testsuite/ld-elf/pr31299-3.error b/ld/testsuite/ld-elf/pr31299-3.error
> new file mode 100644
> index 00000000000..b9ba368ba2c
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/pr31299-3.error
> @@ -0,0 +1 @@
> +.*: error: .* has a TLS segment with execute permission
> --
> 2.43.0
>
If there is no objection, I will check it tomorrow.
Thanks.
On 26.01.2024 22:45, H.J. Lu wrote:
> --- a/ld/lexsup.c
> +++ b/ld/lexsup.c
> @@ -1947,6 +1947,17 @@ parse_args (unsigned argc, char **argv)
> && command_line.check_section_addresses < 0)
> command_line.check_section_addresses = 0;
>
> + /* Override --no-error-execstack and --no-warn-execstack and turn on
> + --error-execstack for --warn-execstack and --error-rwx-segments for
> + --warn-rwx-segments if --fatal-warnings is used. */
> + if (config.fatal_warnings)
> + {
> + if (link_info.warn_execstack)
> + link_info.error_execstack = 1;
> + if (!link_info.no_warn_rwx_segments)
> + link_info.warn_is_error_for_rwx_segments = 1;
> + }
> +
> if (export_list)
> {
> struct bfd_elf_version_expr *head = export_list->head.list;
If I'm not mistaken and if the comment is properly describing things,
this placement of the addition means --no-* last on the command line
wouldn't be honored anymore, when later arguments ought to override
earlier ones.
Jan
On 28.01.2024 16:04, H.J. Lu wrote:
> If there is no objection, I will check it tomorrow.
I'm surprised here: Shouldn't a non-trivial (small != trivial) change
like this require active approval? At the very least I can only once
again mention that waiting for just a single day is too little, imo.
Jan
On Sun, Jan 28, 2024 at 11:33 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 28.01.2024 16:04, H.J. Lu wrote:
> > If there is no objection, I will check it tomorrow.
>
> I'm surprised here: Shouldn't a non-trivial (small != trivial) change
> like this require active approval? At the very least I can only once
> again mention that waiting for just a single day is too little, imo.
>
My patch changes a "warning":
[hjl@gnu-cfl-3 ld]$ touch x.s
[hjl@gnu-cfl-3 ld]$ gcc -c x.s
[hjl@gnu-cfl-3 ld]$ ld -shared -z stack-size=0x123400 --fatal-warnings
--warn-execstack --no-error-execstack x.o
ld: warning: x.o: missing .note.GNU-stack section implies executable stack
ld: NOTE: This behaviour is deprecated and will be removed in a future
version of the linker
[hjl@gnu-cfl-3 ld]$ echo $?
1
[hjl@gnu-cfl-3 ld]$
to an error:
[hjl@gnu-cfl-3 ld]$ ./ld-new -shared -z stack-size=0x123400
--fatal-warnings --warn-execstack --no-error-execstack x.o
./ld-new: error: x.o: is triggering the generation of an executable
stack because it does not have a .note.GNU-stack section
./ld-new: failed to set dynamic section sizes: file format not recognized
[hjl@gnu-cfl-3 ld]$
I view it as trivial since it only corrects the incorrect linker
message.
On Sun, Jan 28, 2024 at 11:30 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 26.01.2024 22:45, H.J. Lu wrote:
> > --- a/ld/lexsup.c
> > +++ b/ld/lexsup.c
> > @@ -1947,6 +1947,17 @@ parse_args (unsigned argc, char **argv)
> > && command_line.check_section_addresses < 0)
> > command_line.check_section_addresses = 0;
> >
> > + /* Override --no-error-execstack and --no-warn-execstack and turn on
> > + --error-execstack for --warn-execstack and --error-rwx-segments for
> > + --warn-rwx-segments if --fatal-warnings is used. */
> > + if (config.fatal_warnings)
> > + {
> > + if (link_info.warn_execstack)
> > + link_info.error_execstack = 1;
> > + if (!link_info.no_warn_rwx_segments)
> > + link_info.warn_is_error_for_rwx_segments = 1;
> > + }
> > +
> > if (export_list)
> > {
> > struct bfd_elf_version_expr *head = export_list->head.list;
>
> If I'm not mistaken and if the comment is properly describing things,
> this placement of the addition means --no-* last on the command line
> wouldn't be honored anymore, when later arguments ought to override
> earlier ones.
>
Before my change, we got
[hjl@gnu-cfl-3 ld]$ touch x.s
[hjl@gnu-cfl-3 ld]$ gcc -c x.s
[hjl@gnu-cfl-3 ld]$ ld -shared -z stack-size=0x123400 --fatal-warnings
--warn-execstack --no-error-execstack x.o
ld: warning: x.o: missing .note.GNU-stack section implies executable stack
ld: NOTE: This behaviour is deprecated and will be removed in a future
version of the linker
[hjl@gnu-cfl-3 ld]$ echo $?
1
The warning is misleading since --fatal-warnings IS position independent
and is always fatal for ANY warning. My patch changes it to
[hjl@gnu-cfl-3 ld]$ ./ld-new -shared -z stack-size=0x123400
--fatal-warnings --warn-execstack --no-error-execstack x.o
./ld-new: error: x.o: is triggering the generation of an executable
stack because it does not have a .note.GNU-stack section
./ld-new: failed to set dynamic section sizes: file format not recognized
[hjl@gnu-cfl-3 ld]$
Any other comments to my patch?
Thanks.
On 29.01.2024 14:03, H.J. Lu wrote:
> On Sun, Jan 28, 2024 at 11:30 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 26.01.2024 22:45, H.J. Lu wrote:
>>> --- a/ld/lexsup.c
>>> +++ b/ld/lexsup.c
>>> @@ -1947,6 +1947,17 @@ parse_args (unsigned argc, char **argv)
>>> && command_line.check_section_addresses < 0)
>>> command_line.check_section_addresses = 0;
>>>
>>> + /* Override --no-error-execstack and --no-warn-execstack and turn on
>>> + --error-execstack for --warn-execstack and --error-rwx-segments for
>>> + --warn-rwx-segments if --fatal-warnings is used. */
>>> + if (config.fatal_warnings)
>>> + {
>>> + if (link_info.warn_execstack)
>>> + link_info.error_execstack = 1;
>>> + if (!link_info.no_warn_rwx_segments)
>>> + link_info.warn_is_error_for_rwx_segments = 1;
>>> + }
>>> +
>>> if (export_list)
>>> {
>>> struct bfd_elf_version_expr *head = export_list->head.list;
>>
>> If I'm not mistaken and if the comment is properly describing things,
>> this placement of the addition means --no-* last on the command line
>> wouldn't be honored anymore, when later arguments ought to override
>> earlier ones.
>>
>
> Before my change, we got
>
> [hjl@gnu-cfl-3 ld]$ touch x.s
> [hjl@gnu-cfl-3 ld]$ gcc -c x.s
> [hjl@gnu-cfl-3 ld]$ ld -shared -z stack-size=0x123400 --fatal-warnings
> --warn-execstack --no-error-execstack x.o
> ld: warning: x.o: missing .note.GNU-stack section implies executable stack
> ld: NOTE: This behaviour is deprecated and will be removed in a future
> version of the linker
> [hjl@gnu-cfl-3 ld]$ echo $?
> 1
>
> The warning is misleading since --fatal-warnings IS position independent
> and is always fatal for ANY warning. My patch changes it to
>
> [hjl@gnu-cfl-3 ld]$ ./ld-new -shared -z stack-size=0x123400
> --fatal-warnings --warn-execstack --no-error-execstack x.o
> ./ld-new: error: x.o: is triggering the generation of an executable
> stack because it does not have a .note.GNU-stack section
> ./ld-new: failed to set dynamic section sizes: file format not recognized
> [hjl@gnu-cfl-3 ld]$
>
> Any other comments to my patch?
Well. To me, it is ambiguous what "--fatal-warnings --warn-execstack
--no-error-execstack" actually is intended to mean (and that's regardless
of --fatal-warnings position on the command line). One way of
interpreting is your way. The other is to say that --no-error-execstack
means "no error for anything related to executable stacks", i.e.
--fatal-warnings not affecting the overall result (when there are no
other warnings).
Jan
On Mon, Jan 29, 2024 at 6:03 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 29.01.2024 14:03, H.J. Lu wrote:
> > On Sun, Jan 28, 2024 at 11:30 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 26.01.2024 22:45, H.J. Lu wrote:
> >>> --- a/ld/lexsup.c
> >>> +++ b/ld/lexsup.c
> >>> @@ -1947,6 +1947,17 @@ parse_args (unsigned argc, char **argv)
> >>> && command_line.check_section_addresses < 0)
> >>> command_line.check_section_addresses = 0;
> >>>
> >>> + /* Override --no-error-execstack and --no-warn-execstack and turn on
> >>> + --error-execstack for --warn-execstack and --error-rwx-segments for
> >>> + --warn-rwx-segments if --fatal-warnings is used. */
> >>> + if (config.fatal_warnings)
> >>> + {
> >>> + if (link_info.warn_execstack)
> >>> + link_info.error_execstack = 1;
> >>> + if (!link_info.no_warn_rwx_segments)
> >>> + link_info.warn_is_error_for_rwx_segments = 1;
> >>> + }
> >>> +
> >>> if (export_list)
> >>> {
> >>> struct bfd_elf_version_expr *head = export_list->head.list;
> >>
> >> If I'm not mistaken and if the comment is properly describing things,
> >> this placement of the addition means --no-* last on the command line
> >> wouldn't be honored anymore, when later arguments ought to override
> >> earlier ones.
> >>
> >
> > Before my change, we got
> >
> > [hjl@gnu-cfl-3 ld]$ touch x.s
> > [hjl@gnu-cfl-3 ld]$ gcc -c x.s
> > [hjl@gnu-cfl-3 ld]$ ld -shared -z stack-size=0x123400 --fatal-warnings
> > --warn-execstack --no-error-execstack x.o
> > ld: warning: x.o: missing .note.GNU-stack section implies executable stack
> > ld: NOTE: This behaviour is deprecated and will be removed in a future
> > version of the linker
> > [hjl@gnu-cfl-3 ld]$ echo $?
> > 1
> >
> > The warning is misleading since --fatal-warnings IS position independent
> > and is always fatal for ANY warning. My patch changes it to
> >
> > [hjl@gnu-cfl-3 ld]$ ./ld-new -shared -z stack-size=0x123400
> > --fatal-warnings --warn-execstack --no-error-execstack x.o
> > ./ld-new: error: x.o: is triggering the generation of an executable
> > stack because it does not have a .note.GNU-stack section
> > ./ld-new: failed to set dynamic section sizes: file format not recognized
> > [hjl@gnu-cfl-3 ld]$
> >
> > Any other comments to my patch?
>
> Well. To me, it is ambiguous what "--fatal-warnings --warn-execstack
> --no-error-execstack" actually is intended to mean (and that's regardless
> of --fatal-warnings position on the command line). One way of
> interpreting is your way. The other is to say that --no-error-execstack
> means "no error for anything related to executable stacks", i.e.
> --fatal-warnings not affecting the overall result (when there are no
> other warnings).
My interpretation is what --fatal-warnings implements.
--
H.J.
@@ -1947,6 +1947,17 @@ parse_args (unsigned argc, char **argv)
&& command_line.check_section_addresses < 0)
command_line.check_section_addresses = 0;
+ /* Override --no-error-execstack and --no-warn-execstack and turn on
+ --error-execstack for --warn-execstack and --error-rwx-segments for
+ --warn-rwx-segments if --fatal-warnings is used. */
+ if (config.fatal_warnings)
+ {
+ if (link_info.warn_execstack)
+ link_info.error_execstack = 1;
+ if (!link_info.no_warn_rwx_segments)
+ link_info.warn_is_error_for_rwx_segments = 1;
+ }
+
if (export_list)
{
struct bfd_elf_version_expr *head = export_list->head.list;
@@ -305,7 +305,23 @@ if { [istarget *-*-*linux*]
{nobits-1.s} \
{} \
"rwx-segments-3.exe"] \
- ]
+ [list "Ensure that an error issued when creating a segment with RWX permissions" \
+ "-e 0 -Tnobits-1.t --warn-rwx-segments \
+ --fatal-warnings --no-error-rwx-segments" \
+ "" \
+ "" \
+ {nobits-1.s} \
+ {{ld pr31299-2.error}} \
+ "pr31299-2.exe"] \
+ [list "Ensure that an error issued when creating a TLS segment with execute permission" \
+ "-e 0 -T rwx-segments-2.t --warn-rwx-segments \
+ --fatal-warnings --no-error-rwx-segments" \
+ "" \
+ "" \
+ {size-2.s} \
+ {{ld pr31299-3.error}} \
+ "pr31299-3.exe"] \
+ ]
set LDFLAGS $curr_ldflags
@@ -318,6 +334,14 @@ if { [istarget *-*-*linux*]
{pr29072-b.s} \
{{ld pr29072.b.warn}} \
"pr29072-b.exe"] \
+ [list "PR ld/31299 (error about absent .note.GNU-stack)" \
+ "-e 0 -z stack-size=0x123400 --warn-execstack \
+ --fatal-warnings --no-error-execstack" \
+ "" \
+ "" \
+ {pr29072-b.s} \
+ {{ld pr31299-1.error}} \
+ "pr31299-1.exe"] \
]
} else {
run_ld_link_tests [list \
new file mode 100644
@@ -0,0 +1,2 @@
+.*: error: .*\.o: is triggering the generation of an executable stack because it does not have a \.note\.GNU-stack section
+.*: failed to set dynamic section sizes: .*
new file mode 100644
@@ -0,0 +1 @@
+.*: error: .* has a LOAD segment with RWX permissions
new file mode 100644
@@ -0,0 +1 @@
+.*: error: .* has a TLS segment with execute permission