[patch] auto-load safe-path default=$ddir/auto-load [Re: [patch] auto-load safe-path reset back by set ""]

Jan Kratochvil
2012-04-23T23:05:12Z

On Sun, 22 Apr 2012 23:26:16 +0200, Doug Evans wrote:
> A thought occurred to me regarding the default value of auto-load-path
> = ${prefix}.

This is unrelated to this patch but thanks for the suggestion.

> So I was wondering if we really want security to be on by default,
> should the default value be gdb's data-directory (e.g.,
> $prefix/share/gdb) + $exec_prefix/lib{,32,64} + ???

Made it therefore $ddir/auto-load, on an ideal system/distro we can change all
the auto-loaded GDB files to be located under $ddir/auto-load. I have filed
for the only remaining violation (/usr/bin/mono-gdb.py) known to me:
https://bugzilla.redhat.com/show_bug.cgi?id=815501
(Sure I will ask about upstreaming of the change.)

> Plus, it seems like at least data-directory should be relocatable.
> Implementing this might be cumbersome unless data-directory was
> represented as something like "$ddir".

Done. Unfortunately this still does not fix the "./gdb" run for a newly built
GDB. Newly built GDB probably could use "-data-directory $PWD/data-directory"
(if GDB's program dir contains "data-directory" sort of relocation).
We could then change current
gdb-gdb.gdb.in -> gdb-gdb.gdb
to
gdb-gdb.gdb.in -> data-directory/auto-load/$PWD/gdb-gdb.gdb
and even install the file (with proper installation directories) as:
/usr/share/gdb/usr/bin/gdb-gdb.gdb
(additionally ensuring for example in Fedora - in its .spec file
@srcdir@ gets substituted right for Fedora *-debuginfo.rpm)

Would it make everyone happy?

Thanks,
Jan

gdb/
2012-04-23 Jan Kratochvil <jan.kratochvil@redhat.com>

Change auto-load safe-path default to $ddir/auto-load.
* auto-load.c (auto_load_safe_path_vec_update): Call
substitute_path_component for $ddir.
* configure: Regenerate.
* configure.ac (--with-auto-load-safe-path): Suggest $ddir syntax.
Change the default to \\\$ddir/auto-load.
* defs.h (substitute_path_component): New declaration.
* utils.c (substitute_path_component): New function.

gdb/doc/
2012-04-23 Jan Kratochvil <jan.kratochvil@redhat.com>

Change auto-load safe-path default to $ddir/auto-load.
* gdb.texinfo (Auto-loading): Change shown safe-path default to
$ddir/auto-load.
(Auto-loading safe path): Change the sample warning to $ddir/auto-load.
Twice. Mention the $ddir substitution.

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 9d19179..81d48ee 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -141,8 +141,12 @@ 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 *expanded, *real_path;
+
+ expanded = tilde_expand (dir);
+ substitute_path_component (&expanded, "$ddir", gdb_datadir);
+
+ real_path = gdb_realpath (expanded);

/* Ensure the current entry is at least tilde_expand-ed. */
VEC_replace (char_ptr, auto_load_safe_path_vec, ix, expanded);
diff --git a/gdb/configure b/gdb/configure
index 54c2399..e0f2d64 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -1481,7 +1481,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 -data-directory
--without-auto-load-safe-path
do not restrict auto-loaded files locations
--with-libunwind-ia64 use libunwind frame unwinding for ia64 targets
@@ -4952,7 +4953,7 @@ if test "${with_auto_load_safe_path+set}" = set; then :
with_auto_load_safe_path=""
fi
else
- with_auto_load_safe_path="$prefix"
+ with_auto_load_safe_path='\\\$ddir/auto-load'
fi

diff --git a/gdb/configure.ac b/gdb/configure.ac
index a40c2e5..a80cfcd 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -137,12 +137,13 @@ 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([--with-auto-load-safe-path=PATH],
+ [directories safe to hold auto-loaded files, use '\\\$ddir' for -data-directory])
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"])
+[with_auto_load_safe_path='\\\$ddir/auto-load'])
AC_DEFINE_DIR(DEFAULT_AUTO_LOAD_SAFE_PATH, with_auto_load_safe_path,
[Directories safe to hold auto-loaded files.])
AC_MSG_RESULT([$with_auto_load_safe_path])
diff --git a/gdb/defs.h b/gdb/defs.h
index f7156cb..caff109 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -380,6 +380,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
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a2a4eb3..a3873fb 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20893,7 +20893,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}
@@ -21095,9 +21095,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

The list of trusted directories is controlled by the following commands:
@@ -21126,6 +21126,11 @@ loading and execution of scripts. Multiple entries may be delimited by the
host platform directory separator in use.
@end table

+Any used string @file{$ddir} will get replaced by @var{data-directory} which is
+determined at @value{GDBN} startup (@pxref{Data Files}). @file{$ddir} must be
+be placed as a directory component - either alone or delimited by @file{/} or
+@file{\} directory separators, depending on the host platform.
+
Setting this variable to an empty string disables this security protection.
This variable is supposed to be set to the system directories writable by the
system superuser only. Users can add their source directories in init files in
diff --git a/gdb/utils.c b/gdb/utils.c
index b70edd8..15956b7 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3724,6 +3724,48 @@ dirnames_to_char_ptr_vec (const char *dirnames)
return retval;
}

+/* Substitute all occurences of string FROM by string TO in *STRINGP. *STRINGP
+ must come from xrealloc-compatible allocator and it may be updated. FROM
+ needs to be delimited by IS_DIR_SEPARATOR (or be located at the start or
+ end of *STRINGP. */
+
+void
+substitute_path_component (char **stringp, const char *from, const char *to)
+{
+ char *string = *stringp, *s;
+ const size_t from_len = strlen (from);
+ const size_t to_len = strlen (to);
+
+ for (s = string;;)
+ {
+ s = strstr (s, from);
+ if (s == NULL)
+ break;
+
+ if ((s == string || IS_DIR_SEPARATOR (s[-1]))
+ && (s[from_len] == '\0' || IS_DIR_SEPARATOR (s[from_len])))
+ {
+ char *string_new;
+
+ string_new = xrealloc (string, (strlen (string) + to_len + 1));
+
+ /* Relocate the current S pointer. */
+ s = s - string + string_new;
+ string = string_new;
+
+ /* Replace from by to. */
+ memmove (&s[to_len], &s[from_len], strlen (&s[from_len]) + 1);
+ memcpy (s, to, to_len);
+
+ s += to_len;
+ }
+ else
+ s++;
+ }
+
+ *stringp = string;
+}
+
#ifdef HAVE_WAITPID

#ifdef SIGALRM