public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [patch] New set auto-load-local-gdbinit + disable it by default
@ 2012-01-17 10:16 Jan Kratochvil
  2012-01-17 12:34 ` Eli Zaretskii
                   ` (4 more replies)
  0 siblings, 5 replies; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 10:16 UTC (permalink / raw)
  To: gdb-patches

Hi,

this is a patch I want to post for many years.  There was:
	[RFA] .gdbinit security (revived) [incl doc]
	http://sourceware.org/ml/gdb-patches/2010-11/msg00276.html
which was a follow-up for its referenced:
	RFC: Check permissions of .gdbinit files
	http://sourceware.org/ml/gdb-patches/2005-05/msg00637.html
which was addressing:
	http://cve.mitre.org/cgi-bin/cvename.cgi?name=2005-1705
Current Fedora patch (AFAIK in some way contained in many other distros):
	http://pkgs.fedoraproject.org/gitweb/?p=gdb.git;a=blob;f=gdb-6.3-security-errata-20050610.patch;hb=master

There is always discussion whether != UID and/or != GID is secure enough vs.
convenient enough.

But from my experience any UID or GID policies just cannot work:
	Save Bugzilla bugreport attachment crash.tar.gz as a regular user.
	$ tar xzf crash.tar.gz; cd crash
	$ gdb crashprog
	 - You are 0wn3d!

Besides security problems the automatic execution is even inconvenient:
	$ gdb testsuite/gdb.base/return
	[...]
	Setting up the environment for debugging gdb.
	Function "internal_error" not defined.
	Make breakpoint pending on future shared library load? (y or [n]) [answered N; input not from terminal]
	Function "info_command" not defined.
	Make breakpoint pending on future shared library load? (y or [n]) [answered N; input not from terminal]
	.gdbinit:8: Error in sourced command file:
	Argument required (one or more breakpoint numbers).
	 - What had happened?  Oh, I forgot -nx again!

I even always run my `gdbn' as otherwise some random .gdbinit occasionally
gets executed and I get some unknown debugging environment I have to quit:
	function gdbn { gdb -nx --command=~/.gdbinit "$@"; }

I do not see a precedent for executing anything from current directory by
default.  . (current directory) is also not contained in $PATH at all.
bash also does not execute .bashrc in any current directory.
And "gdb -x ./.gdbinit" is a pretty simple way to do what one wants to do.

From what I know still there may be a resistance to this change, Eli please
save your work with doc reviewing only after the change has been approved.

Still at least the setting should go in and then one can then have
"set auto-load-local-gdbinit off" at least in ~/.gdbinit.   Anyway I would
file a FESCo (Fedora Engineering Steering Committee) ticket for such "off" in
/etc/gdbinit at least in distro and IMHO it needs to get approved (but maybe
not, it would be another fork from upstream).

No regressions on {x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu.

I do not think a testcase makes sense but I may make one if requested.


Thanks,
Jan


gdb/
2012-01-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	New set auto-load-local-gdbinit + disable it by default.
	* NEWS: New item.
	* main.c (captured_main): Execute LOCAL_GDBINIT only if
	AUTO_LOAD_LOCAL_GDBINIT_P.
	(print_gdb_help): New note for LOCAL_GDBINIT.
	* top.c (auto_load_local_gdbinit_p, show_auto_load_local_gdbinit_p):
	New.
	(init_main): Call add_setshow_boolean_cmd for "auto-load-local-gdbinit".
	* top.h (auto_load_local_gdbinit_p): New declaration.

gdb/doc/
2012-01-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.texinfo (Startup): Describe set auto-load-local-gdbinit and its
	default off now.

--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -30,6 +30,9 @@
   now set a breakpoint in build/gcc/expr.c, but not
   build/libcpp/expr.c.
 
+* GDB no longer reads .gdbinit file from current directory by default.
+  Use "gdb -x .gdbinit" to retain the original behavior.
+
 *** Changes in GDB 7.4
 
 * GDB now handles ambiguous linespecs more consistently; the existing
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1250,12 +1250,34 @@ that file.
 Processes command line options and operands.
 
 @item
-Reads and executes the commands from init file (if any) in the current
-working directory.  This is only done if the current directory is
-different from your home directory.  Thus, you can have more than one
-init file, one generic in your home directory, and another, specific
-to the program you are debugging, in the directory where you invoke
-@value{GDBN}.
+If you have explicitly set @samp{set auto-load-local-gdbinit on} then
+@value{GDBN} reads and executes the commands from init file (if any) in
+the current working directory.  This is only done if the current
+directory is different from your home directory.  Thus, you can have
+more than one init file, one generic in your home directory, and
+another, specific to the program you are debugging, in the directory
+where you invoke @value{GDBN}.
+
+Setting it to @samp{on} has security implications if you run
+@value{GDBN} from a directory with untrusted files, such as home
+directories of other users, shared temporary directories or extracted
+downloaded archives.  Appropriate @samp{set auto-load-local-gdbinit}
+command can be also placed into the system-wide init file or into the
+init file in your home directory.
+
+@table @code
+@kindex set auto-load-local-gdbinit
+@item set auto-load-local-gdbinit [yes|no]
+Enable or disable the auto-loading of init file (if any) in the current
+working directory.  The default is @samp{set auto-load-local-gdbinit
+off}---no file from current working directory is executed during
+startup.
+
+@kindex show auto-load-local-gdbinit
+@item show auto-load-local-gdbinit
+Show whether auto-loading of init file (if any) in the current working
+directory is enabled or disabled.
+@end table
 
 @item
 If the command line specified a program to debug, or a process to
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -898,7 +898,7 @@ captured_main (void *data)
 
   /* Read the .gdbinit file in the current directory, *if* it isn't
      the same as the $HOME/.gdbinit file (it should exist, also).  */
-  if (local_gdbinit && !inhibit_gdbinit)
+  if (local_gdbinit && !inhibit_gdbinit && auto_load_local_gdbinit_p)
     catch_command_errors (source_script, local_gdbinit, 0, RETURN_MASK_ALL);
 
   /* Now that all .gdbinit's have been read and all -d options have been
@@ -1042,7 +1042,7 @@ At startup, GDB reads the following init files and executes their commands:\n\
 "), home_gdbinit);
   if (local_gdbinit)
     fprintf_unfiltered (stream, _("\
-   * local init file: ./%s\n\
+   * local init file (if set auto-load-local-gdbinit is on): ./%s\n\
 "), local_gdbinit);
   fputs_unfiltered (_("\n\
 For more information, type \"help\" from within GDB, or consult the\n\
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1174,6 +1174,21 @@ set_prompt (const char *s)
   xfree (top_prompt);
   top_prompt = p;
 }
+
+/* Set to non-zero to automatically load file ./.gdbinit during GDB
+   startup.  */
+int auto_load_local_gdbinit_p = 0;
+
+/* Show the current state of AUTO_LOAD_LOCAL_GDBINIT_P.  */
+
+static void
+show_auto_load_local_gdbinit_p (struct ui_file *file, int from_tty,
+				struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("\
+Automatic loading of ./.gdbinit file during GDB startup is %s.\n"),
+		    value);
+}
 \f
 
 struct qt_args
@@ -1666,6 +1681,16 @@ When set, GDB uses the specified path to search for data files."),
                            NULL, NULL,
                            &setlist,
                            &showlist);
+
+  add_setshow_boolean_cmd ("auto-load-local-gdbinit", class_support,
+			   &auto_load_local_gdbinit_p, _("\
+Set to automatically load file ./.gdbinit during GDB startup."), _("\
+Show automatic load of file ./.gdbinit during GDB startup."), _("\
+Automatic loading may have security implications if you start GDB in\n\
+a directory with untrusted files."),
+			   NULL,
+			   show_auto_load_local_gdbinit_p,
+			   &setlist, &showlist);
 }
 
 void
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -82,4 +82,6 @@ extern void set_verbose (char *, int, struct cmd_list_element *);
 
 extern void do_restore_instream_cleanup (void *stream);
 
+extern int auto_load_local_gdbinit_p;
+
 #endif

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 10:16 [patch] New set auto-load-local-gdbinit + disable it by default Jan Kratochvil
@ 2012-01-17 12:34 ` Eli Zaretskii
  2012-01-17 13:42   ` Joel Brobecker
  2012-01-17 20:22   ` [patch] New set auto-load-local-gdbinit + disable it by default Tom Tromey
  2012-01-17 16:15 ` Doug Evans
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 54+ messages in thread
From: Eli Zaretskii @ 2012-01-17 12:34 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

> Date: Tue, 17 Jan 2012 10:55:52 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> >From what I know still there may be a resistance to this change, Eli please
> save your work with doc reviewing only after the change has been approved.

Thanks for the heads-up, I'm one of those who will resist ;-)

> Still at least the setting should go in and then one can then have
> "set auto-load-local-gdbinit off" at least in ~/.gdbinit.   Anyway I would
> file a FESCo (Fedora Engineering Steering Committee) ticket for such "off" in
> /etc/gdbinit at least in distro and IMHO it needs to get approved (but maybe
> not, it would be another fork from upstream).

I think this is a draconian measure.  It prevents me from having a
.gdbinit file loaded automatically as appropriate for a program I'm
debugging.  Prominent examples include GDB itself and Emacs, which
both come with a .gdbinit file that makes debugging much easier.

> And "gdb -x ./.gdbinit" is a pretty simple way to do what one wants to do.

"gdb -x .gdbinit" is much longer than just "gdb".  It could really
make a difference in some special use cases, e.g. when you need to
attach to a failing process very quickly.  It is also easy to forget.

So I'm very much against this incompatible change in behavior, FWIW.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 12:34 ` Eli Zaretskii
@ 2012-01-17 13:42   ` Joel Brobecker
  2012-01-17 14:49     ` [patch 7.4] Deprecate local .gdbinit [Re: [patch] New set auto-load-local-gdbinit + disable it by default] Jan Kratochvil
  2012-01-17 20:22   ` [patch] New set auto-load-local-gdbinit + disable it by default Tom Tromey
  1 sibling, 1 reply; 54+ messages in thread
From: Joel Brobecker @ 2012-01-17 13:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Jan Kratochvil, gdb-patches

> I think this is a draconian measure.  It prevents me from having a
> .gdbinit file loaded automatically as appropriate for a program I'm
> debugging.  Prominent examples include GDB itself and Emacs, which
> both come with a .gdbinit file that makes debugging much easier.

I tend to agree as well, although I would not object as strongly.

If we do move forward on that change, I think we should do it more
gradually:
  1. We start by emitting a warning when seeing a local .gdbinit file,
     but otherwise read it.
  2. We stop reading them once a release with the warning is out.

> > And "gdb -x ./.gdbinit" is a pretty simple way to do what one wants to do.
> 
> "gdb -x .gdbinit" is much longer than just "gdb".  It could really
> make a difference in some special use cases, e.g. when you need to
> attach to a failing process very quickly.  It is also easy to forget.

I also agree it's (too) easy to forget.

-- 
Joel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [patch 7.4] Deprecate local .gdbinit  [Re: [patch] New set auto-load-local-gdbinit + disable it by default]
  2012-01-17 13:42   ` Joel Brobecker
@ 2012-01-17 14:49     ` Jan Kratochvil
  2012-01-17 16:22       ` Doug Evans
  0 siblings, 1 reply; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 14:49 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Eli Zaretskii, gdb-patches

On Tue, 17 Jan 2012 14:39:13 +0100, Joel Brobecker wrote:
> If we do move forward on that change, I think we should do it more
> gradually:
>   1. We start by emitting a warning when seeing a local .gdbinit file,
>      but otherwise read it.
>   2. We stop reading them once a release with the warning is out.

Like still into the 7.4 branch?

No regressions for gdb_7_4-branch on
{x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu.

OTOH TBH I do not think from #gdb etc. users in general use very every GDB
release with the current rapid release cycles.


Thanks,
Jan


gdb/
2012-01-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Deprecate reading of file .gdbinit in current directory.
	* NEWS: New entry.
	* main.c: Include readline/tilde.h.
	(get_init_files): New parameter local_gdbinit_stat.  Describe it in the
	function comment.  Change local variable cwdbuf to localinit_stat.
	(captured_main): New variable local_gdbinit_stat, initialize it by
	get_init_files.  Check local_gdbinit duplicity against CMDARG.  New
	warning before calling catch_command_errors for local_gdbinit.
	(print_gdb_help): Update the get_init_files caller.  New message for
	"local init file".

--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -22,6 +22,10 @@
   watchpoints are slower than real hardware watchpoints but are
   significantly faster than gdb software watchpoints.
 
+* Automatic reading of file .gdbinit in current directory has been deprecated
+  and it will be removed in gdb-7.5.
+  Use explicit 'gdb -x .gdbinit ...' command instead.
+
 * Python scripting
 
   ** The register_pretty_printer function in module gdb.printing now takes
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -41,6 +41,7 @@
 #include "cli/cli-cmds.h"
 #include "python/python.h"
 #include "objfiles.h"
+#include "readline/tilde.h"
 
 /* The selected interpreter.  This will be used as a set command
    variable, so it should always be malloc'ed - since
@@ -152,23 +153,26 @@ relocate_gdb_directory (const char *initial, int flag)
 }
 
 /* Compute the locations of init files that GDB should source and
-   return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT.  If
-   there is no system gdbinit (resp. home gdbinit and local gdbinit)
-   to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
-   LOCAL_GDBINIT) is set to NULL.  */
+   return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT and
+   LOCAL_GDBINIT_STAT.  If there is no system gdbinit (resp. home
+   gdbinit and local gdbinit) to be loaded, then SYSTEM_GDBINIT (resp.
+   HOME_GDBINIT and LOCAL_GDBINIT) is set to NULL, LOCAL_GDBINIT_STAT is
+   zeroed.  */
+
 static void
 get_init_files (char **system_gdbinit,
 		char **home_gdbinit,
-		char **local_gdbinit)
+		char **local_gdbinit, struct stat *local_gdbinit_stat)
 {
   static char *sysgdbinit = NULL;
   static char *homeinit = NULL;
   static char *localinit = NULL;
+  static struct stat localinit_stat;
   static int initialized = 0;
 
   if (!initialized)
     {
-      struct stat homebuf, cwdbuf, s;
+      struct stat homebuf, s;
       char *homedir, *relocated_sysgdbinit;
 
       if (SYSTEM_GDBINIT[0])
@@ -186,12 +190,12 @@ get_init_files (char **system_gdbinit,
 
       /* If the .gdbinit file in the current directory is the same as
 	 the $HOME/.gdbinit file, it should not be sourced.  homebuf
-	 and cwdbuf are used in that purpose.  Make sure that the stats
-	 are zero in case one of them fails (this guarantees that they
-	 won't match if either exists).  */
+	 and localinit_stat are used in that purpose.  Make sure
+	 that the stats are zero in case one of them fails (this
+	 guarantees that they won't match if either exists).  */
 
       memset (&homebuf, 0, sizeof (struct stat));
-      memset (&cwdbuf, 0, sizeof (struct stat));
+      memset (&localinit_stat, 0, sizeof (struct stat));
 
       if (homedir)
 	{
@@ -203,11 +207,10 @@ get_init_files (char **system_gdbinit,
 	    }
 	}
 
-      if (stat (gdbinit, &cwdbuf) == 0)
+      if (stat (gdbinit, &localinit_stat) == 0)
 	{
 	  if (!homeinit
-	      || memcmp ((char *) &homebuf, (char *) &cwdbuf,
-			 sizeof (struct stat)))
+	      || memcmp (&homebuf, &localinit_stat, sizeof (struct stat)))
 	    localinit = gdbinit;
 	}
       
@@ -217,6 +220,8 @@ get_init_files (char **system_gdbinit,
   *system_gdbinit = sysgdbinit;
   *home_gdbinit = homeinit;
   *local_gdbinit = localinit;
+  if (local_gdbinit_stat)
+    *local_gdbinit_stat = localinit_stat;
 }
 
 /* Call command_loop.  If it happens to return, pass that through as a
@@ -293,6 +298,7 @@ captured_main (void *data)
   char *system_gdbinit;
   char *home_gdbinit;
   char *local_gdbinit;
+  struct stat local_gdbinit_stat;
 
   int i;
   int save_auto_load;
@@ -727,7 +733,8 @@ captured_main (void *data)
   /* Lookup gdbinit files.  Note that the gdbinit file name may be
      overriden during file initialization, so get_init_files should be
      called after gdb_init.  */
-  get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
+  get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit,
+		  &local_gdbinit_stat);
 
   /* Do these (and anything which might call wrap_here or *_filtered)
      after initialize_all_files() but before the interpreter has been
@@ -899,7 +906,45 @@ captured_main (void *data)
   /* Read the .gdbinit file in the current directory, *if* it isn't
      the same as the $HOME/.gdbinit file (it should exist, also).  */
   if (local_gdbinit && !inhibit_gdbinit)
-    catch_command_errors (source_script, local_gdbinit, 0, RETURN_MASK_ALL);
+    {
+      for (i = 0; i < ncmd; i++)
+	if (cmdarg[i].type == CMDARG_FILE)
+	  {
+	    struct cleanup *old_cleanups;
+	    char *file;
+	    int fd;
+
+	    file = tilde_expand (cmdarg[i].string);
+	    old_cleanups = make_cleanup (xfree, file);
+
+	    fd = openp (source_path, OPF_TRY_CWD_FIRST, file, O_RDONLY, NULL);
+
+	    do_cleanups (old_cleanups);
+
+	    if (fd != -1)
+	      {
+		struct stat statbuf;
+
+		if (fstat (fd, &statbuf) == 0
+		    && memcmp (&statbuf, &local_gdbinit_stat,
+			       sizeof (statbuf)) == 0)
+		  local_gdbinit = NULL;
+
+		close (fd);
+		if (local_gdbinit == NULL)
+		  break;
+	      }
+	  }
+
+      if (local_gdbinit)
+	{
+	  warning (_("Automatic reading of file .gdbinit in current directory "
+		     "has been deprecated and it will be removed in gdb-7.5.  "
+		     "Use explicit 'gdb -x .gdbinit ...' command instead."));
+	  catch_command_errors (source_script, local_gdbinit, 0,
+				RETURN_MASK_ALL);
+	}
+    }
 
   /* Now that all .gdbinit's have been read and all -d options have been
      processed, we can read any scripts mentioned in SYMARG.
@@ -966,7 +1011,7 @@ print_gdb_help (struct ui_file *stream)
   char *home_gdbinit;
   char *local_gdbinit;
 
-  get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
+  get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit, NULL);
 
   fputs_unfiltered (_("\
 This is the GNU debugger.  Usage:\n\n\
@@ -1042,7 +1087,7 @@ At startup, GDB reads the following init files and executes their commands:\n\
 "), home_gdbinit);
   if (local_gdbinit)
     fprintf_unfiltered (stream, _("\
-   * local init file: ./%s\n\
+   * local init file (it will no longer be read by default since gdb-7.5): ./%s\n\
 "), local_gdbinit);
   fputs_unfiltered (_("\n\
 For more information, type \"help\" from within GDB, or consult the\n\

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 10:16 [patch] New set auto-load-local-gdbinit + disable it by default Jan Kratochvil
  2012-01-17 12:34 ` Eli Zaretskii
@ 2012-01-17 16:15 ` Doug Evans
  2012-01-17 16:34   ` Jan Kratochvil
  2012-01-17 16:26 ` Matt Rice
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 54+ messages in thread
From: Doug Evans @ 2012-01-17 16:15 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Tue, Jan 17, 2012 at 1:55 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Besides security problems the automatic execution is even inconvenient:
>        $ gdb testsuite/gdb.base/return
>        [...]
>        Setting up the environment for debugging gdb.
>        Function "internal_error" not defined.
>        Make breakpoint pending on future shared library load? (y or [n]) [answered N; input not from terminal]
>        Function "info_command" not defined.
>        Make breakpoint pending on future shared library load? (y or [n]) [answered N; input not from terminal]
>        .gdbinit:8: Error in sourced command file:
>        Argument required (one or more breakpoint numbers).
>         - What had happened?  Oh, I forgot -nx again!

This only affects gdb developers though.
Another way to go is to enhance gdb's .gdbinit to check for which
binary is being debugged and only do those things when it's gdb (and
if necessary enhance the scripting language to support such a check).
Seems generally useful, we should add support for it anyway.

One problem I have with -nx is that it also turns off system.gdbinit.
I've sometimes wanted to turn off everything but system.gdbinit,
without having to specify the path to system.gdbinit in -x.

> I do not see a precedent for executing anything from current directory by
> default.  . (current directory) is also not contained in $PATH at all.
> bash also does not execute .bashrc in any current directory.
> And "gdb -x ./.gdbinit" is a pretty simple way to do what one wants to do.

Well, there is make (and I'm sure others).  E.g.,
echo "default:; @echo Gotcha." > GNUmakefile && make
:-)

> From what I know still there may be a resistance to this change, Eli please
> save your work with doc reviewing only after the change has been approved.
>
> Still at least the setting should go in and then one can then have
> "set auto-load-local-gdbinit off" at least in ~/.gdbinit.   Anyway I would
> file a FESCo (Fedora Engineering Steering Committee) ticket for such "off" in
> /etc/gdbinit at least in distro and IMHO it needs to get approved (but maybe
> not, it would be another fork from upstream).

I don't mind "set auto-load-local-gdbinit", though "set auto-load
local-gdbinit" feels better, I could do "show auto-load" and see all
the auto-load settings (assuming we migrate "auto-load-scripts" to
"auto-load scripts" - though I'm beginning to like plain "scripts" in
the name less ...).
If you want to default it to "off", I think I'd give several releases
warning notice,
e.g., at least a year, to give enough time to change our minds if the
user community really doesn't want it. :-)

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch 7.4] Deprecate local .gdbinit [Re: [patch] New set auto-load-local-gdbinit + disable it by default]
  2012-01-17 14:49     ` [patch 7.4] Deprecate local .gdbinit [Re: [patch] New set auto-load-local-gdbinit + disable it by default] Jan Kratochvil
@ 2012-01-17 16:22       ` Doug Evans
  0 siblings, 0 replies; 54+ messages in thread
From: Doug Evans @ 2012-01-17 16:22 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Joel Brobecker, Eli Zaretskii, gdb-patches

On Tue, Jan 17, 2012 at 6:45 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 17 Jan 2012 14:39:13 +0100, Joel Brobecker wrote:
>> If we do move forward on that change, I think we should do it more
>> gradually:
>>   1. We start by emitting a warning when seeing a local .gdbinit file,
>>      but otherwise read it.
>>   2. We stop reading them once a release with the warning is out.
>
> Like still into the 7.4 branch?

IMO this should not go in 7.4.
I think more discussion is needed, and we don't want to hold up 7.4 for it.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 10:16 [patch] New set auto-load-local-gdbinit + disable it by default Jan Kratochvil
  2012-01-17 12:34 ` Eli Zaretskii
  2012-01-17 16:15 ` Doug Evans
@ 2012-01-17 16:26 ` Matt Rice
  2012-01-17 16:57   ` Doug Evans
  2012-01-17 20:09 ` Tom Tromey
  2012-01-24  0:33 ` Stan Shebs
  4 siblings, 1 reply; 54+ messages in thread
From: Matt Rice @ 2012-01-17 16:26 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Tue, Jan 17, 2012 at 1:55 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Still at least the setting should go in and then one can then have
> "set auto-load-local-gdbinit off" at least in ~/.gdbinit.   Anyway I would
> file a FESCo (Fedora Engineering Steering Committee) ticket for such "off" in
> /etc/gdbinit at least in distro and IMHO it needs to get approved (but maybe
> not, it would be another fork from upstream).

I'm of the opinion it should just be nuked outright, and people who
want the old behaviour should add something to the effect of the
following to their ~/.gdbinit, I fear a on/off is just going to lead
to different behaviours across distros, (and still i wish the
cwd/.gdbinit wasn't hidden/too late for that.)

py
if os.getcwd() != os.environ['HOME']:
  gdb.execute("source .gdbinit")
end

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 16:15 ` Doug Evans
@ 2012-01-17 16:34   ` Jan Kratochvil
  2012-01-17 16:48     ` Doug Evans
  2012-01-17 18:00     ` Eli Zaretskii
  0 siblings, 2 replies; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 16:34 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

On Tue, 17 Jan 2012 17:11:19 +0100, Doug Evans wrote:
> On Tue, Jan 17, 2012 at 1:55 AM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
> > Besides security problems the automatic execution is even inconvenient:
> >        $ gdb testsuite/gdb.base/return
> >        [...]
> >        Setting up the environment for debugging gdb.
> >        Function "internal_error" not defined.
> >        Make breakpoint pending on future shared library load? (y or [n]) [answered N; input not from terminal]
> >        Function "info_command" not defined.
> >        Make breakpoint pending on future shared library load? (y or [n]) [answered N; input not from terminal]
> >        .gdbinit:8: Error in sourced command file:
> >        Argument required (one or more breakpoint numbers).
> >         - What had happened?  Oh, I forgot -nx again!
> 
> This only affects gdb developers though.

It affects also gcc, gas, emacs, former Frysk, ReactOS, there will be more
malicious .gdbinit files; not listed non-intrusive .gdbinit files which only
define some new commands.


> Another way to go is to enhance gdb's .gdbinit to check for which
> binary is being debugged and only do those things when it's gdb (and
> if necessary enhance the scripting language to support such a check).
> Seems generally useful, we should add support for it anyway.

<bite> When one wants to debug gdb in gdb the file gdb/.gdbinit is really the
most destructive as all its various commands succeed making the debugging
session completely unusable. </bite>


> One problem I have with -nx is that it also turns off system.gdbinit.
> I've sometimes wanted to turn off everything but system.gdbinit,
> without having to specify the path to system.gdbinit in -x.

I do not use system.gdbinit but I use home.gdbinit sharing the problem.


> Well, there is make (and I'm sure others).  E.g.,
> echo "default:; @echo Gotcha." > GNUmakefile && make
> :-)

I was even thinking about make but I do not find it as a valid case.
The primary goal of `make' is to read local Makefile - therefore one cannot be
surprised by it.  The primary goal of GDB is to process its arguments.
I would bet some GDB users do not know there exists anything like .gdbinit.


> I don't mind "set auto-load-local-gdbinit", though "set auto-load
> local-gdbinit" feels better, I could do "show auto-load" and see all
> the auto-load settings (assuming we migrate "auto-load-scripts" to
> "auto-load scripts" - though I'm beginning to like plain "scripts" in
> the name less ...).

I sure followed "auto-load-scripts".  I can change it (later).


> If you want to default it to "off", I think I'd give several releases
> warning notice,
> e.g., at least a year, to give enough time to change our minds if the
> user community really doesn't want it. :-)

I find it OK that way.  Maybe there will be some general disagreement.

I need to know this decision to choose the way fixing next security hole
- "set auto-load-scripts" + libthread_db.so.1 as implemented by Google.
So that the other security options/commands can assume it that way.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 16:34   ` Jan Kratochvil
@ 2012-01-17 16:48     ` Doug Evans
  2012-01-17 17:27       ` Jan Kratochvil
  2012-01-17 18:00     ` Eli Zaretskii
  1 sibling, 1 reply; 54+ messages in thread
From: Doug Evans @ 2012-01-17 16:48 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Tue, Jan 17, 2012 at 8:26 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
>> Another way to go is to enhance gdb's .gdbinit to check for which
>> binary is being debugged and only do those things when it's gdb (and
>> if necessary enhance the scripting language to support such a check).
>> Seems generally useful, we should add support for it anyway.
>
> <bite> When one wants to debug gdb in gdb the file gdb/.gdbinit is really the
> most destructive as all its various commands succeed making the debugging
> session completely unusable. </bite>

<bite> ???  I don't understand.

I also don't understand the reference to making the debugging session
completely unusable.

>> Well, there is make (and I'm sure others).  E.g.,
>> echo "default:; @echo Gotcha." > GNUmakefile && make
>> :-)
>
> I was even thinking about make but I do not find it as a valid case.
> The primary goal of `make' is to read local Makefile - therefore one cannot be
> surprised by it.

The user will be surprised if s/he has been using "Makefile" and not
noticing that someone slipped in "GNUmakefile".  S/he might see it and
not even know that it trumps his/her own Makefile.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 16:26 ` Matt Rice
@ 2012-01-17 16:57   ` Doug Evans
  0 siblings, 0 replies; 54+ messages in thread
From: Doug Evans @ 2012-01-17 16:57 UTC (permalink / raw)
  To: Matt Rice; +Cc: Jan Kratochvil, gdb-patches

On Tue, Jan 17, 2012 at 8:22 AM, Matt Rice <ratmice@gmail.com> wrote:
> On Tue, Jan 17, 2012 at 1:55 AM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
>> Still at least the setting should go in and then one can then have
>> "set auto-load-local-gdbinit off" at least in ~/.gdbinit.   Anyway I would
>> file a FESCo (Fedora Engineering Steering Committee) ticket for such "off" in
>> /etc/gdbinit at least in distro and IMHO it needs to get approved (but maybe
>> not, it would be another fork from upstream).
>
> I'm of the opinion it should just be nuked outright, and people who
> want the old behaviour should add something to the effect of the
> following to their ~/.gdbinit, I fear a on/off is just going to lead
> to different behaviours across distros, (and still i wish the
> cwd/.gdbinit wasn't hidden/too late for that.)
>
> py
> if os.getcwd() != os.environ['HOME']:
>  gdb.execute("source .gdbinit")
> end

[for reference sake]
That won't behave the same way as is.
Find a way to do that *after* the symbols of the program have been
loaded and then you've got something (hook into new objfile
notifications or some such)

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 16:48     ` Doug Evans
@ 2012-01-17 17:27       ` Jan Kratochvil
  2012-01-17 17:33         ` Doug Evans
  0 siblings, 1 reply; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 17:27 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

On Tue, 17 Jan 2012 17:44:32 +0100, Doug Evans wrote:
> <bite> ???  I don't understand.

I was exaggerating the "(top-gdb)" & co. to be "destructive" but I feel it so.


> I also don't understand the reference to making the debugging session
> completely unusable.

Personally I always feared what is that "(top-gdb)" there and what new
breakpoints it created and why it prints complaints which are not printed
normally etc. etc., I just rather quit and and run normal GDB for GDB.

If I want some special environment (.gdbinit) for GDB-to-GDB I can study the
supplied one and _then_ I can load the supplied one.  For a newbie it is
a needless burden to complicate the debugging of GDB even more.


> The user will be surprised if s/he has been using "Makefile" and not
> noticing that someone slipped in "GNUmakefile".  S/he might see it and
> not even know that it trumps his/her own Makefile.

GNUmakefile vs. Makefile... OK.  But running "make" in untrusted source
directory is never safe as Makefile itself is not trusted.

While I occasionally do run "gdb -nx file core" on untrusted crash tarballs
- supplied by customers.  Now I have to use secured VM for it after Python and
libthread_db.so.1 automatic loading crept in.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 17:27       ` Jan Kratochvil
@ 2012-01-17 17:33         ` Doug Evans
  2012-01-17 17:55           ` Jan Kratochvil
  2012-01-17 19:30           ` Matt Rice
  0 siblings, 2 replies; 54+ messages in thread
From: Doug Evans @ 2012-01-17 17:33 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Tue, Jan 17, 2012 at 8:56 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 17 Jan 2012 17:44:32 +0100, Doug Evans wrote:
>> <bite> ???  I don't understand.
>
> I was exaggerating the "(top-gdb)" & co. to be "destructive" but I feel it so.
>
>
>> I also don't understand the reference to making the debugging session
>> completely unusable.
>
> Personally I always feared what is that "(top-gdb)" there and what new
> breakpoints it created and why it prints complaints which are not printed
> normally etc. etc., I just rather quit and and run normal GDB for GDB.

I, OTOH, like the (top-gdb) prompt.
When I'm switching back and forth between parent and child gdb, I like
trivially knowing which one I'm in.
I also like the automagic breakpoints on internal_error, and other stuff.

> If I want some special environment (.gdbinit) for GDB-to-GDB I can study the
> supplied one and _then_ I can load the supplied one.  For a newbie it is
> a needless burden to complicate the debugging of GDB even more.

One could argue the converse too ... Having to manually request
whatever goodies the developers find useful adds a needless burden.
Why can't gdb just auto-adjust itself for debugging the program at
hand?

>> The user will be surprised if s/he has been using "Makefile" and not
>> noticing that someone slipped in "GNUmakefile".  S/he might see it and
>> not even know that it trumps his/her own Makefile.
>
> GNUmakefile vs. Makefile... OK.  But running "make" in untrusted source
> directory is never safe as Makefile itself is not trusted.
>
> While I occasionally do run "gdb -nx file core" on untrusted crash tarballs
> - supplied by customers.  Now I have to use secured VM for it after Python and
> libthread_db.so.1 automatic loading crept in.

libthread_db has been auto-loaded for ages AFAIR - user's wanted
threaded debugging to Just Work ...
[insert discussion of removing the few remaining reasons for having
libthread_db]
There is the path that gdb searches for libthread_db, maybe you meant
to refer to that;
one can change it before choosing the file to debug.   You seem to
suggest that -nx was "good enough" before.  If that was good enough
before (and that's a big "if"), why is changing the search path first
not also "good enough".

As for Python auto-loading, if -nx was good enough before (and if one
*really* wanted to run gdb in a security hardened environment one
would use a secured VM or some such anyway ...), then why isn't
turning off auto-loading of Python scripts ahead of time also good
enough?

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 17:33         ` Doug Evans
@ 2012-01-17 17:55           ` Jan Kratochvil
  2012-01-17 18:29             ` Eli Zaretskii
  2012-01-17 18:31             ` Doug Evans
  2012-01-17 19:30           ` Matt Rice
  1 sibling, 2 replies; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 17:55 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

On Tue, 17 Jan 2012 18:26:49 +0100, Doug Evans wrote:
> I, OTOH, like the (top-gdb) prompt.

I think you and me can very easily do whatever we like with GDB.  I care more
about both newbie users of GDB and also about newcomers to GDB development.


> One could argue the converse too ... Having to manually request
> whatever goodies the developers find useful adds a needless burden.
> Why can't gdb just auto-adjust itself for debugging the program at
> hand?

Because it is then different than normal GDB.

It is difficult to argue myself but IMO in a survey between GDB newbies they
find easier if GDB behaves always the same than if it behaves differently
according to which program you load into it.


> There is the path that gdb searches for libthread_db, maybe you meant
> to refer to that;

Yes, I meant this part.


> one can change it before choosing the file to debug.   You seem to
> suggest that -nx was "good enough" before.  If that was good enough
> before (and that's a big "if"), why is changing the search path first
> not also "good enough".

Now instead of just -nx one has to use also "set auto-load-scripts off",
use -ex "file X" and -ex "core-file Y" instead of just X and Y to get that
"set auto-load-scripts off" executed first,
use beforehand -ex "set libthread-db-search-path /foo", OK, that may be enough
if I did not miss anything else.

I admit I did not know about "set auto-load-scripts off" myself until
recently.


> As for Python auto-loading, if -nx was good enough before (and if one
> *really* wanted to run gdb in a security hardened environment one
> would use a secured VM or some such anyway ...), then why isn't
> turning off auto-loading of Python scripts ahead of time also good
> enough?

Yes, it is, it is just too complicated, similiar to the extra VM.  I plan to
just merge the paragraph above into some user-accessible command-line option
"-safe".  I am just not sure how to load pretty printers from system libraries
which are safe but which do not get loaded after "set auto-load-scripts off".


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 16:34   ` Jan Kratochvil
  2012-01-17 16:48     ` Doug Evans
@ 2012-01-17 18:00     ` Eli Zaretskii
  2012-01-17 18:28       ` Jan Kratochvil
  1 sibling, 1 reply; 54+ messages in thread
From: Eli Zaretskii @ 2012-01-17 18:00 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: dje, gdb-patches

> Date: Tue, 17 Jan 2012 17:26:22 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> > >        .gdbinit:8: Error in sourced command file:
> > >        Argument required (one or more breakpoint numbers).
> > >         - What had happened?  Oh, I forgot -nx again!
> > 
> > This only affects gdb developers though.
> 
> It affects also gcc, gas, emacs, former Frysk, ReactOS, there will be more
> malicious .gdbinit files; not listed non-intrusive .gdbinit files which only
> define some new commands.

??? How's the .gdbinit file that comes with Emacs "malicious"?  I find
it indispensable, including during the past 2.5 years, when I needed
some heavy debugging of the Emacs display engine.

> <bite> When one wants to debug gdb in gdb the file gdb/.gdbinit is really the
> most destructive as all its various commands succeed making the debugging
> session completely unusable. </bite>

If that's really so (and I don't think I agree), then we should simply
stop distributing that file, not under the name .gdbinit anyway.

But using these use-cases, even if all of them indeed suffer from bad
.gdbinit files, to deprive users of having helpful init files in the
source tree is really excessive, IMO.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 18:00     ` Eli Zaretskii
@ 2012-01-17 18:28       ` Jan Kratochvil
  2012-01-17 18:43         ` Eli Zaretskii
  2012-01-17 20:29         ` Tom Tromey
  0 siblings, 2 replies; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 18:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: dje, gdb-patches

On Tue, 17 Jan 2012 18:54:57 +0100, Eli Zaretskii wrote:
> ??? How's the .gdbinit file that comes with Emacs "malicious"?

If one runs "gdb" one expects to get "gdb".

If one runs "gdb" in any of these directories (Emacs) one surprisingly gets
some weirdly behaving beast instead.


> I find it indispensable, including during the past 2.5 years, when I needed
> some heavy debugging of the Emacs display engine.

The problem is if one gets used to those .gdbinit files then sure those files
are great.  I do not think projects should be accessible only to their
longterm developers.  The longterm developers always can do some "gdb -x foo".


> If that's really so (and I don't think I agree), then we should simply
> stop distributing that file, not under the name .gdbinit anyway.
> 
> But using these use-cases, even if all of them indeed suffer from bad
> .gdbinit files, to deprive users of having helpful init files in the
> source tree is really excessive, IMO.

I agree if this change gets in then src/gdb/.gdbinit should be renamed to
something else.  For example src/gdb/gdbinit.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 17:55           ` Jan Kratochvil
@ 2012-01-17 18:29             ` Eli Zaretskii
  2012-01-17 18:35               ` Jan Kratochvil
  2012-01-17 18:31             ` Doug Evans
  1 sibling, 1 reply; 54+ messages in thread
From: Eli Zaretskii @ 2012-01-17 18:29 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: dje, gdb-patches

> Date: Tue, 17 Jan 2012 18:48:40 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> > One could argue the converse too ... Having to manually request
> > whatever goodies the developers find useful adds a needless burden.
> > Why can't gdb just auto-adjust itself for debugging the program at
> > hand?
> 
> Because it is then different than normal GDB.
> 
> It is difficult to argue myself but IMO in a survey between GDB newbies they
> find easier if GDB behaves always the same than if it behaves differently
> according to which program you load into it.

Newbies are not expected to debug GDB or Emacs.

Actually, it is impractical to debug Emacs without the .gdbinit whcih
comes with it.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 17:55           ` Jan Kratochvil
  2012-01-17 18:29             ` Eli Zaretskii
@ 2012-01-17 18:31             ` Doug Evans
  2012-01-17 18:44               ` Jan Kratochvil
  2012-01-17 19:55               ` Tom Tromey
  1 sibling, 2 replies; 54+ messages in thread
From: Doug Evans @ 2012-01-17 18:31 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Tue, Jan 17, 2012 at 9:48 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 17 Jan 2012 18:26:49 +0100, Doug Evans wrote:
>> I, OTOH, like the (top-gdb) prompt.
>
> I think you and me can very easily do whatever we like with GDB.  I care more
> about both newbie users of GDB and also about newcomers to GDB development.

As if to suggest I don't care ...
Ummm ... eh?

>> One could argue the converse too ... Having to manually request
>> whatever goodies the developers find useful adds a needless burden.
>> Why can't gdb just auto-adjust itself for debugging the program at
>> hand?
>
> Because it is then different than normal GDB.
>
> It is difficult to argue myself but IMO in a survey between GDB newbies they
> find easier if GDB behaves always the same than if it behaves differently
> according to which program you load into it.

Which survey is that?  Is it online?
[And I'm curious once they understand what's going on, what do they prefer.
Every new thing involves a bit of a learning curve ... I'd be curious
to know what the long term cost/benefit is for these newbies in
addition to just the short term ... Once they understand it, do they
prefer it?]

>> There is the path that gdb searches for libthread_db, maybe you meant
>> to refer to that;
>
> Yes, I meant this part.
>
>
>> one can change it before choosing the file to debug.   You seem to
>> suggest that -nx was "good enough" before.  If that was good enough
>> before (and that's a big "if"), why is changing the search path first
>> not also "good enough".
>
> Now instead of just -nx one has to use also "set auto-load-scripts off",
> use -ex "file X" and -ex "core-file Y" instead of just X and Y to get that
> "set auto-load-scripts off" executed first,
> use beforehand -ex "set libthread-db-search-path /foo", OK, that may be enough
> if I did not miss anything else.

Script it.

> I admit I did not know about "set auto-load-scripts off" myself until
> recently.
>
>
>> As for Python auto-loading, if -nx was good enough before (and if one
>> *really* wanted to run gdb in a security hardened environment one
>> would use a secured VM or some such anyway ...), then why isn't
>> turning off auto-loading of Python scripts ahead of time also good
>> enough?
>
> Yes, it is, it is just too complicated,

Too complicated how?
Write the script once and you're done.
If we had a contrib-like directory we could even ship one with gdb.

> similiar to the extra VM.  I plan to
> just merge the paragraph above into some user-accessible command-line option
> "-safe".  I am just not sure how to load pretty printers from system libraries
> which are safe but which do not get loaded after "set auto-load-scripts off".

Are we sure we want to claim to the user community -safe is, umm, safe?
It seems like we're a fair ways from being ready to claim it, setting
aside auto-loading.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 18:29             ` Eli Zaretskii
@ 2012-01-17 18:35               ` Jan Kratochvil
  2012-01-17 19:54                 ` Tom Tromey
  0 siblings, 1 reply; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 18:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: dje, gdb-patches

On Tue, 17 Jan 2012 19:27:42 +0100, Eli Zaretskii wrote:
> Newbies are not expected to debug GDB or Emacs.

And newbies can never write a kernel.</sarcasm>


Regards,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 18:28       ` Jan Kratochvil
@ 2012-01-17 18:43         ` Eli Zaretskii
  2012-01-17 19:02           ` Jan Kratochvil
  2012-01-17 20:29         ` Tom Tromey
  1 sibling, 1 reply; 54+ messages in thread
From: Eli Zaretskii @ 2012-01-17 18:43 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: dje, gdb-patches

> Date: Tue, 17 Jan 2012 18:59:57 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: dje@google.com, gdb-patches@sourceware.org
> 
> > I find it indispensable, including during the past 2.5 years, when I needed
> > some heavy debugging of the Emacs display engine.
> 
> The problem is if one gets used to those .gdbinit files then sure those files
> are great.  I do not think projects should be accessible only to their
> longterm developers.  The longterm developers always can do some "gdb -x foo".

These projects cannot be efficiently debugged without these files.
When did you last try looking at a long Lisp list with just the stock
commands?  It's impractical.  The first thing newbies should do is
learn at least some of the commands in .gdbinit, if they really want
to debug Emacs.

> > If that's really so (and I don't think I agree), then we should simply
> > stop distributing that file, not under the name .gdbinit anyway.
> > 
> > But using these use-cases, even if all of them indeed suffer from bad
> > .gdbinit files, to deprive users of having helpful init files in the
> > source tree is really excessive, IMO.
> 
> I agree if this change gets in then src/gdb/.gdbinit should be renamed to
> something else.  For example src/gdb/gdbinit.

If we are renaming the file, why make the change?

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 18:31             ` Doug Evans
@ 2012-01-17 18:44               ` Jan Kratochvil
  2012-01-17 19:12                 ` Doug Evans
  2012-01-17 19:55               ` Tom Tromey
  1 sibling, 1 reply; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 18:44 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

On Tue, 17 Jan 2012 19:29:04 +0100, Doug Evans wrote:
> > It is difficult to argue myself but IMO in a survey between GDB newbies they
> > find easier if GDB behaves always the same than if it behaves differently
> > according to which program you load into it.
> 
> Which survey is that?  Is it online?

Unfortunately I do not know about any.  I was just guessing results of
a hypothetical survey.  Sorry for being unclear.


> [And I'm curious once they understand what's going on, what do they prefer.
> Every new thing involves a bit of a learning curve ...

If anything requires a needless learning curve it will be changed.


> I'd be curious to know what the long term cost/benefit is for these newbies
> in addition to just the short term ... Once they understand it, do they
> prefer it?]

They do not need to understand it.  They just already use and develop other
debuggers.


> Script it.

If you prefer it in FSF GDB as a script I am can code it that way.


> Too complicated how?

I find

(a) Extract first and second argument in shell, that will be several lines of
    code.
(b) exec gdb -nx -x /etc/gdbinit -x ~/.gdbinit -ex "set auto-load-scripts off" -ex "set libthread-db-search-path" -ex "file $file -ex "core-file $corefile" "$@"

as more complicated than

gdb -secure "$@"

Don't you?


> Write the script once and you're done.
> If we had a contrib-like directory we could even ship one with gdb.

I have to ship it anyway so either Fedora + Red Hat will have to fork again or
it needs to be shipped with gdb.  It is a normal task of developers to analyze
shipped crashes/binaries.


> Are we sure we want to claim to the user community -safe is, umm, safe?
> It seems like we're a fair ways from being ready to claim it, setting
> aside auto-loading.

If we are not ready for -safe then we should not.

I am aware of DWARF reading unhandled run-offs but that is AFAIK only DoS
category of exploit.

Are you aware of any new exploits?  This Python/libthread_db is CVE-2011-4355.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 18:43         ` Eli Zaretskii
@ 2012-01-17 19:02           ` Jan Kratochvil
  0 siblings, 0 replies; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 19:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: dje, gdb-patches

On Tue, 17 Jan 2012 19:34:39 +0100, Eli Zaretskii wrote:
> If we are renaming the file, why make the change?

Because it is one of the multiple ways GDB is now exploitable and CVEs are
open for it.

Besides it is inconvenient but we do not agree on that part.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 18:44               ` Jan Kratochvil
@ 2012-01-17 19:12                 ` Doug Evans
  2012-01-17 19:20                   ` Jan Kratochvil
  0 siblings, 1 reply; 54+ messages in thread
From: Doug Evans @ 2012-01-17 19:12 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Tue, Jan 17, 2012 at 10:42 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 17 Jan 2012 19:29:04 +0100, Doug Evans wrote:
>> > It is difficult to argue myself but IMO in a survey between GDB newbies they
>> > find easier if GDB behaves always the same than if it behaves differently
>> > according to which program you load into it.
>>
>> Which survey is that?  Is it online?
>
> Unfortunately I do not know about any.  I was just guessing results of
> a hypothetical survey.  Sorry for being unclear.

I wouldn't want to make such a substantial change based on a guess.

>> [And I'm curious once they understand what's going on, what do they prefer.
>> Every new thing involves a bit of a learning curve ...
>
> If anything requires a needless learning curve it will be changed.

Depends on the result, inclusion of the word "needless" is a strawman argument.

>> I'd be curious to know what the long term cost/benefit is for these newbies
>> in addition to just the short term ... Once they understand it, do they
>> prefer it?]
>
> They do not need to understand it.  They just already use and develop other
> debuggers.
>
>
>> Script it.
>
> If you prefer it in FSF GDB as a script I am can code it that way.
>
>
>> Too complicated how?
>
> I find
>
> (a) Extract first and second argument in shell, that will be several lines of
>    code.
> (b) exec gdb -nx -x /etc/gdbinit -x ~/.gdbinit -ex "set auto-load-scripts off" -ex "set libthread-db-search-path" -ex "file $file -ex "core-file $corefile" "$@"
>
> as more complicated than
>
> gdb -secure "$@"
>
> Don't you?

As opposed to a script named, say, secure-gdb that did that?

>> Write the script once and you're done.
>> If we had a contrib-like directory we could even ship one with gdb.
>
> I have to ship it anyway so either Fedora + Red Hat will have to fork again or
> it needs to be shipped with gdb.  It is a normal task of developers to analyze
> shipped crashes/binaries.

Maintenance of pure additions is far easier than maintenance of local
mods that involve changes.
And maintenance of pure additions that are simply new files is easier than that.
IOW, if Fedora had to ship a script that wasn't in FSF GDB, is it
really that big a deal?
[OTOH, I for one, wish we had a contrib-like directory.]

>> Are we sure we want to claim to the user community -safe is, umm, safe?
>> It seems like we're a fair ways from being ready to claim it, setting
>> aside auto-loading.
>
> If we are not ready for -safe then we should not.
>
> I am aware of DWARF reading unhandled run-offs but that is AFAIK only DoS
> category of exploit.
>
> Are you aware of any new exploits?  This Python/libthread_db is CVE-2011-4355.

My point is a security audit of GDB is more than just fixing the bugs
we know of.
As is taking on the job of keeping it that way.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 19:12                 ` Doug Evans
@ 2012-01-17 19:20                   ` Jan Kratochvil
  0 siblings, 0 replies; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 19:20 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

On Tue, 17 Jan 2012 20:02:21 +0100, Doug Evans wrote:
> I wouldn't want to make such a substantial change based on a guess.

OK, I can post an online survey to <devel at lists.fedoraproject.org> or if
you have any other idea.


> > (a) Extract first and second argument in shell, that will be several lines of
> >    code.
[...]
> As opposed to a script named, say, secure-gdb that did that?

The problem is that script needs to parse out properly the arguments
(executable/core/PID) including properly recognizing --args, which is probably
doable but I find it a bit fragile and definitely not easy.


> Maintenance of pure additions is far easier than maintenance of local
> mods that involve changes.

It will become difficult wrt maintenance of GDB options parsing compatibility.


But I accept it if you really oppose a new GDB option.


> My point is a security audit of GDB is more than just fixing the bugs
> we know of.
> As is taking on the job of keeping it that way.

I agree but there are no assigned resources for it.

And I find better to ship program with no known security flaws (*) than to
ship it with known security flaws.  And after all I cannot choose the second
option anyway.

(*) I believe there possibly may not be any.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 17:33         ` Doug Evans
  2012-01-17 17:55           ` Jan Kratochvil
@ 2012-01-17 19:30           ` Matt Rice
  2012-01-17 19:37             ` Jan Kratochvil
  2012-01-17 20:26             ` Tom Tromey
  1 sibling, 2 replies; 54+ messages in thread
From: Matt Rice @ 2012-01-17 19:30 UTC (permalink / raw)
  To: Doug Evans; +Cc: Jan Kratochvil, gdb-patches

On Tue, Jan 17, 2012 at 9:26 AM, Doug Evans <dje@google.com> wrote:

>
> I, OTOH, like the (top-gdb) prompt.

FWIW I found it insufficient, and avoid the gdb/.gdbinit so rewrote it
in python so i get

(gdb)
(^gdb)
(^^gdb)
with each prompt being a different color, and absent the tethering to
the gdb/ dir like .gdbinit is.

the last being a limitation of the .gdbinit mechanism.  I'll have to
clean it up a bit before posting it.  Anyhow i've found it a far more
enjoyable experience than the plain old top-gdb prompt which i'd
always notice only after hitting enter.

sorry, if it is off-topic

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 19:30           ` Matt Rice
@ 2012-01-17 19:37             ` Jan Kratochvil
  2012-01-17 20:26             ` Tom Tromey
  1 sibling, 0 replies; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 19:37 UTC (permalink / raw)
  To: Matt Rice; +Cc: Doug Evans, gdb-patches

On Tue, 17 Jan 2012 20:20:09 +0100, Matt Rice wrote:
> On Tue, Jan 17, 2012 at 9:26 AM, Doug Evans <dje@google.com> wrote:
> > I, OTOH, like the (top-gdb) prompt.
> 
> FWIW I found it insufficient, and avoid the gdb/.gdbinit so rewrote it
> in python so i get

BTW not so recently but in some years ago GDB usually crashed (=problem B)
while I was debugging GDB for problem A.  So I had commonly running then three
GDBs in row (to have the problem B reproducible), so "top-gdb" would not be
sufficient (if I used it) anyway.


Regards,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 18:35               ` Jan Kratochvil
@ 2012-01-17 19:54                 ` Tom Tromey
  0 siblings, 0 replies; 54+ messages in thread
From: Tom Tromey @ 2012-01-17 19:54 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Eli Zaretskii, dje, gdb-patches

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> On Tue, 17 Jan 2012 19:27:42 +0100, Eli Zaretskii wrote:
>> Newbies are not expected to debug GDB or Emacs.

Jan> And newbies can never write a kernel.</sarcasm>

Please, no sarcasm.

Tom

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 18:31             ` Doug Evans
  2012-01-17 18:44               ` Jan Kratochvil
@ 2012-01-17 19:55               ` Tom Tromey
  2012-01-17 20:24                 ` Pedro Alves
  1 sibling, 1 reply; 54+ messages in thread
From: Tom Tromey @ 2012-01-17 19:55 UTC (permalink / raw)
  To: Doug Evans; +Cc: Jan Kratochvil, gdb-patches

Jan> similiar to the extra VM.  I plan to just merge the paragraph above
Jan> into some user-accessible command-line option "-safe".  I am just
Jan> not sure how to load pretty printers from system libraries which
Jan> are safe but which do not get loaded after "set auto-load-scripts
Jan> off".

Doug> Are we sure we want to claim to the user community -safe is, umm, safe?
Doug> It seems like we're a fair ways from being ready to claim it, setting
Doug> aside auto-loading.

It seems to me that having the option would give us some impetus toward
fixing the problems, if we can actually make them occur; and furthermore
the option gives us a framework in which to fix all such problems,
namely, check the flag.

Tom

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 10:16 [patch] New set auto-load-local-gdbinit + disable it by default Jan Kratochvil
                   ` (2 preceding siblings ...)
  2012-01-17 16:26 ` Matt Rice
@ 2012-01-17 20:09 ` Tom Tromey
  2012-01-24  0:33 ` Stan Shebs
  4 siblings, 0 replies; 54+ messages in thread
From: Tom Tromey @ 2012-01-17 20:09 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> +* GDB no longer reads .gdbinit file from current directory by default.
Jan> +  Use "gdb -x .gdbinit" to retain the original behavior.

I think the NEWS entry should also mention the new parameter.
This is the simplest way for people to get back the old behavior;
and anyway new parameters should always be mentioned in NEWS.

Tom

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 12:34 ` Eli Zaretskii
  2012-01-17 13:42   ` Joel Brobecker
@ 2012-01-17 20:22   ` Tom Tromey
  1 sibling, 0 replies; 54+ messages in thread
From: Tom Tromey @ 2012-01-17 20:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Jan Kratochvil, gdb-patches

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Jan> Still at least the setting should go in and then one can then have
Jan> "set auto-load-local-gdbinit off" at least in ~/.gdbinit.  Anyway I
Jan> would file a FESCo (Fedora Engineering Steering Committee) ticket
Jan> for such "off" in /etc/gdbinit at least in distro and IMHO it needs
Jan> to get approved (but maybe not, it would be another fork from
Jan> upstream).

Eli> I think this is a draconian measure.  It prevents me from having a
Eli> .gdbinit file loaded automatically as appropriate for a program I'm
Eli> debugging.  Prominent examples include GDB itself and Emacs, which
Eli> both come with a .gdbinit file that makes debugging much easier.

Jan> And "gdb -x ./.gdbinit" is a pretty simple way to do what one wants to do.

Eli> "gdb -x .gdbinit" is much longer than just "gdb".

You can also put the setting into your ~/.gdbinit and get the old
behavior back.

I read through this thread and, while I agree that this change is
inconvenient, I think the inconvenience is outweighed by the security
implications.  I consider this to be similar to putting "." in PATH.
We're just lucky that it hasn't been exploited yet (or at least that we
haven't gotten the blame).

Well, I "agree" with it -- I am not happy about it by a long stretch.
I'm tempted by the idea that gdb should be insecure by default and we
should require "-safe"... but on balance this seems irresponsible to me,
as I think that tools should generally be secure by default.

FWIW, I think this patch doesn't go far enough :-(
Some other topics for consideration; perhaps the new option could cover
all such cases.

* Auto-loading of Python code from the filesystem.
  Perhaps we could have a set of trusted directories; but I wonder if
  there are issues with sysroots starting with "remote:".

* Auto-loading of Python code from objfiles.
  Even more dangerous.

* The JIT interface.
  I'm not sure whether this can be exploited or not.

FWIW, I will probably set the new parameter in my own .gdbinit and use
"-safe" in those rare instances that I do something other than debug
programs that I built myself.

Tom

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 19:55               ` Tom Tromey
@ 2012-01-17 20:24                 ` Pedro Alves
  2012-01-17 20:26                   ` Tom Tromey
  2012-01-17 20:35                   ` Jan Kratochvil
  0 siblings, 2 replies; 54+ messages in thread
From: Pedro Alves @ 2012-01-17 20:24 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Doug Evans, Jan Kratochvil, gdb-patches

On 01/17/2012 07:54 PM, Tom Tromey wrote:
> Jan> similiar to the extra VM.  I plan to just merge the paragraph above
> Jan> into some user-accessible command-line option "-safe".  I am just
> Jan> not sure how to load pretty printers from system libraries which
> Jan> are safe but which do not get loaded after "set auto-load-scripts
> Jan> off".
> 
> Doug> Are we sure we want to claim to the user community -safe is, umm, safe?
> Doug> It seems like we're a fair ways from being ready to claim it, setting
> Doug> aside auto-loading.
> 
> It seems to me that having the option would give us some impetus toward
> fixing the problems, if we can actually make them occur; and furthermore
> the option gives us a framework in which to fix all such problems,
> namely, check the flag.

Having to request a safe mode with an extra flag is a flawed design, IMO.
New, and non-aware-of-the-issues-implied (read, most) users, will just
not know about it, and will therefore not use it.  IMO, if we'll have
a flag for something like this, its better to have a flag that does
the opposite, one that enables unsafe functionality.  This also makes it
so that veteran users that trip on missing functionality they always
knew about, learn about the flags/options as consequence when they go
look for the alternatives.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 19:30           ` Matt Rice
  2012-01-17 19:37             ` Jan Kratochvil
@ 2012-01-17 20:26             ` Tom Tromey
  1 sibling, 0 replies; 54+ messages in thread
From: Tom Tromey @ 2012-01-17 20:26 UTC (permalink / raw)
  To: Matt Rice; +Cc: Doug Evans, Jan Kratochvil, gdb-patches

>>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:

Matt> FWIW I found it insufficient, and avoid the gdb/.gdbinit so rewrote it
Matt> in python so i get

Matt> (gdb)
Matt> (^gdb)
Matt> (^^gdb)
Matt> with each prompt being a different color, and absent the tethering to
Matt> the gdb/ dir like .gdbinit is.

Nice idea.
I've occasionally found the 2-level approach insufficient.

Tom

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 20:24                 ` Pedro Alves
@ 2012-01-17 20:26                   ` Tom Tromey
  2012-01-17 20:35                   ` Jan Kratochvil
  1 sibling, 0 replies; 54+ messages in thread
From: Tom Tromey @ 2012-01-17 20:26 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Doug Evans, Jan Kratochvil, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Having to request a safe mode with an extra flag is a flawed design, IMO.
Pedro> New, and non-aware-of-the-issues-implied (read, most) users, will just
Pedro> not know about it, and will therefore not use it.  IMO, if we'll have
Pedro> a flag for something like this, its better to have a flag that does
Pedro> the opposite, one that enables unsafe functionality.  This also makes it
Pedro> so that veteran users that trip on missing functionality they always
Pedro> knew about, learn about the flags/options as consequence when they go
Pedro> look for the alternatives.

Yeah, I agree; though I think safe mode should be the default and the
"-safe" command-line argument should only serve to disable
auto-load-local-gdbinit (or whatever the option will be called).

That way I can have auto-load-local-gdbinit by default for myself, but
also easily override it on the command line.

Tom

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 18:28       ` Jan Kratochvil
  2012-01-17 18:43         ` Eli Zaretskii
@ 2012-01-17 20:29         ` Tom Tromey
  2012-01-17 20:49           ` Jan Kratochvil
  2012-01-17 21:13           ` Eli Zaretskii
  1 sibling, 2 replies; 54+ messages in thread
From: Tom Tromey @ 2012-01-17 20:29 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Eli Zaretskii, dje, gdb-patches

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> On Tue, 17 Jan 2012 18:54:57 +0100, Eli Zaretskii wrote:
>> ??? How's the .gdbinit file that comes with Emacs "malicious"?

Jan> If one runs "gdb" one expects to get "gdb".

Jan> If one runs "gdb" in any of these directories (Emacs) one surprisingly gets
Jan> some weirdly behaving beast instead.

I'm not convinced by this line of argument.

I think gdb has touted this -- rightly -- as a feature.  And, if
anything, I think we've gone even more in this direction in recent
years, what with pretty-printers and Python auto-loading.  Furthermore,
I think this is a good trend in general; applications are getting more
complex, and this provides an important way to adapt the debugger to
them.

That said, I agree with your conclusions for other reasons.

Tom

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 20:24                 ` Pedro Alves
  2012-01-17 20:26                   ` Tom Tromey
@ 2012-01-17 20:35                   ` Jan Kratochvil
  2012-01-17 20:56                     ` Pedro Alves
  1 sibling, 1 reply; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 20:35 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, Doug Evans, gdb-patches

On Tue, 17 Jan 2012 21:21:42 +0100, Pedro Alves wrote:
> Having to request a safe mode with an extra flag is a flawed design, IMO.
> New, and non-aware-of-the-issues-implied (read, most) users, will just
> not know about it, and will therefore not use it.

I agree with it.  I am thinking about it for more days already.  But I have
not found a way out.

The problem is to differentiate these two cases:
(a) as regular user:
	$ tar xzf foreign-crash.tar.gz; cd foreign-crash
	$ gdb crashprog crashcore # *-gdb.py files lay here around
	 - You are 0wn3d!
(b) as regular user:
	$ cd my/great/project; make
	# gdb myprog-using-local-new-gdb.py-pretty-printers

If we want to fix (a) by default we have to regress (b) in the default case.

The second question would be which directories are safe and which are not but
that can be solved by some configuration variable and site-adjustable value.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 20:29         ` Tom Tromey
@ 2012-01-17 20:49           ` Jan Kratochvil
  2012-01-17 20:54             ` Doug Evans
  2012-01-17 21:10             ` Tom Tromey
  2012-01-17 21:13           ` Eli Zaretskii
  1 sibling, 2 replies; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-17 20:49 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Eli Zaretskii, dje, gdb-patches

On Tue, 17 Jan 2012 21:28:29 +0100, Tom Tromey wrote:
> I think gdb has touted this -- rightly -- as a feature.  And, if
> anything, I think we've gone even more in this direction in recent
> years, what with pretty-printers and Python auto-loading.  Furthermore,
> I think this is a good trend in general; applications are getting more
> complex, and this provides an important way to adapt the debugger to
> them.

In such case the `canned sequences of commands' should be bound to the binary,
not to the directory.  One (me) runs GDB for so many different executablse
while staying in src/gdb/ .


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 20:49           ` Jan Kratochvil
@ 2012-01-17 20:54             ` Doug Evans
  2012-01-17 21:10             ` Tom Tromey
  1 sibling, 0 replies; 54+ messages in thread
From: Doug Evans @ 2012-01-17 20:54 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Tom Tromey, Eli Zaretskii, gdb-patches

On Tue, Jan 17, 2012 at 12:35 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 17 Jan 2012 21:28:29 +0100, Tom Tromey wrote:
>> I think gdb has touted this -- rightly -- as a feature.  And, if
>> anything, I think we've gone even more in this direction in recent
>> years, what with pretty-printers and Python auto-loading.  Furthermore,
>> I think this is a good trend in general; applications are getting more
>> complex, and this provides an important way to adapt the debugger to
>> them.
>
> In such case the `canned sequences of commands' should be bound to the binary,
> not to the directory.  One (me) runs GDB for so many different executablse
> while staying in src/gdb/ .

Or in binutils, for example, there are lots of different binaries,
though they share a lot in common.

For reference sake, we already have -gdb.py, which is looked for when
the objfile is loaded.

echo "print 'Hi there.'" > gdb-gdb.py && gdb -nx gdb

One could have objdump-gdb.py and readelf-gdb.py, and then if I change
binaries mid-session, the new one will get loaded, which can't happen
with .gdbinit.
[And to what extent vestiges of the old one get discarded depends on
what was done.  I can imagine wanting to be able to keep more state
per-main-objfile.
I could be simultaneously debugging gdb and readelf, and not want to
see (top-gdb) (or (^gdb) or whatever) when I switch to readelf. :-)

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 20:35                   ` Jan Kratochvil
@ 2012-01-17 20:56                     ` Pedro Alves
  0 siblings, 0 replies; 54+ messages in thread
From: Pedro Alves @ 2012-01-17 20:56 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Tom Tromey, Doug Evans, gdb-patches

On 01/17/2012 08:32 PM, Jan Kratochvil wrote:
> On Tue, 17 Jan 2012 21:21:42 +0100, Pedro Alves wrote:
>> Having to request a safe mode with an extra flag is a flawed design, IMO.
>> New, and non-aware-of-the-issues-implied (read, most) users, will just
>> not know about it, and will therefore not use it.
> 
> I agree with it.  I am thinking about it for more days already.  But I have
> not found a way out.
> 
> The problem is to differentiate these two cases:
> (a) as regular user:
> 	$ tar xzf foreign-crash.tar.gz; cd foreign-crash
> 	$ gdb crashprog crashcore # *-gdb.py files lay here around
> 	 - You are 0wn3d!

Make gdb warn about files that could be autoloaded, but don't load them:

 	$ gdb crashprog crashcore # *-gdb.py files lay here around
        security warning: found possibly unsafe files that could be autoloaded
          foo-gdb.py
          bar-gdb.py
        Start gdb with the -superman option to load them automatically.
        *debug core as usual*
        (gdb)

There's probably a better spelling for that option...  System/package/distro
.py files (printers, and stuff) would be trusted.  The mechanism for that
could be path list based.

> (b) as regular user:
> 	$ cd my/great/project; make
> 	# gdb myprog-using-local-new-gdb.py-pretty-printers

 	$ cd my/great/project; make
 	# gdb -trustothers myprog-using-local-new-gdb.py-pretty-printers

Or the equivalent in ~/.gdbinit -- trustothers-ness possibly enabled with python
(checking for example, if the executable is under a /home/me/myprojects/
directory, or whatever other policy one prefers).  If you forget to specify
the flag, gdb will warn you about the files that could be loaded.

The question is: is this too much inconvenience?  I am willing to give it
a try for a while.

> 
> If we want to fix (a) by default we have to regress (b) in the default case.
> 
> The second question would be which directories are safe and which are not but
> that can be solved by some configuration variable and site-adjustable value.

Yes, agreed.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 20:49           ` Jan Kratochvil
  2012-01-17 20:54             ` Doug Evans
@ 2012-01-17 21:10             ` Tom Tromey
  1 sibling, 0 replies; 54+ messages in thread
From: Tom Tromey @ 2012-01-17 21:10 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Eli Zaretskii, dje, gdb-patches

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Tom> I think gdb has touted this -- rightly -- as a feature.  And, if
Tom> anything, I think we've gone even more in this direction in recent
Tom> years, what with pretty-printers and Python auto-loading.  Furthermore,
Tom> I think this is a good trend in general; applications are getting more
Tom> complex, and this provides an important way to adapt the debugger to
Tom> them.

Jan> In such case the `canned sequences of commands' should be bound to
Jan> the binary, not to the directory.  One (me) runs GDB for so many
Jan> different executablse while staying in src/gdb/ .

Yeah, well...

It is fixable if you think it is important enough.
I personally just use -nx in these cases.

Tom

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 20:29         ` Tom Tromey
  2012-01-17 20:49           ` Jan Kratochvil
@ 2012-01-17 21:13           ` Eli Zaretskii
  2012-01-17 21:14             ` Tom Tromey
  2012-01-17 22:16             ` Doug Evans
  1 sibling, 2 replies; 54+ messages in thread
From: Eli Zaretskii @ 2012-01-17 21:13 UTC (permalink / raw)
  To: Tom Tromey; +Cc: jan.kratochvil, dje, gdb-patches

> From: Tom Tromey <tromey@redhat.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, dje@google.com, gdb-patches@sourceware.org
> Date: Tue, 17 Jan 2012 13:28:29 -0700
> 
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> On Tue, 17 Jan 2012 18:54:57 +0100, Eli Zaretskii wrote:
> >> ??? How's the .gdbinit file that comes with Emacs "malicious"?
> 
> Jan> If one runs "gdb" one expects to get "gdb".
> 
> Jan> If one runs "gdb" in any of these directories (Emacs) one surprisingly gets
> Jan> some weirdly behaving beast instead.
> 
> I'm not convinced by this line of argument.
> 
> I think gdb has touted this -- rightly -- as a feature.  And, if
> anything, I think we've gone even more in this direction in recent
> years, what with pretty-printers and Python auto-loading.  Furthermore,
> I think this is a good trend in general; applications are getting more
> complex, and this provides an important way to adapt the debugger to
> them.
> 
> That said, I agree with your conclusions for other reasons.

If those other reasons are safety, then this is not the way to do it.
Legitimate uses of .gdbinit of many should not be precluded on behalf
of potential wrong-doing of the few.  If safety is our concern, we
should make unsafe uses harder or impossible without hurting safe
uses.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 21:13           ` Eli Zaretskii
@ 2012-01-17 21:14             ` Tom Tromey
  2012-01-18  4:26               ` Joel Brobecker
  2012-01-17 22:16             ` Doug Evans
  1 sibling, 1 reply; 54+ messages in thread
From: Tom Tromey @ 2012-01-17 21:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: jan.kratochvil, dje, gdb-patches

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> If those other reasons are safety, then this is not the way to do it.
Eli> Legitimate uses of .gdbinit of many should not be precluded on behalf
Eli> of potential wrong-doing of the few.  If safety is our concern, we
Eli> should make unsafe uses harder or impossible without hurting safe
Eli> uses.

The problem is that nobody has thought of a way to do this; Jan sent a
message in this thread that concisely explains the two cases that
conflict.  I, and I assume others in this thread, am proceeding from the
basis that there is no ideal solution and so we must choose between
lesser approaches.

Tom

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 21:13           ` Eli Zaretskii
  2012-01-17 21:14             ` Tom Tromey
@ 2012-01-17 22:16             ` Doug Evans
  2012-01-18  3:05               ` Tom Tromey
  1 sibling, 1 reply; 54+ messages in thread
From: Doug Evans @ 2012-01-17 22:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Tom Tromey, jan.kratochvil, gdb-patches

On Tue, Jan 17, 2012 at 1:10 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Tom Tromey <tromey@redhat.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>, dje@google.com, gdb-patches@sourceware.org
>> Date: Tue, 17 Jan 2012 13:28:29 -0700
>>
>> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
>>
>> Jan> On Tue, 17 Jan 2012 18:54:57 +0100, Eli Zaretskii wrote:
>> >> ??? How's the .gdbinit file that comes with Emacs "malicious"?
>>
>> Jan> If one runs "gdb" one expects to get "gdb".
>>
>> Jan> If one runs "gdb" in any of these directories (Emacs) one surprisingly gets
>> Jan> some weirdly behaving beast instead.
>>
>> I'm not convinced by this line of argument.
>>
>> I think gdb has touted this -- rightly -- as a feature.  And, if
>> anything, I think we've gone even more in this direction in recent
>> years, what with pretty-printers and Python auto-loading.  Furthermore,
>> I think this is a good trend in general; applications are getting more
>> complex, and this provides an important way to adapt the debugger to
>> them.
>>
>> That said, I agree with your conclusions for other reasons.
>
> If those other reasons are safety, then this is not the way to do it.
> Legitimate uses of .gdbinit of many should not be precluded on behalf
> of potential wrong-doing of the few.  If safety is our concern, we
> should make unsafe uses harder or impossible without hurting safe
> uses.

Maybe we need something like "safe tcl" for our scripting languages.
[is there a safe python?]
1/2 :-)


/me ducks!

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 22:16             ` Doug Evans
@ 2012-01-18  3:05               ` Tom Tromey
  0 siblings, 0 replies; 54+ messages in thread
From: Tom Tromey @ 2012-01-18  3:05 UTC (permalink / raw)
  To: Doug Evans; +Cc: Eli Zaretskii, jan.kratochvil, gdb-patches

Doug> Maybe we need something like "safe tcl" for our scripting languages.
Doug> [is there a safe python?]
Doug> 1/2 :-)

Doug> /me ducks!

I think this is a good idea, but a quick look suggests that there is
nothing really usable here :(

An ok source seems to be http://wiki.python.org/moin/SandboxedPython

Tom

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 21:14             ` Tom Tromey
@ 2012-01-18  4:26               ` Joel Brobecker
  2012-01-18 19:38                 ` Jan Kratochvil
  2012-01-19 21:06                 ` Tom Tromey
  0 siblings, 2 replies; 54+ messages in thread
From: Joel Brobecker @ 2012-01-18  4:26 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Eli Zaretskii, jan.kratochvil, dje, gdb-patches

It's become really hard, at least for me, to focus on this discussion.
We started from discussing about the local .gdbinit file to everything
auto-loaded is unsafe. I see why the latter was discussed, and how it
was useful, so this is not a criticism.

After having read everyone's emails so far, my stance on this is:

  . About reading the .gdbinit file in the current directory:
    It's a question of how seriously the security weakness should be
    taken. I confess I have a hard time taking them seriously, but
    I know I am probably too naive.  This is a feature that I could
    personally live without, and therefore will not oppose its removal.

    I would like to propose the following, however, to help the users
    who want to continue relying on it. I am happy to implement it
    if necessary:

        Provide a new command that would read the .gdbinit file in
        the current working directory if present, and do nothing
        otherwise.  I would like to provide options that select
        between loading silently, and loading with a warning first,
        and why not, asking before loading.

        The idea is that the user who would like to preserve
        the old behavior can put that command in his $HOME/.gdbinit
        file.

  . To me, it is extremely important that system-gdbinit is still
    automatically loaded. The system gdbinit file is there to help
    the user setup his debugging session. It should be considered
    as trusted, and I oppose a change that would stop is automatic
    loading. The language is strong, but it does not mean that
    I have veto right - so if I am outvoted, so be it.

  . About the auto-loading of Python code: I think that the cost
    of removing the auto-loading, even if it is only for non-trusted
    directories, would be too high. I would prefer if it discussed
    this separately after the .gdbinit issue has been resolved.

-- 
Joel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-18  4:26               ` Joel Brobecker
@ 2012-01-18 19:38                 ` Jan Kratochvil
  2012-01-18 20:01                   ` Doug Evans
                                     ` (2 more replies)
  2012-01-19 21:06                 ` Tom Tromey
  1 sibling, 3 replies; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-18 19:38 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Tom Tromey, Eli Zaretskii, dje, gdb-patches

On Wed, 18 Jan 2012 04:59:27 +0100, Joel Brobecker wrote:
>     I would like to propose the following, however, to help the users
>     who want to continue relying on it. I am happy to implement it
>     if necessary:
> 
>         Provide a new command that would read the .gdbinit file in
>         the current working directory if present, and do nothing
>         otherwise.  I would like to provide options that select
>         between loading silently, and loading with a warning first,
>         and why not, asking before loading.
> 
>         The idea is that the user who would like to preserve
>         the old behavior can put that command in his $HOME/.gdbinit
>         file.

The goal was you can do either
	echo 'set auto-load-local-gdbinit on' >>~/.gdbinit
or
	echo 'set auto-load-local-gdbinit on' >>/etc/gdbinit
and you get the former behavior.

Did you still prefer the command you propose?


>   . To me, it is extremely important that system-gdbinit is still
>     automatically loaded.

This patch/discussion affect neither /etc/gdbinit nor ~/.gdbinit in any way.


>     It should be considered as trusted,

Yes, they are.


>   . About the auto-loading of Python code: I think that the cost
>     of removing the auto-loading, even if it is only for non-trusted
>     directories, would be too high. I would prefer if it discussed
>     this separately after the .gdbinit issue has been resolved.

After the discussion my current plan is:
 * change .gdbinit to PROGRAM-gdb.rc (like -gdb.py),
   rename FSF GDB src/gdb/.gdbinit to src/gdb/gdb-gdb.rc,
   delete FSF GDB src/testsuite/.gdbinit ("set height 400" only, why?)
   This will fix the annoying problems of running gdb for other programs in
   the src/gdb/ directory.
 * deprecate .gdbinit
 * propose less intrusive gdb-gdb.rc, this sure affects only GDB developers,
   like do not change the outer prompt to "(top-gdb)"; I do not mind much.
 * in a second patch series deal with security, propose some "-safe" option,
   PROGRAM-gdb.rc is more visible than .gdbinit, it has the same security
   issue like PROGRAM-gdb.py and we have to deal with -gdb.py already anyway.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-18 19:38                 ` Jan Kratochvil
@ 2012-01-18 20:01                   ` Doug Evans
  2012-01-19  6:30                   ` Joel Brobecker
  2012-01-19 21:07                   ` [patch] New set auto-load-local-gdbinit + disable it by default Tom Tromey
  2 siblings, 0 replies; 54+ messages in thread
From: Doug Evans @ 2012-01-18 20:01 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Joel Brobecker, Tom Tromey, Eli Zaretskii, gdb-patches

On Wed, Jan 18, 2012 at 11:18 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> After the discussion my current plan is:
>  * change .gdbinit to PROGRAM-gdb.rc (like -gdb.py),
>   rename FSF GDB src/gdb/.gdbinit to src/gdb/gdb-gdb.rc,

For reference sake, I think you mean $obj/gdb/.gdbinit.
src/gdb/.gdbinit does not exist, it's gdbinit.in.

Plus I think you need to specify the semantics more:
e.g.,
When will -gdb.rc files get loaded?
Will gdb look for -gdb.rc files precisely when it looks for -gdb.py files?
What happens if I do "gdb foo ; file bar ; file baz" ?

[and remember we'll also have to handle -gdb.scm at some point.  1/2 :-)]

>   delete FSF GDB src/testsuite/.gdbinit ("set height 400" only, why?)
>   This will fix the annoying problems of running gdb for other programs in
>   the src/gdb/ directory.
>  * deprecate .gdbinit
>  * propose less intrusive gdb-gdb.rc, this sure affects only GDB developers,
>   like do not change the outer prompt to "(top-gdb)"; I do not mind much.

Please do not remove (top-gdb).
Rename it to ^gdb, ^^gdb, etc. if you must.

[the few times you have to debug gdb under gdb under gdb is not
persuasive to change it IMO]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-18 19:38                 ` Jan Kratochvil
  2012-01-18 20:01                   ` Doug Evans
@ 2012-01-19  6:30                   ` Joel Brobecker
  2012-01-19 12:57                     ` [commit] rm gdb/testsuite/.gdbinit [Re: [patch] New set auto-load-local-gdbinit + disable it by default] Jan Kratochvil
  2012-01-19 21:07                   ` [patch] New set auto-load-local-gdbinit + disable it by default Tom Tromey
  2 siblings, 1 reply; 54+ messages in thread
From: Joel Brobecker @ 2012-01-19  6:30 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Tom Tromey, Eli Zaretskii, dje, gdb-patches

> The goal was you can do either
> 	echo 'set auto-load-local-gdbinit on' >>~/.gdbinit
> or
> 	echo 'set auto-load-local-gdbinit on' >>/etc/gdbinit
> and you get the former behavior.
> 
> Did you still prefer the command you propose?

No, that would be fine too. I wasn't sure that this would work or not.

> >   . To me, it is extremely important that system-gdbinit is still
> >     automatically loaded.
> 
> This patch/discussion affect neither /etc/gdbinit nor ~/.gdbinit in any way.

Good.

> After the discussion my current plan is:
>  * change .gdbinit to PROGRAM-gdb.rc (like -gdb.py),
>    rename FSF GDB src/gdb/.gdbinit to src/gdb/gdb-gdb.rc,
>    delete FSF GDB src/testsuite/.gdbinit ("set height 400" only, why?)
>    This will fix the annoying problems of running gdb for other programs in
>    the src/gdb/ directory.

I think you can delete the .gdbinit in the testsuite directory
right now. I cannot imagine anyway would really depend on it.

The plan of making the .gdbinit file more visible seems like
a good idea.

-- 
Joel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [commit] rm gdb/testsuite/.gdbinit  [Re: [patch] New set auto-load-local-gdbinit + disable it by default]
  2012-01-19  6:30                   ` Joel Brobecker
@ 2012-01-19 12:57                     ` Jan Kratochvil
  0 siblings, 0 replies; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-19 12:57 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Tom Tromey, Eli Zaretskii, dje, gdb-patches

On Thu, 19 Jan 2012 06:52:26 +0100, Joel Brobecker wrote:
> I think you can delete the .gdbinit in the testsuite directory
> right now. I cannot imagine anyway would really depend on it.

Checked in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2012-01/msg00160.html

--- src/gdb/testsuite/ChangeLog	2012/01/19 12:37:28	1.3028
+++ src/gdb/testsuite/ChangeLog	2012/01/19 12:44:01	1.3029
@@ -1,3 +1,7 @@
+2012-01-19  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* .gdbinit: Remove.
+
 2012-01-19  Pedro Alves  <palves@redhat.com>
 
 	* gdb.java/jprint.exp: Don't rely on inferior output, but instead
--- src/gdb/testsuite/.gdbinit	1999-04-16 03:34:28.000000000 +0200
+++ /dev/null	2012-01-07 16:59:32.218401923 +0100
@@ -1 +0,0 @@
-set height 400

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-18  4:26               ` Joel Brobecker
  2012-01-18 19:38                 ` Jan Kratochvil
@ 2012-01-19 21:06                 ` Tom Tromey
  1 sibling, 0 replies; 54+ messages in thread
From: Tom Tromey @ 2012-01-19 21:06 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Eli Zaretskii, jan.kratochvil, dje, gdb-patches

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel>   . About the auto-loading of Python code: I think that the cost
Joel>     of removing the auto-loading, even if it is only for non-trusted
Joel>     directories, would be too high. I would prefer if it discussed
Joel>     this separately after the .gdbinit issue has been resolved.

The reason I think the issues belong together is that they are really
both instances of the same bug: gdb running possibly-untrustworthy code.

I don't think it makes sense to fix one such hole but leave another one
wide open.

Tom

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-18 19:38                 ` Jan Kratochvil
  2012-01-18 20:01                   ` Doug Evans
  2012-01-19  6:30                   ` Joel Brobecker
@ 2012-01-19 21:07                   ` Tom Tromey
  2012-01-19 21:47                     ` Jan Kratochvil
  2 siblings, 1 reply; 54+ messages in thread
From: Tom Tromey @ 2012-01-19 21:07 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Joel Brobecker, Eli Zaretskii, dje, gdb-patches

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> After the discussion my current plan is:
Jan>  * change .gdbinit to PROGRAM-gdb.rc (like -gdb.py),
Jan>    rename FSF GDB src/gdb/.gdbinit to src/gdb/gdb-gdb.rc,
Jan>    delete FSF GDB src/testsuite/.gdbinit ("set height 400" only, why?)
Jan>    This will fix the annoying problems of running gdb for other programs in
Jan>    the src/gdb/ directory.
Jan>  * deprecate .gdbinit
Jan>  * propose less intrusive gdb-gdb.rc, this sure affects only GDB developers,
Jan>    like do not change the outer prompt to "(top-gdb)"; I do not mind much.
Jan>  * in a second patch series deal with security, propose some "-safe" option,
Jan>    PROGRAM-gdb.rc is more visible than .gdbinit, it has the same security
Jan>    issue like PROGRAM-gdb.py and we have to deal with -gdb.py already anyway.

Would you mind very much separating the different patches?

I am sold on the security changes, but I'm not as convinced about the
other stuff.  In particular, as Doug points out, the PROGRAM-gdb.rc
thing needs design work; but there doesn't seem to be a technical need
to link this to a security improvement.

Tom

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-19 21:07                   ` [patch] New set auto-load-local-gdbinit + disable it by default Tom Tromey
@ 2012-01-19 21:47                     ` Jan Kratochvil
  2012-01-19 21:53                       ` Doug Evans
  0 siblings, 1 reply; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-19 21:47 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Joel Brobecker, Eli Zaretskii, dje, gdb-patches

On Thu, 19 Jan 2012 22:05:46 +0100, Tom Tromey wrote:
> I am sold on the security changes, but I'm not as convinced about the
> other stuff.  In particular, as Doug points out, the PROGRAM-gdb.rc
> thing needs design work; but there doesn't seem to be a technical need
> to link this to a security improvement.

Yes, I am going to post it separately.  The general plan is something like:

set auto-load scripts (from set auto-load-scripts), for all of *-gdb.py, *-gdb.rc and .debug_gdb_scripts
set auto-load local-gdbinit # I think system-gdbinit and home-gdbinit make no sense to configure this way
set auto-load jit
set auto-load libthread_db
set auto-load safe-path ""
info auto-load (from info auto-load-scripts)

Thanks for catching JIT, that is really another security issue.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-19 21:47                     ` Jan Kratochvil
@ 2012-01-19 21:53                       ` Doug Evans
  2012-01-20  4:11                         ` Jan Kratochvil
  0 siblings, 1 reply; 54+ messages in thread
From: Doug Evans @ 2012-01-19 21:53 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Tom Tromey, Joel Brobecker, Eli Zaretskii, gdb-patches

On Thu, Jan 19, 2012 at 1:31 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> set auto-load libthread_db

What's the semantics of this?

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-19 21:53                       ` Doug Evans
@ 2012-01-20  4:11                         ` Jan Kratochvil
  0 siblings, 0 replies; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-20  4:11 UTC (permalink / raw)
  To: Doug Evans; +Cc: Tom Tromey, Joel Brobecker, Eli Zaretskii, gdb-patches

On Thu, 19 Jan 2012 22:47:26 +0100, Doug Evans wrote:
> On Thu, Jan 19, 2012 at 1:31 PM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
> > set auto-load libthread_db
> 
> What's the semantics of this?

set auto-load libthread_db [yes|no]

If one is found it checks `set auto-load safe-path' and appropriately loads it
or just warns on it.

I am aware one can already do `set libthread-db-search-path /_X' instead of
`set auto-load libthread_db no' but this way all to auto-loads will be at one
place.

Sorry I do not yet have it coded, this was just a draft.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-17 10:16 [patch] New set auto-load-local-gdbinit + disable it by default Jan Kratochvil
                   ` (3 preceding siblings ...)
  2012-01-17 20:09 ` Tom Tromey
@ 2012-01-24  0:33 ` Stan Shebs
  2012-01-24 15:54   ` Jan Kratochvil
  4 siblings, 1 reply; 54+ messages in thread
From: Stan Shebs @ 2012-01-24  0:33 UTC (permalink / raw)
  To: gdb-patches

On 1/17/12 1:55 AM, Jan Kratochvil wrote:
> Hi,
>
> this is a patch I want to post for many years.  There was:
> 	[RFA] .gdbinit security (revived) [incl doc]
> 	http://sourceware.org/ml/gdb-patches/2010-11/msg00276.html
> which was a follow-up for its referenced:
> 	RFC: Check permissions of .gdbinit files
> 	http://sourceware.org/ml/gdb-patches/2005-05/msg00637.html
> which was addressing:
> 	http://cve.mitre.org/cgi-bin/cvename.cgi?name=2005-1705

Sorry to come in late on this, but is this *really* an actual problem?

 From the tenor of the discussion, I get the impression of willingness 
to break longstanding development habits for most GNU folks in order to 
tick off a couple boxes on the security checklist.  Before making any 
specific changes, I think it would be prudent to ping all the groups 
that have their own .gdbinit files; if they're OK with the changes, then 
great.  Otherwise I think there will be a flood of complaints, and 
possibly people distributing versions of GDB with the change reverted, 
which would defeat the purpose. :-)

I would imagine that the people who open tarballs from unknown sources 
and run GDB on the contents already know about -nx and -x, eh?

Stan

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [patch] New set auto-load-local-gdbinit + disable it by default
  2012-01-24  0:33 ` Stan Shebs
@ 2012-01-24 15:54   ` Jan Kratochvil
  0 siblings, 0 replies; 54+ messages in thread
From: Jan Kratochvil @ 2012-01-24 15:54 UTC (permalink / raw)
  To: Stan Shebs; +Cc: gdb-patches

On Tue, 24 Jan 2012 01:28:02 +0100, Stan Shebs wrote:
> From the tenor of the discussion, I get the impression of
> willingness to break longstanding development habits for most GNU
> folks in order to tick off a couple boxes on the security checklist.

This CVE is a result of my request to Red Hat security people to evaluate the
security risk of .gdbinit + PythonGDB + other issues being addressed.  Red Hat
security considers this behavior as a valid risk and therefore they filed CVE
for it.  This way I can reference a professionals consider this GDB behavior
risky and it is just not my false and unfounded opinion.


> Before making any specific changes, I think it would be prudent to
> ping all the groups that have their own .gdbinit files; if they're
> OK with the changes, then great. 

I do not find acceptable to keep GDB insecure just because other projects want
it so.


> Otherwise I think there will be a
> flood of complaints, and possibly people distributing versions of
> GDB with the change reverted, which would defeat the purpose. :-)

I am fine many people will want the old .gdbinit behavior, various settings for
it are being both implemented
	echo 'set auto-load-local-gdbinit on' >>~/.gdbinit
and futher discussed.  But the users of .gdbinit
(a) should be at least warned it is insecure in some cases.
(b) New users should no longer get used to this problematic behavior.


> I would imagine that the people who open tarballs from unknown
> sources and run GDB on the contents already know about -nx and -x,
> eh?

-nx is definitely not enough:

On Tue, 17 Jan 2012 18:48:39 +0100, Jan Kratochvil wrote:
# Now instead of just -nx one has to use also "set auto-load-scripts off",
# use -ex "file X" and -ex "core-file Y" instead of just X and Y to get that
# "set auto-load-scripts off" executed first,
# use beforehand -ex "set libthread-db-search-path /foo", OK, that may be enough
# if I did not miss anything else.

And I forgot in the paragraph above about JIT which I have no idea how to
disable.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 54+ messages in thread

end of thread, other threads:[~2012-01-24 15:18 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-17 10:16 [patch] New set auto-load-local-gdbinit + disable it by default Jan Kratochvil
2012-01-17 12:34 ` Eli Zaretskii
2012-01-17 13:42   ` Joel Brobecker
2012-01-17 14:49     ` [patch 7.4] Deprecate local .gdbinit [Re: [patch] New set auto-load-local-gdbinit + disable it by default] Jan Kratochvil
2012-01-17 16:22       ` Doug Evans
2012-01-17 20:22   ` [patch] New set auto-load-local-gdbinit + disable it by default Tom Tromey
2012-01-17 16:15 ` Doug Evans
2012-01-17 16:34   ` Jan Kratochvil
2012-01-17 16:48     ` Doug Evans
2012-01-17 17:27       ` Jan Kratochvil
2012-01-17 17:33         ` Doug Evans
2012-01-17 17:55           ` Jan Kratochvil
2012-01-17 18:29             ` Eli Zaretskii
2012-01-17 18:35               ` Jan Kratochvil
2012-01-17 19:54                 ` Tom Tromey
2012-01-17 18:31             ` Doug Evans
2012-01-17 18:44               ` Jan Kratochvil
2012-01-17 19:12                 ` Doug Evans
2012-01-17 19:20                   ` Jan Kratochvil
2012-01-17 19:55               ` Tom Tromey
2012-01-17 20:24                 ` Pedro Alves
2012-01-17 20:26                   ` Tom Tromey
2012-01-17 20:35                   ` Jan Kratochvil
2012-01-17 20:56                     ` Pedro Alves
2012-01-17 19:30           ` Matt Rice
2012-01-17 19:37             ` Jan Kratochvil
2012-01-17 20:26             ` Tom Tromey
2012-01-17 18:00     ` Eli Zaretskii
2012-01-17 18:28       ` Jan Kratochvil
2012-01-17 18:43         ` Eli Zaretskii
2012-01-17 19:02           ` Jan Kratochvil
2012-01-17 20:29         ` Tom Tromey
2012-01-17 20:49           ` Jan Kratochvil
2012-01-17 20:54             ` Doug Evans
2012-01-17 21:10             ` Tom Tromey
2012-01-17 21:13           ` Eli Zaretskii
2012-01-17 21:14             ` Tom Tromey
2012-01-18  4:26               ` Joel Brobecker
2012-01-18 19:38                 ` Jan Kratochvil
2012-01-18 20:01                   ` Doug Evans
2012-01-19  6:30                   ` Joel Brobecker
2012-01-19 12:57                     ` [commit] rm gdb/testsuite/.gdbinit [Re: [patch] New set auto-load-local-gdbinit + disable it by default] Jan Kratochvil
2012-01-19 21:07                   ` [patch] New set auto-load-local-gdbinit + disable it by default Tom Tromey
2012-01-19 21:47                     ` Jan Kratochvil
2012-01-19 21:53                       ` Doug Evans
2012-01-20  4:11                         ` Jan Kratochvil
2012-01-19 21:06                 ` Tom Tromey
2012-01-17 22:16             ` Doug Evans
2012-01-18  3:05               ` Tom Tromey
2012-01-17 16:26 ` Matt Rice
2012-01-17 16:57   ` Doug Evans
2012-01-17 20:09 ` Tom Tromey
2012-01-24  0:33 ` Stan Shebs
2012-01-24 15:54   ` Jan Kratochvil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).