RFC: Default to a start address of zero for shared libraries

Message ID 87pmt7kwby.fsf@redhat.com
State New
Headers show
Series
  • RFC: Default to a start address of zero for shared libraries
Related show

Commit Message

Alan Modra via Binutils Sept. 17, 2021, 4:20 p.m.
Hi Guys,

  Shared libraries and dynamic objects like plugins are not intended to
  be run by themselves, so it does not make sense to give them an entry
  address unless explicitly requested by their creator.  Hence I am
  proposing the attached patch which will modify the linker's heuristic
  for generating a start address such that the address of the start of
  the .text section would only be used for executables, not for shared
  objects:
The point of this is that it then makes it easier for the loader to
  detect mistaken attempts by users to run shared objects.

  Any objects or concerns ?

Cheers
  Nick

Comments

Alan Modra via Binutils Sept. 17, 2021, 10:48 p.m. | #1
On Fri, Sep 17, 2021 at 05:20:33PM +0100, Nick Clifton via Binutils wrote:
>   The point of this is that it then makes it easier for the loader to

>   detect mistaken attempts by users to run shared objects.


Have you tried a glibc build?  glibc ld.so is a shared object that is
supposed to be runnable.

$ elf/ld.so --help
Usage: elf/ld.so [OPTION]... EXECUTABLE-FILE [ARGS-FOR-PROGRAM...]
You have invoked 'ld.so', the program interpreter for dynamically-linked
[snip]

-- 
Alan Modra
Australia Development Lab, IBM
Fangrui Song Sept. 17, 2021, 11:31 p.m. | #2
On 2021-09-18, Alan Modra via Binutils wrote:
>On Fri, Sep 17, 2021 at 05:20:33PM +0100, Nick Clifton via Binutils wrote:

>>   The point of this is that it then makes it easier for the loader to

>>   detect mistaken attempts by users to run shared objects.

>

>Have you tried a glibc build?  glibc ld.so is a shared object that is

>supposed to be runnable.

>

>$ elf/ld.so --help

>Usage: elf/ld.so [OPTION]... EXECUTABLE-FILE [ARGS-FOR-PROGRAM...]

>You have invoked 'ld.so', the program interpreter for dynamically-linked

>[snip]


In glibc, elf/librtld.os is linked into ld.so.
elf/librtld.os defines _start, therefore glibc is unaffected.

For ld.lld, I'll just be more aggressive and remove the .text fallback
for all modes (including executables) :)
Fangrui Song Sept. 17, 2021, 11:45 p.m. | #3
On 2021-09-17, Fangrui Song wrote:
>On 2021-09-18, Alan Modra via Binutils wrote:

>>On Fri, Sep 17, 2021 at 05:20:33PM +0100, Nick Clifton via Binutils wrote:

>>>  The point of this is that it then makes it easier for the loader to

>>>  detect mistaken attempts by users to run shared objects.

>>

>>Have you tried a glibc build?  glibc ld.so is a shared object that is

>>supposed to be runnable.

>>

>>$ elf/ld.so --help

>>Usage: elf/ld.so [OPTION]... EXECUTABLE-FILE [ARGS-FOR-PROGRAM...]

>>You have invoked 'ld.so', the program interpreter for dynamically-linked

>>[snip]

>

>In glibc, elf/librtld.os is linked into ld.so.

>elf/librtld.os defines _start, therefore glibc is unaffected.

>

>For ld.lld, I'll just be more aggressive and remove the .text fallback

>for all modes (including executables) :)


A useful data point: gold doesn't fall back to .text
Alan Modra via Binutils Sept. 21, 2021, 12:21 p.m. | #4
Hi Guys,

   Right - I am going to go ahead and apply the patch, along with a
   few updates to some linker tests that expected the old start
   address to be set.

Cheers
   Nick

ld/ChangeLog
2021-09-21  Nick Clifton  <nickc@redhat.com>

	* ldlang.c (lang_end): When computing the entry point, only
	try the start address of the entry section when creating an
	executable.
	* ld.texi (Entry point): Update description of heuristic used to
	choose the entry point.
	testsuite/ld-alpha/tlspic.rd: Update expected entry point address.
	testsuite/ld-arm/tls-gdesc-got.d: Likewise.
	testsuite/ld-i386/tlsnopic.rd: Likewise.
	testsuite/ld-ia64/tlspic.rd: Likewise.
	testsuite/ld-sparc/gotop32.rd: Likewise.
	testsuite/ld-sparc/gotop64.rd: Likewise.
	testsuite/ld-sparc/tlssunnopic32.rd: Likewise.
	testsuite/ld-sparc/tlssunnopic64.rd: Likewise.
	testsuite/ld-sparc/tlssunpic32.rd: Likewise.
	testsuite/ld-sparc/tlssunpic64.rd: Likewise.
	testsuite/ld-tic6x/shlib-1.rd: Likewise.
	testsuite/ld-tic6x/shlib-1b.rd: Likewise.
	testsuite/ld-tic6x/shlib-1r.rd: Likewise.
	testsuite/ld-tic6x/shlib-1rb.rd: Likewise.
	testsuite/ld-tic6x/shlib-noindex.rd: Likewise.
	testsuite/ld-x86-64/pr14207.d: Likewise.
	testsuite/ld-x86-64/tlsdesc.rd: Likewise.
	testsuite/ld-x86-64/tlspic.rd: Likewise.
	testsuite/ld-x86-64/tlspic2.rd: Likewise.
diff --git a/ld/ld.texi b/ld/ld.texi
index 72b5c373ba1..db410f0bf2d 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -3910,7 +3910,9 @@ the value of a target-specific symbol, if it is defined;  For many
 targets this is @code{start}, but PE- and BeOS-based systems for example
 check a list of possible entry symbols, matching the first one found.
 @item
-the address of the first byte of the @samp{.text} section, if present;
+the address of the first byte of the code section, if present and an
+executable is being created - the code section is usually
+@samp{.text}, but can be something else; 
 @item
 The address @code{0}.
 @end itemize
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 2610be995ca..818ec44c318 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6984,7 +6984,8 @@ lang_end (void)
 	  if (!bfd_set_start_address (link_info.output_bfd, val))
 	    einfo (_("%F%P: can't set start address\n"));
 	}
-      else
+      /* BZ 2004952: Only use the start of the entry section for executables.  */
+      else if bfd_link_executable (&link_info)
 	{
 	  asection *ts;
 
@@ -7010,6 +7011,13 @@ lang_end (void)
 		       entry_symbol.name);
 	    }
 	}
+      else
+	{
+	  if (warn)
+	    einfo (_("%P: warning: cannot find entry symbol %s;"
+		     " not setting start address\n"),
+		   entry_symbol.name);
+	}
     }
 }
 
diff --git a/ld/testsuite/ld-alpha/tlspic.rd b/ld/testsuite/ld-alpha/tlspic.rd
index b79fc71463d..a19df375b7c 100644
--- a/ld/testsuite/ld-alpha/tlspic.rd
+++ b/ld/testsuite/ld-alpha/tlspic.rd
@@ -29,7 +29,7 @@ Section Headers:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x1000
+Entry point 0x[0-9a-f]+
 There are [0-9]+ program headers, starting at offset [0-9]+
 
 Program Headers:
diff --git a/ld/testsuite/ld-arm/tls-gdesc-got.d b/ld/testsuite/ld-arm/tls-gdesc-got.d
index 873d11f3328..38efaa88f5c 100644
--- a/ld/testsuite/ld-arm/tls-gdesc-got.d
+++ b/ld/testsuite/ld-arm/tls-gdesc-got.d
@@ -2,7 +2,7 @@
 .*/tls-lib2-got.so:     file format elf32-.*arm.*
 architecture: arm.*, flags 0x00000150:
 HAS_SYMS, DYNAMIC, D_PAGED
-start address 0x0+8(1e8|220)
+start address 0x[0-9a-f]+
 
 
 Disassembly of section .got:
diff --git a/ld/testsuite/ld-i386/tlsnopic.rd b/ld/testsuite/ld-i386/tlsnopic.rd
index 229ce23a9af..719e44d0e0f 100644
--- a/ld/testsuite/ld-i386/tlsnopic.rd
+++ b/ld/testsuite/ld-i386/tlsnopic.rd
@@ -26,7 +26,7 @@ Key to Flags:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x1000
+Entry point 0x[-9a-f]+
 There are [0-9]+ program headers, starting at offset [0-9]+
 
 Program Headers:
diff --git a/ld/testsuite/ld-ia64/tlspic.rd b/ld/testsuite/ld-ia64/tlspic.rd
index 02c98006ea0..4b6e76e5943 100644
--- a/ld/testsuite/ld-ia64/tlspic.rd
+++ b/ld/testsuite/ld-ia64/tlspic.rd
@@ -31,7 +31,7 @@ Key to Flags:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x1000
+Entry point 0x[0-9a-f]+
 There are [0-9]+ program headers, starting at offset [0-9]+
 
 Program Headers:
diff --git a/ld/testsuite/ld-sparc/gotop32.rd b/ld/testsuite/ld-sparc/gotop32.rd
index 0f02e9191cc..a6b7107dee6 100644
--- a/ld/testsuite/ld-sparc/gotop32.rd
+++ b/ld/testsuite/ld-sparc/gotop32.rd
@@ -23,7 +23,7 @@ Section Headers:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x1000
+Entry point 0x[0-9a-f]+
 There are [0-9]+ program headers, starting at offset [0-9]+
 
 Program Headers:
diff --git a/ld/testsuite/ld-sparc/gotop64.rd b/ld/testsuite/ld-sparc/gotop64.rd
index 249d7312a47..22b3769292b 100644
--- a/ld/testsuite/ld-sparc/gotop64.rd
+++ b/ld/testsuite/ld-sparc/gotop64.rd
@@ -23,7 +23,7 @@ Section Headers:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x1000
+Entry point 0x[0-9a-f]+
 There are [0-9]+ program headers, starting at offset [0-9]+
 
 Program Headers:
diff --git a/ld/testsuite/ld-sparc/tlssunnopic32.rd b/ld/testsuite/ld-sparc/tlssunnopic32.rd
index 01f89334587..5574868393f 100644
--- a/ld/testsuite/ld-sparc/tlssunnopic32.rd
+++ b/ld/testsuite/ld-sparc/tlssunnopic32.rd
@@ -23,7 +23,7 @@ Section Headers:
  +\[[ 0-9]+\] .shstrtab +.*
 #...
 Elf file type is DYN \(Shared object file\)
-Entry point 0x1000
+Entry point 0x[0-9a-f]+
 There are [0-9]+ program headers, starting at offset [0-9]+
 
 Program Headers:
diff --git a/ld/testsuite/ld-sparc/tlssunnopic64.rd b/ld/testsuite/ld-sparc/tlssunnopic64.rd
index 8104c67c8a1..cadc77c344f 100644
--- a/ld/testsuite/ld-sparc/tlssunnopic64.rd
+++ b/ld/testsuite/ld-sparc/tlssunnopic64.rd
@@ -23,7 +23,7 @@ Section Headers:
  +\[[ 0-9]+\] .shstrtab +.*
 #...
 Elf file type is DYN \(Shared object file\)
-Entry point 0x1000
+Entry point 0x[0-9a-f]+
 There are [0-9]+ program headers, starting at offset [0-9]+
 
 Program Headers:
diff --git a/ld/testsuite/ld-sparc/tlssunpic32.rd b/ld/testsuite/ld-sparc/tlssunpic32.rd
index d4012626a49..7ad8c99d134 100644
--- a/ld/testsuite/ld-sparc/tlssunpic32.rd
+++ b/ld/testsuite/ld-sparc/tlssunpic32.rd
@@ -27,7 +27,7 @@ Section Headers:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x1000
+Entry point 0x[0-9a-f]+
 There are [0-9]+ program headers, starting at offset [0-9]+
 
 Program Headers:
diff --git a/ld/testsuite/ld-sparc/tlssunpic64.rd b/ld/testsuite/ld-sparc/tlssunpic64.rd
index 58162057f9b..61f84c580f2 100644
--- a/ld/testsuite/ld-sparc/tlssunpic64.rd
+++ b/ld/testsuite/ld-sparc/tlssunpic64.rd
@@ -27,7 +27,7 @@ Section Headers:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x1000
+Entry point 0x[0-9a-f]+
 There are [0-9]+ program headers, starting at offset [0-9]+
 
 Program Headers:
diff --git a/ld/testsuite/ld-tic6x/shlib-1.rd b/ld/testsuite/ld-tic6x/shlib-1.rd
index 6b64d01271b..9ad2396170e 100644
--- a/ld/testsuite/ld-tic6x/shlib-1.rd
+++ b/ld/testsuite/ld-tic6x/shlib-1.rd
@@ -23,7 +23,7 @@ Key to Flags:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x10000080
+Entry point 0x[0-9a-f]+
 There are 4 program headers, starting at offset 52
 
 Program Headers:
diff --git a/ld/testsuite/ld-tic6x/shlib-1b.rd b/ld/testsuite/ld-tic6x/shlib-1b.rd
index 6b64d01271b..9ad2396170e 100644
--- a/ld/testsuite/ld-tic6x/shlib-1b.rd
+++ b/ld/testsuite/ld-tic6x/shlib-1b.rd
@@ -23,7 +23,7 @@ Key to Flags:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x10000080
+Entry point 0x[0-9a-f]+
 There are 4 program headers, starting at offset 52
 
 Program Headers:
diff --git a/ld/testsuite/ld-tic6x/shlib-1r.rd b/ld/testsuite/ld-tic6x/shlib-1r.rd
index 6b64d01271b..9ad2396170e 100644
--- a/ld/testsuite/ld-tic6x/shlib-1r.rd
+++ b/ld/testsuite/ld-tic6x/shlib-1r.rd
@@ -23,7 +23,7 @@ Key to Flags:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x10000080
+Entry point 0x[0-9a-f]+
 There are 4 program headers, starting at offset 52
 
 Program Headers:
diff --git a/ld/testsuite/ld-tic6x/shlib-1rb.rd b/ld/testsuite/ld-tic6x/shlib-1rb.rd
index 6b64d01271b..9ad2396170e 100644
--- a/ld/testsuite/ld-tic6x/shlib-1rb.rd
+++ b/ld/testsuite/ld-tic6x/shlib-1rb.rd
@@ -23,7 +23,7 @@ Key to Flags:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x10000080
+Entry point 0x[0-9a-f]+
 There are 4 program headers, starting at offset 52
 
 Program Headers:
diff --git a/ld/testsuite/ld-tic6x/shlib-noindex.rd b/ld/testsuite/ld-tic6x/shlib-noindex.rd
index 38934ba2b5f..f087d15bffd 100644
--- a/ld/testsuite/ld-tic6x/shlib-noindex.rd
+++ b/ld/testsuite/ld-tic6x/shlib-noindex.rd
@@ -24,7 +24,7 @@ Key to Flags:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x10000080
+Entry point 0x[0-9a-f]+
 There are 4 program headers, starting at offset 52
 
 Program Headers:
diff --git a/ld/testsuite/ld-x86-64/pr14207.d b/ld/testsuite/ld-x86-64/pr14207.d
index 1713888ff71..f330600b916 100644
--- a/ld/testsuite/ld-x86-64/pr14207.d
+++ b/ld/testsuite/ld-x86-64/pr14207.d
@@ -5,7 +5,7 @@
 #target: x86_64-*-linux*
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x149
+Entry point 0x[0-9a-f]+
 There are 4 program headers, starting at offset 64
 
 Program Headers:
diff --git a/ld/testsuite/ld-x86-64/tlsdesc.rd b/ld/testsuite/ld-x86-64/tlsdesc.rd
index 58feb20e55a..98bda5b2a39 100644
--- a/ld/testsuite/ld-x86-64/tlsdesc.rd
+++ b/ld/testsuite/ld-x86-64/tlsdesc.rd
@@ -29,7 +29,7 @@ Key to Flags:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x1000
+Entry point 0x[0-9a-f]+
 There are [0-9]+ program headers, starting at offset [0-9]+
 
 Program Headers:
diff --git a/ld/testsuite/ld-x86-64/tlspic.rd b/ld/testsuite/ld-x86-64/tlspic.rd
index 2e44dc6ac6b..e5d991ab323 100644
--- a/ld/testsuite/ld-x86-64/tlspic.rd
+++ b/ld/testsuite/ld-x86-64/tlspic.rd
@@ -29,7 +29,7 @@ Key to Flags:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x1000
+Entry point 0x[0-9a-f]+
 There are [0-9]+ program headers, starting at offset [0-9]+
 
 Program Headers:
diff --git a/ld/testsuite/ld-x86-64/tlspic2.rd b/ld/testsuite/ld-x86-64/tlspic2.rd
index 60decd2787b..4c20f3b4252 100644
--- a/ld/testsuite/ld-x86-64/tlspic2.rd
+++ b/ld/testsuite/ld-x86-64/tlspic2.rd
@@ -29,7 +29,7 @@ Key to Flags:
 #...
 
 Elf file type is DYN \(Shared object file\)
-Entry point 0x1000
+Entry point 0x[0-9a-f]+
 There are [0-9]+ program headers, starting at offset [0-9]+
 
 Program Headers:
Alan Modra via Binutils Sept. 22, 2021, 12:57 a.m. | #5
On Tue, Sep 21, 2021 at 01:21:24PM +0100, Nick Clifton wrote:
> --- a/ld/testsuite/ld-i386/tlsnopic.rd

> +++ b/ld/testsuite/ld-i386/tlsnopic.rd

> @@ -26,7 +26,7 @@ Key to Flags:

>  #...

>  

>  Elf file type is DYN \(Shared object file\)

> -Entry point 0x1000

> +Entry point 0x[-9a-f]+

>  There are [0-9]+ program headers, starting at offset [0-9]+

>  

>  Program Headers:


Typo.  I've commited a patch to fix it.

-- 
Alan Modra
Australia Development Lab, IBM
postmaster@partis.co.uk Sept. 22, 2021, 6:13 a.m. | #6
Delivery is delayed to these recipients or groups:

gary@partis.co.uk<mailto:gary@partis.co.uk>

Subject: Re: RFC: Default to a start address of zero for shared libraries

This message hasn't been delivered yet. Delivery will continue to be attempted.

The server will keep trying to deliver this message for the next 19 days, 19 hours and 57 minutes. You'll be notified if the message can't be delivered by that time.
Alan Modra via Binutils Sept. 22, 2021, 7:27 p.m. | #7
* Nick Clifton via Binutils:

> diff --git a/ld/ldlang.c b/ld/ldlang.c

> index 2610be995ca..818ec44c318 100644

> --- a/ld/ldlang.c

> +++ b/ld/ldlang.c

> @@ -6984,7 +6984,8 @@ lang_end (void)

>  	  if (!bfd_set_start_address (link_info.output_bfd, val))

>  	    einfo (_("%F%P: can't set start address\n"));

>  	}

> -      else

> +      /* BZ 2004952: Only use the start of the entry section for executables.  */

> +      else if bfd_link_executable (&link_info)


Not sure if the downstream bug reference is appropriate here.

> @@ -7010,6 +7011,13 @@ lang_end (void)

>  		       entry_symbol.name);

>  	    }

>  	}

> +      else

> +	{

> +	  if (warn)

> +	    einfo (_("%P: warning: cannot find entry symbol %s;"

> +		     " not setting start address\n"),

> +		   entry_symbol.name);

> +	}


In which cases is this warning printed?

Thanks,
Florian

(sorry for any duplicates)
Fangrui Song Sept. 22, 2021, 8:15 p.m. | #8
On 2021-09-22, Florian Weimer wrote:
>* Nick Clifton via Binutils:

>

>> diff --git a/ld/ldlang.c b/ld/ldlang.c

>> index 2610be995ca..818ec44c318 100644

>> --- a/ld/ldlang.c

>> +++ b/ld/ldlang.c

>> @@ -6984,7 +6984,8 @@ lang_end (void)

>>  	  if (!bfd_set_start_address (link_info.output_bfd, val))

>>  	    einfo (_("%F%P: can't set start address\n"));

>>  	}

>> -      else

>> +      /* BZ 2004952: Only use the start of the entry section for executables.  */

>> +      else if bfd_link_executable (&link_info)

>

>Not sure if the downstream bug reference is appropriate here.

>

>> @@ -7010,6 +7011,13 @@ lang_end (void)

>>  		       entry_symbol.name);

>>  	    }

>>  	}

>> +      else

>> +	{

>> +	  if (warn)

>> +	    einfo (_("%P: warning: cannot find entry symbol %s;"

>> +		     " not setting start address\n"),

>> +		   entry_symbol.name);

>> +	}

>

>In which cases is this warning printed?

>

>Thanks,

>Florian

>

>(sorry for any duplicates)

>


gold does not have the '.text' fallback for both executable and shared objects.

FWIW I changed LLD 14.0.0 to not have the ".text" fallback.

It'd be good to see GNU ld drops the fallback for executable as well,
but the current state (AIUI just shared objects) isn't bad.

Patch

diff --git a/ld/ld.texi b/ld/ld.texi
index 72b5c373ba1..db410f0bf2d 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -3910,7 +3910,9 @@  the value of a target-specific symbol, if it is defined;  For many
 targets this is @code{start}, but PE- and BeOS-based systems for example
 check a list of possible entry symbols, matching the first one found.
 @item
-the address of the first byte of the @samp{.text} section, if present;
+the address of the first byte of the code section, if present and an
+executable is being created - the code section is usually
+@samp{.text}, but can be something else; 
 @item
 The address @code{0}.
 @end itemize
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 2610be995ca..818ec44c318 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6984,7 +6984,8 @@  lang_end (void)
 	  if (!bfd_set_start_address (link_info.output_bfd, val))
 	    einfo (_("%F%P: can't set start address\n"));
 	}
-      else
+      /* BZ 2004952: Only use the start of the .text section for executables.  */
+      else if bfd_link_executable (&link_info)
 	{
 	  asection *ts;
 
@@ -7010,6 +7011,13 @@  lang_end (void)
 		       entry_symbol.name);
 	    }
 	}
+      else
+	{
+	  if (warn)
+	    einfo (_("%P: warning: cannot find entry symbol %s;"
+		     " not setting start address\n"),
+		   entry_symbol.name);
+	}
     }
 }