diff mbox series

[07/28] elf: Implement ld.so --help

Message ID 05bb7e01b9a1cb063ec4619285a08bcb25fd6d97.1601569371.git.fweimer@redhat.com
State Committed
Headers show
Series glibc-hwcaps support | expand

Commit Message

Florian Weimer Oct. 1, 2020, 4:32 p.m. UTC
--help processing is deferred to the point where the executable has
been loaded, so that it is possible to eventually include information
from the main executable in the help output.

As suggested in the GNU command-line interface guidelines, the help
message is printed to standard output, and the exit status is
successful.

Handle usage errors closer to the GNU command-line interface
guidelines.
---
 elf/dl-main.h  |  9 +++++++--
 elf/dl-usage.c | 24 ++++++++++++++++++----
 elf/rtld.c     | 55 ++++++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 74 insertions(+), 14 deletions(-)

Comments

Adhemerval Zanella Oct. 7, 2020, 5:16 p.m. UTC | #1
On 01/10/2020 13:32, Florian Weimer via Libc-alpha wrote:
> --help processing is deferred to the point where the executable has
> been loaded, so that it is possible to eventually include information
> from the main executable in the help output.
> 
> As suggested in the GNU command-line interface guidelines, the help
> message is printed to standard output, and the exit status is
> successful.
> 
> Handle usage errors closer to the GNU command-line interface
> guidelines.

LGTM with some nits below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  elf/dl-main.h  |  9 +++++++--
>  elf/dl-usage.c | 24 ++++++++++++++++++----
>  elf/rtld.c     | 55 ++++++++++++++++++++++++++++++++++++++++++--------
>  3 files changed, 74 insertions(+), 14 deletions(-)
> 
> diff --git a/elf/dl-main.h b/elf/dl-main.h
> index 68dd27d0d7..71ca5114de 100644
> --- a/elf/dl-main.h
> +++ b/elf/dl-main.h
> @@ -60,7 +60,7 @@ struct audit_list
>  };
>  
>  /* This is a list of all the modes the dynamic loader can be in.  */
> -enum mode { normal, list, verify, trace };
> +enum mode { normal, list, verify, trace, rtld_help };
>  
>  /* Aggregated state information extracted from environment variables
>     and the ld.so command line.  */

Ok.

> @@ -98,6 +98,11 @@ call_init_paths (const struct dl_main_state *state)
>  }
>  
>  /* Print ld.so usage information and exit.  */
> -void _dl_usage (void) attribute_hidden __attribute__ ((__noreturn__));
> +void _dl_usage (const char *argv0, const char *wrong_option)
> +  attribute_hidden __attribute__ ((__noreturn__));
> +
> +/* Print ld.so --help output and exit.  */
> +void _dl_help (const char *argv0, struct dl_main_state *state)
> +  attribute_hidden __attribute__ ((__noreturn__));
>  
>  #endif /* _DL_MAIN */

Maybe use _Noreturn here?

> diff --git a/elf/dl-usage.c b/elf/dl-usage.c
> index f3d89d22b7..72ff99e70f 100644
> --- a/elf/dl-usage.c
> +++ b/elf/dl-usage.c
> @@ -19,12 +19,24 @@
>  #include <dl-cache.h>
>  #include <dl-main.h>
>  #include <ldsodefs.h>
> +#include <unistd.h>
>  
>  void
> -_dl_usage (void)
> +_dl_usage (const char *argv0, const char *wrong_option)
>  {
> -  _dl_fatal_printf ("\
> -Usage: ld.so [OPTION]... EXECUTABLE-FILE [ARGS-FOR-PROGRAM...]\n\
> +  if (wrong_option != NULL)
> +    _dl_error_printf ("%s: unrecognized option '%s'\n", argv0, wrong_option);
> +  else
> +    _dl_error_printf ("%s: missing program name\n", argv0);
> +  _dl_error_printf ("Try '%s --help' for more information.\n", argv0);
> +  _exit (1);
> +}
> +

Maybe EXIT_FAILURE here?

> +void
> +_dl_help (const char *argv0, struct dl_main_state *state)
> +{
> +  _dl_printf ("\
> +Usage: %s [OPTION]... EXECUTABLE-FILE [ARGS-FOR-PROGRAM...]\n\
>  You have invoked `ld.so', the helper program for shared library executables.\n\
>  This program usually lives in the file `/lib/ld.so', and special directives\n\
>  in executable files using ELF shared libraries tell the system's program\n\
> @@ -47,5 +59,9 @@ of this helper program; chances are you did not intend to run this program.\n\
>                          in LIST\n\
>    --audit LIST          use objects named in LIST as auditors\n\
>    --preload LIST        preload objects named in LIST\n\
> -  --argv0 STRING        set argv[0] to STRING before running\n");
> +  --argv0 STRING        set argv[0] to STRING before running\n\
> +  --help                display this help and exit\n\
> +",
> +              argv0);
> +  _exit (0);
>  }

Maybe EXIT_SUCCESS here?

> diff --git a/elf/rtld.c b/elf/rtld.c
> index d11fe22b83..5cdab3c99c 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1151,6 +1151,7 @@ dl_main (const ElfW(Phdr) *phdr,
>    _dl_starting_up = 1;
>  #endif
>  
> +  const char *ld_so_name = _dl_argv[0];
>    if (*user_entry == (ElfW(Addr)) ENTRY_POINT)
>      {
>        /* Ho ho.  We are not the program interpreter!  We are the program
> @@ -1178,8 +1179,12 @@ dl_main (const ElfW(Phdr) *phdr,
>        while (_dl_argc > 1)
>  	if (! strcmp (_dl_argv[1], "--list"))
>  	  {
> -	    state.mode = list;
> -	    GLRO(dl_lazy) = -1;	/* This means do no dependency analysis.  */
> +	    if (state.mode != rtld_help)
> +	      {
> +	       state.mode = list;
> +		/* This means do no dependency analysis.  */
> +		GLRO(dl_lazy) = -1;
> +	      }
>  
>  	    ++_dl_skip_args;
>  	    --_dl_argc;

Ok.

> @@ -1187,7 +1192,8 @@ dl_main (const ElfW(Phdr) *phdr,
>  	  }
>  	else if (! strcmp (_dl_argv[1], "--verify"))
>  	  {
> -	    state.mode = verify;
> +	    if (state.mode != rtld_help)
> +	      state.mode = verify;
>  
>  	    ++_dl_skip_args;
>  	    --_dl_argc;

Ok.

> @@ -1242,13 +1248,34 @@ dl_main (const ElfW(Phdr) *phdr,
>  	    _dl_argc -= 2;
>  	    _dl_argv += 2;
>  	  }
> +	else if (strcmp (_dl_argv[1], "--help") == 0)
> +	  {
> +	    state.mode = rtld_help;
> +	    --_dl_argc;
> +	    ++_dl_argv;
> +	  }
> +	else if (_dl_argv[1][0] == '-' && _dl_argv[1][1] == '-')
> +	  {
> +	   if (_dl_argv[1][1] == '\0')
> +	     /* End of option list.  */
> +	     break;
> +	   else
> +	     /* Unrecognized option.  */
> +	     _dl_usage (ld_so_name, _dl_argv[1]);
> +	  }
>  	else
>  	  break;
>  
>        /* If we have no further argument the program was called incorrectly.
>  	 Grant the user some education.  */
>        if (_dl_argc < 2)
> -	_dl_usage ();
> +	{
> +	  if (state.mode == rtld_help)
> +	    /* --help without an executable is not an error.  */
> +	    _dl_help (ld_so_name, &state);
> +	  else
> +	    _dl_usage (ld_so_name, NULL);
> +	}
>  

Why open brackets here? There is no stack variable to possible escape.

>        ++_dl_skip_args;
>        --_dl_argc;
> @@ -1273,7 +1300,7 @@ dl_main (const ElfW(Phdr) *phdr,
>  	    break;
>  	  }
>  
> -      if (__builtin_expect (state.mode, normal) == verify)
> +      if (state.mode == verify || state.mode == rtld_help)
>  	{
>  	  const char *objname;
>  	  const char *err_str = NULL;

Ok.

> @@ -1286,9 +1313,16 @@ dl_main (const ElfW(Phdr) *phdr,
>  	  (void) _dl_catch_error (&objname, &err_str, &malloced, map_doit,
>  				  &args);
>  	  if (__glibc_unlikely (err_str != NULL))
> -	    /* We don't free the returned string, the programs stops
> -	       anyway.  */
> -	    _exit (EXIT_FAILURE);
> +	    {
> +	      /* We don't free the returned string, the programs stops
> +		 anyway.  */
> +	      if (state.mode == rtld_help)
> +		/* Mask the failure to load the main object.  The help
> +		   message contains less information in this case.  */
> +		_dl_help (ld_so_name, &state);
> +	      else
> +		_exit (EXIT_FAILURE);
> +	    }
>  	}
>        else
>  	{

Ok.

> @@ -1647,6 +1681,11 @@ dl_main (const ElfW(Phdr) *phdr,
>    audit_list_add_dynamic_tag (&state.audit_list, main_map, DT_AUDIT);
>    audit_list_add_dynamic_tag (&state.audit_list, main_map, DT_DEPAUDIT);
>  
> +  /* At this point, all data has been obtained that is included in the
> +     --help output.  */
> +  if (__builtin_expect (state.mode, normal) == rtld_help)
> +    _dl_help (ld_so_name, &state);
> +
>    /* If we have auditing DSOs to load, do it now.  */
>    bool need_security_init = true;
>    if (state.audit_list.length > 0)
> 

Use __glibc_{un}likely or just remove the branch hint (I really don't think
this matter here anyway).
Florian Weimer Oct. 8, 2020, 1:13 p.m. UTC | #2
* Adhemerval Zanella via Libc-alpha:

>> @@ -98,6 +98,11 @@ call_init_paths (const struct dl_main_state *state)
>>  }
>>  
>>  /* Print ld.so usage information and exit.  */
>> -void _dl_usage (void) attribute_hidden __attribute__ ((__noreturn__));
>> +void _dl_usage (const char *argv0, const char *wrong_option)
>> +  attribute_hidden __attribute__ ((__noreturn__));
>> +
>> +/* Print ld.so --help output and exit.  */
>> +void _dl_help (const char *argv0, struct dl_main_state *state)
>> +  attribute_hidden __attribute__ ((__noreturn__));
>>  
>>  #endif /* _DL_MAIN */
>
> Maybe use _Noreturn here?

Fixed.

>> diff --git a/elf/dl-usage.c b/elf/dl-usage.c
>> index f3d89d22b7..72ff99e70f 100644
>> --- a/elf/dl-usage.c
>> +++ b/elf/dl-usage.c
>> @@ -19,12 +19,24 @@
>>  #include <dl-cache.h>
>>  #include <dl-main.h>
>>  #include <ldsodefs.h>
>> +#include <unistd.h>
>>  
>>  void
>> -_dl_usage (void)
>> +_dl_usage (const char *argv0, const char *wrong_option)
>>  {
>> -  _dl_fatal_printf ("\
>> -Usage: ld.so [OPTION]... EXECUTABLE-FILE [ARGS-FOR-PROGRAM...]\n\
>> +  if (wrong_option != NULL)
>> +    _dl_error_printf ("%s: unrecognized option '%s'\n", argv0, wrong_option);
>> +  else
>> +    _dl_error_printf ("%s: missing program name\n", argv0);
>> +  _dl_error_printf ("Try '%s --help' for more information.\n", argv0);
>> +  _exit (1);
>> +}
>> +
>
> Maybe EXIT_FAILURE here?

Fixed.

>> +void
>> +_dl_help (const char *argv0, struct dl_main_state *state)
>> +{
>> +  _dl_printf ("\
>> +Usage: %s [OPTION]... EXECUTABLE-FILE [ARGS-FOR-PROGRAM...]\n\
>>  You have invoked `ld.so', the helper program for shared library executables.\n\
>>  This program usually lives in the file `/lib/ld.so', and special directives\n\
>>  in executable files using ELF shared libraries tell the system's program\n\
>> @@ -47,5 +59,9 @@ of this helper program; chances are you did not intend to run this program.\n\
>>                          in LIST\n\
>>    --audit LIST          use objects named in LIST as auditors\n\
>>    --preload LIST        preload objects named in LIST\n\
>> -  --argv0 STRING        set argv[0] to STRING before running\n");
>> +  --argv0 STRING        set argv[0] to STRING before running\n\
>> +  --help                display this help and exit\n\
>> +",
>> +              argv0);
>> +  _exit (0);
>>  }
>
> Maybe EXIT_SUCCESS here?

Fixed.

>>        /* If we have no further argument the program was called incorrectly.
>>  	 Grant the user some education.  */
>>        if (_dl_argc < 2)
>> -	_dl_usage ();
>> +	{
>> +	  if (state.mode == rtld_help)
>> +	    /* --help without an executable is not an error.  */
>> +	    _dl_help (ld_so_name, &state);
>> +	  else
>> +	    _dl_usage (ld_so_name, NULL);
>> +	}
>>  
>
> Why open brackets here? There is no stack variable to possible escape.

They are guarded by an if statement:

      /* If we have no further argument the program was called incorrectly.
	 Grant the user some education.  */
      if (_dl_argc < 2)
	{
	  if (state.mode == rtld_mode_help)
	    /* --help without an executable is not an error.  */
	    _dl_help (ld_so_name, &state);
	  else
	    _dl_usage (ld_so_name, NULL);
	}


>> @@ -1647,6 +1681,11 @@ dl_main (const ElfW(Phdr) *phdr,
>>    audit_list_add_dynamic_tag (&state.audit_list, main_map, DT_AUDIT);
>>    audit_list_add_dynamic_tag (&state.audit_list, main_map, DT_DEPAUDIT);
>>  
>> +  /* At this point, all data has been obtained that is included in the
>> +     --help output.  */
>> +  if (__builtin_expect (state.mode, normal) == rtld_help)
>> +    _dl_help (ld_so_name, &state);
>> +
>>    /* If we have auditing DSOs to load, do it now.  */
>>    bool need_security_init = true;
>>    if (state.audit_list.length > 0)
>> 
>
> Use __glibc_{un}likely or just remove the branch hint (I really don't think
> this matter here anyway).

Fixed.

I've pushed this with the indicated changes (and the adjustments for
renaming the enum constant to rtld_mode_help).

Thanks,
Florian
diff mbox series

Patch

diff --git a/elf/dl-main.h b/elf/dl-main.h
index 68dd27d0d7..71ca5114de 100644
--- a/elf/dl-main.h
+++ b/elf/dl-main.h
@@ -60,7 +60,7 @@  struct audit_list
 };
 
 /* This is a list of all the modes the dynamic loader can be in.  */
-enum mode { normal, list, verify, trace };
+enum mode { normal, list, verify, trace, rtld_help };
 
 /* Aggregated state information extracted from environment variables
    and the ld.so command line.  */
@@ -98,6 +98,11 @@  call_init_paths (const struct dl_main_state *state)
 }
 
 /* Print ld.so usage information and exit.  */
-void _dl_usage (void) attribute_hidden __attribute__ ((__noreturn__));
+void _dl_usage (const char *argv0, const char *wrong_option)
+  attribute_hidden __attribute__ ((__noreturn__));
+
+/* Print ld.so --help output and exit.  */
+void _dl_help (const char *argv0, struct dl_main_state *state)
+  attribute_hidden __attribute__ ((__noreturn__));
 
 #endif /* _DL_MAIN */
diff --git a/elf/dl-usage.c b/elf/dl-usage.c
index f3d89d22b7..72ff99e70f 100644
--- a/elf/dl-usage.c
+++ b/elf/dl-usage.c
@@ -19,12 +19,24 @@ 
 #include <dl-cache.h>
 #include <dl-main.h>
 #include <ldsodefs.h>
+#include <unistd.h>
 
 void
-_dl_usage (void)
+_dl_usage (const char *argv0, const char *wrong_option)
 {
-  _dl_fatal_printf ("\
-Usage: ld.so [OPTION]... EXECUTABLE-FILE [ARGS-FOR-PROGRAM...]\n\
+  if (wrong_option != NULL)
+    _dl_error_printf ("%s: unrecognized option '%s'\n", argv0, wrong_option);
+  else
+    _dl_error_printf ("%s: missing program name\n", argv0);
+  _dl_error_printf ("Try '%s --help' for more information.\n", argv0);
+  _exit (1);
+}
+
+void
+_dl_help (const char *argv0, struct dl_main_state *state)
+{
+  _dl_printf ("\
+Usage: %s [OPTION]... EXECUTABLE-FILE [ARGS-FOR-PROGRAM...]\n\
 You have invoked `ld.so', the helper program for shared library executables.\n\
 This program usually lives in the file `/lib/ld.so', and special directives\n\
 in executable files using ELF shared libraries tell the system's program\n\
@@ -47,5 +59,9 @@  of this helper program; chances are you did not intend to run this program.\n\
                         in LIST\n\
   --audit LIST          use objects named in LIST as auditors\n\
   --preload LIST        preload objects named in LIST\n\
-  --argv0 STRING        set argv[0] to STRING before running\n");
+  --argv0 STRING        set argv[0] to STRING before running\n\
+  --help                display this help and exit\n\
+",
+              argv0);
+  _exit (0);
 }
diff --git a/elf/rtld.c b/elf/rtld.c
index d11fe22b83..5cdab3c99c 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1151,6 +1151,7 @@  dl_main (const ElfW(Phdr) *phdr,
   _dl_starting_up = 1;
 #endif
 
+  const char *ld_so_name = _dl_argv[0];
   if (*user_entry == (ElfW(Addr)) ENTRY_POINT)
     {
       /* Ho ho.  We are not the program interpreter!  We are the program
@@ -1178,8 +1179,12 @@  dl_main (const ElfW(Phdr) *phdr,
       while (_dl_argc > 1)
 	if (! strcmp (_dl_argv[1], "--list"))
 	  {
-	    state.mode = list;
-	    GLRO(dl_lazy) = -1;	/* This means do no dependency analysis.  */
+	    if (state.mode != rtld_help)
+	      {
+	       state.mode = list;
+		/* This means do no dependency analysis.  */
+		GLRO(dl_lazy) = -1;
+	      }
 
 	    ++_dl_skip_args;
 	    --_dl_argc;
@@ -1187,7 +1192,8 @@  dl_main (const ElfW(Phdr) *phdr,
 	  }
 	else if (! strcmp (_dl_argv[1], "--verify"))
 	  {
-	    state.mode = verify;
+	    if (state.mode != rtld_help)
+	      state.mode = verify;
 
 	    ++_dl_skip_args;
 	    --_dl_argc;
@@ -1242,13 +1248,34 @@  dl_main (const ElfW(Phdr) *phdr,
 	    _dl_argc -= 2;
 	    _dl_argv += 2;
 	  }
+	else if (strcmp (_dl_argv[1], "--help") == 0)
+	  {
+	    state.mode = rtld_help;
+	    --_dl_argc;
+	    ++_dl_argv;
+	  }
+	else if (_dl_argv[1][0] == '-' && _dl_argv[1][1] == '-')
+	  {
+	   if (_dl_argv[1][1] == '\0')
+	     /* End of option list.  */
+	     break;
+	   else
+	     /* Unrecognized option.  */
+	     _dl_usage (ld_so_name, _dl_argv[1]);
+	  }
 	else
 	  break;
 
       /* If we have no further argument the program was called incorrectly.
 	 Grant the user some education.  */
       if (_dl_argc < 2)
-	_dl_usage ();
+	{
+	  if (state.mode == rtld_help)
+	    /* --help without an executable is not an error.  */
+	    _dl_help (ld_so_name, &state);
+	  else
+	    _dl_usage (ld_so_name, NULL);
+	}
 
       ++_dl_skip_args;
       --_dl_argc;
@@ -1273,7 +1300,7 @@  dl_main (const ElfW(Phdr) *phdr,
 	    break;
 	  }
 
-      if (__builtin_expect (state.mode, normal) == verify)
+      if (state.mode == verify || state.mode == rtld_help)
 	{
 	  const char *objname;
 	  const char *err_str = NULL;
@@ -1286,9 +1313,16 @@  dl_main (const ElfW(Phdr) *phdr,
 	  (void) _dl_catch_error (&objname, &err_str, &malloced, map_doit,
 				  &args);
 	  if (__glibc_unlikely (err_str != NULL))
-	    /* We don't free the returned string, the programs stops
-	       anyway.  */
-	    _exit (EXIT_FAILURE);
+	    {
+	      /* We don't free the returned string, the programs stops
+		 anyway.  */
+	      if (state.mode == rtld_help)
+		/* Mask the failure to load the main object.  The help
+		   message contains less information in this case.  */
+		_dl_help (ld_so_name, &state);
+	      else
+		_exit (EXIT_FAILURE);
+	    }
 	}
       else
 	{
@@ -1647,6 +1681,11 @@  dl_main (const ElfW(Phdr) *phdr,
   audit_list_add_dynamic_tag (&state.audit_list, main_map, DT_AUDIT);
   audit_list_add_dynamic_tag (&state.audit_list, main_map, DT_DEPAUDIT);
 
+  /* At this point, all data has been obtained that is included in the
+     --help output.  */
+  if (__builtin_expect (state.mode, normal) == rtld_help)
+    _dl_help (ld_so_name, &state);
+
   /* If we have auditing DSOs to load, do it now.  */
   bool need_security_init = true;
   if (state.audit_list.length > 0)