Re: [patch 1/2] Provide $ddir substitution for --with-auto-load-safe-path

[Eli Zaretskii <eliz@gnu.org>]

> Date: Wed, 9 May 2012 17:46:40 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> Doug has suggested using for auto-load safe-path $ddir representing
> $prefix/share/gdb as current $prefix is still insecure due to $prefix/tmp
> (such as /usr/tmp).
> 	Re: [patch] auto-load safe-path reset back by set ""
> 	http://sourceware.org/ml/gdb-patches/2012-04/msg00724.html
> 
> I had some concerns about relocatability of auto-load safe-path.  Currently it
> is not relocatable but that seems to be a bug.  With $ddir it should no longer
> be needed as data-directory itself is relocated.
> 
> I am sorry I have made a bit "kitchen sink" from it, such as some rename and
> new docs.  I find it all just to be fix ups of the already checked-in
> auto-load patchset.
> 
> No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu.
> 
> 
> Thanks,
> Jan
> 
> 
> gdb/
> 2012-05-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Provide $ddir substitution for --with-auto-load-safe-path.
> 	* NEWS (--with-auto-load-safe-path, --without-auto-load-safe-path): New
> 	entries.
> 	* auto-load.c: Include observer.h.
> 	(auto_load_safe_path_vec_update): Call substitute_path_component for
> 	each component.  New variable ddir_subst.
> 	(auto_load_gdb_datadir_changed): New function.
> 	(set_auto_load_safe_path): Rename DEFAULT_AUTO_LOAD_SAFE_PATH to
> 	AUTO_LOAD_SAFE_PATH.  New comment.
> 	(_initialize_auto_load): Rename DEFAULT_AUTO_LOAD_SAFE_PATH to
> 	AUTO_LOAD_SAFE_PATH.  Install auto_load_gdb_datadir_changed.
> 	* config.in: Regenerate.
> 	* configure: Regenerate.
> 	* configure.ac (--auto-load-safe-path): Rename
> 	DEFAULT_AUTO_LOAD_SAFE_PATH to AUTO_LOAD_SAFE_PATH.  Default to
> 	GDB_DATADIR/auto-load.
> 	* defs.h (substitute_path_component): New declaration.
> 	* top.c: Include observer.h.
> 	(set_gdb_datadir): New function.
> 	(init_main): Install it for "set data-directory".
> 	* utils.c (substitute_path_component): New function.
> 
> gdb/doc/
> 2012-05-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Provide $ddir substitution for --with-auto-load-safe-path.
> 	* gdb.texinfo (Auto-loading): Replace /usr/local by $ddir/auto-load.
> 	(Auto-loading safe path): Likewise.  Mention the default value,
> 	$ddir substitution, --with-auto-load-safe-path and
> 	--without-auto-load-safe-path.
> 	* observer.texi (gdb_datadir_changed): New.
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -181,6 +181,17 @@ set debug auto-load on|off
>  show debug auto-load
>    Control display of debugging info for auto-loading the files above.
>  
> +* New configure options
> +
> +--with-auto-load-safe-path
> +  Configure default value for the 'set auto-load safe-path' setting
> +  above.  It defaults to '$ddir/auto-load', $ddir representing the value
> +  of configure option --with-gdb-datadir.
> +
> +--without-auto-load-safe-path
> +  Set 'set auto-load safe-path' to '/', effectively disabling this
> +  security feature.
> +
>  * New remote packets
>  
>  z0/z1 conditional breakpoints extension
> --- a/gdb/auto-load.c
> +++ b/gdb/auto-load.c
> @@ -35,6 +35,7 @@
>  #include "gdb_vecs.h"
>  #include "readline/tilde.h"
>  #include "completer.h"
> +#include "observer.h"
>  
>  /* The suffix of per-objfile scripts to auto-load as non-Python command files.
>     E.g. When the program loads libfoo.so, look for libfoo-gdb.gdb.  */
> @@ -141,10 +142,16 @@ auto_load_safe_path_vec_update (void)
>    for (ix = 0; ix < len; ix++)
>      {
>        char *dir = VEC_index (char_ptr, auto_load_safe_path_vec, ix);
> -      char *expanded = tilde_expand (dir);
> -      char *real_path = gdb_realpath (expanded);
> +      char *ddir_subst, *expanded, *real_path;
> +
> +      ddir_subst = xstrdup (dir);
> +      substitute_path_component (&ddir_subst, "$ddir", gdb_datadir);
> +      expanded = tilde_expand (ddir_subst);
> +      xfree (ddir_subst);
> +      real_path = gdb_realpath (expanded);
>  
> -      /* Ensure the current entry is at least tilde_expand-ed.  */
> +      /* Ensure the current entry is at least a valid path (therefore
> +	 $ddir-expanded and tilde-expanded).  */
>        VEC_replace (char_ptr, auto_load_safe_path_vec, ix, expanded);
>  
>        if (debug_auto_load)
> @@ -176,15 +183,24 @@ auto_load_safe_path_vec_update (void)
>      }
>  }
>  
> +/* Variable gdb_datadir has been set.  Update content depending on $ddir.  */
> +
> +static void
> +auto_load_gdb_datadir_changed (void)
> +{
> +  auto_load_safe_path_vec_update ();
> +}
> +
>  /* "set" command for the auto_load_safe_path configuration variable.  */
>  
>  static void
>  set_auto_load_safe_path (char *args, int from_tty, struct cmd_list_element *c)
>  {
> +  /* Setting the variable to "" resets it to the compile time defaults.  */
>    if (auto_load_safe_path[0] == '\0')
>      {
>        xfree (auto_load_safe_path);
> -      auto_load_safe_path = xstrdup (DEFAULT_AUTO_LOAD_SAFE_PATH);
> +      auto_load_safe_path = xstrdup (AUTO_LOAD_SAFE_PATH);
>      }
>  
>    auto_load_safe_path_vec_update ();
> @@ -1040,7 +1056,7 @@ This options has security implications for untrusted inferiors."),
>  Usage: info auto-load local-gdbinit"),
>  	   auto_load_info_cmdlist_get ());
>  
> -  auto_load_safe_path = xstrdup (DEFAULT_AUTO_LOAD_SAFE_PATH);
> +  auto_load_safe_path = xstrdup (AUTO_LOAD_SAFE_PATH);
>    auto_load_safe_path_vec_update ();
>    add_setshow_optional_filename_cmd ("safe-path", class_support,
>  				     &auto_load_safe_path, _("\
> @@ -1058,6 +1074,7 @@ This options has security implications for untrusted inferiors."),
>  				     show_auto_load_safe_path,
>  				     auto_load_set_cmdlist_get (),
>  				     auto_load_show_cmdlist_get ());
> +  observer_attach_gdb_datadir_changed (auto_load_gdb_datadir_changed);
>  
>    cmd = add_cmd ("add-auto-load-safe-path", class_support,
>  		 add_auto_load_safe_path,
> --- a/gdb/config.in
> +++ b/gdb/config.in
> @@ -3,6 +3,9 @@
>  /* Define if building universal (internal helper macro) */
>  #undef AC_APPLE_UNIVERSAL_BUILD
>  
> +/* Directories safe to hold auto-loaded files. */
> +#undef AUTO_LOAD_SAFE_PATH
> +
>  /* Directory of programs. */
>  #undef BINDIR
>  
> @@ -24,9 +27,6 @@
>     moved. */
>  #undef DEBUGDIR_RELOCATABLE
>  
> -/* Directories safe to hold auto-loaded files. */
> -#undef DEFAULT_AUTO_LOAD_SAFE_PATH
> -
>  /* Define to BFD's default architecture. */
>  #undef DEFAULT_BFD_ARCH
>  
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -1486,7 +1486,8 @@ Optional Packages:
>    --with-relocated-sources=PATH
>                            automatically relocate this path for source files
>    --with-auto-load-safe-path=PATH
> -                          directories safe to hold auto-loaded files
> +                          directories safe to hold auto-loaded files, use
> +                          $ddir for --with-gdb-datadir path [$ddir/auto-load]
>    --without-auto-load-safe-path
>                            do not restrict auto-loaded files locations
>    --with-libunwind-ia64   use libunwind frame unwinding for ia64 targets
> @@ -4964,20 +4965,21 @@ $as_echo_n "checking for default auto-load safe-path... " >&6; }
>  # Check whether --with-auto-load-safe-path was given.
>  if test "${with_auto_load_safe_path+set}" = set; then :
>    withval=$with_auto_load_safe_path; if test "$with_auto_load_safe_path" = "no"; then
> -   with_auto_load_safe_path="/"
> - fi
> +     with_auto_load_safe_path="/"
> +     fi
>  else
> -  with_auto_load_safe_path="$prefix"
> +  with_auto_load_safe_path='$ddir/auto-load'
>  fi
>  
> +escape_dir=`echo $with_auto_load_safe_path | sed 's/[$]ddir\>/\\\\\\\\\\\\&/g'`
>  
>    test "x$prefix" = xNONE && prefix="$ac_default_prefix"
>    test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
> -  ac_define_dir=`eval echo $with_auto_load_safe_path`
> +  ac_define_dir=`eval echo $escape_dir`
>    ac_define_dir=`eval echo $ac_define_dir`
>  
>  cat >>confdefs.h <<_ACEOF
> -#define DEFAULT_AUTO_LOAD_SAFE_PATH "$ac_define_dir"
> +#define AUTO_LOAD_SAFE_PATH "$ac_define_dir"
>  _ACEOF
>  
>  
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -138,13 +138,16 @@ AS_HELP_STRING([--with-relocated-sources=PATH], [automatically relocate this pat
>  
>  AC_MSG_CHECKING([for default auto-load safe-path])
>  AC_ARG_WITH(auto-load-safe-path,
> -AS_HELP_STRING([--with-auto-load-safe-path=PATH], [directories safe to hold auto-loaded files])
> -AS_HELP_STRING([--without-auto-load-safe-path], [do not restrict auto-loaded files locations]),
> -[if test "$with_auto_load_safe_path" = "no"; then
> -   with_auto_load_safe_path="/"
> - fi],
> -[with_auto_load_safe_path="$prefix"])
> -AC_DEFINE_DIR(DEFAULT_AUTO_LOAD_SAFE_PATH, with_auto_load_safe_path,
> +AS_HELP_STRING([--with-auto-load-safe-path=PATH],
> +  [directories safe to hold auto-loaded files, use $ddir for --with-gdb-datadir path @<:@$ddir/auto-load@:>@])
> +AS_HELP_STRING([--without-auto-load-safe-path],
> +               [do not restrict auto-loaded files locations]),
> +    [if test "$with_auto_load_safe_path" = "no"; then
> +     with_auto_load_safe_path="/"
> +     fi],
> +[with_auto_load_safe_path='$ddir/auto-load'])
> +escape_dir=`echo $with_auto_load_safe_path | sed 's/[[$]]ddir\>/\\\\\\\\\\\\&/g'`
> +AC_DEFINE_DIR(AUTO_LOAD_SAFE_PATH, escape_dir,
>  	      [Directories safe to hold auto-loaded files.])
>  AC_MSG_RESULT([$with_auto_load_safe_path])
>  
> --- a/gdb/defs.h
> +++ b/gdb/defs.h
> @@ -381,6 +381,9 @@ extern struct cleanup *make_bpstat_clear_actions_cleanup (void);
>  
>  extern int producer_is_gcc_ge_4 (const char *producer);
>  
> +extern void substitute_path_component (char **stringp, const char *from,
> +				       const char *to);
> +
>  #ifdef HAVE_WAITPID
>  extern pid_t wait_to_die_with_timeout (pid_t pid, int *status, int timeout);
>  #endif
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -21007,7 +21007,7 @@ libthread-db:  Auto-loading of inferior specific libthread_db is on.
>  local-gdbinit:  Auto-loading of .gdbinit script from current directory is on.
>  python-scripts:  Auto-loading of Python scripts is on.
>  safe-path:  List of directories from which it is safe to auto-load files
> -            is /usr/local.
> +            is $ddir/auto-load.
>  @end smallexample
>  
>  @anchor{info auto-load}
> @@ -21209,9 +21209,9 @@ get loaded:
>  $ ./gdb -q ./gdb
>  Reading symbols from /home/user/gdb/gdb...done.
>  warning: File "/home/user/gdb/gdb-gdb.gdb" auto-loading has been
> -         declined by your `auto-load safe-path' set to "/usr/local".
> +         declined by your `auto-load safe-path' set to "$ddir/auto-load".
>  warning: File "/home/user/gdb/gdb-gdb.py" auto-loading has been
> -         declined by your `auto-load safe-path' set to "/usr/local".
> +         declined by your `auto-load safe-path' set to "$ddir/auto-load".
>  @end smallexample

I think these lines just got too long for @smallexample.  Did you try
to produce the PDF version, and if so, did you see these lines
overflow the margin?

> +be placed as a directory component - either alone or delimited by @file{/} or

"---", not "-".  We want an em-dash here. not a minus.

Okay with that.  Thanks.