[patch] [trivial] fix NULL deref

[patch] [trivial] fix NULL deref

[Ali Lakhia]

Please see patch to fix NULL dereference in strchr() function. Thanks.

-Ali

--- gdb-7.1/gdb/fork-child.c    2009-12-31 23:31:31.000000000 -0800
+++ gdb-7.1/gdb/fork-child.c 2010-09-16 10:17:25.000000000 -0700
@@ -52,7 +52,7 @@
 static void
 breakup_args (char *scratch, char **argv)
 {
-  char *cp = scratch;
+  char *cp = scratch, *tmp;

   for (;;)
     {
@@ -68,15 +68,16 @@
       *argv++ = cp;

       /* Scan for next arg separator.  */
-      cp = strchr (cp, ' ');
-      if (cp == NULL)
-       cp = strchr (cp, '\t');
-      if (cp == NULL)
-       cp = strchr (cp, '\n');
+      tmp = strchr (cp, ' ');
+      if (tmp == NULL)
+       tmp = strchr (cp, '\t');
+      if (tmp == NULL)
+       tmp = strchr (cp, '\n');

       /* No separators => end of string => break.  */
-      if (cp == NULL)
+      if (tmp == NULL)
        break;
+      cp = tmp;

       /* Replace the separator with a terminator.  */
       *cp++ = '\0';

Re: [patch] [trivial] fix NULL deref

[Daniel Jacobowitz]

On Thu, Sep 16, 2010 at 10:26:27AM -0700, Ali Lakhia wrote:
> Please see patch to fix NULL dereference in strchr() function. Thanks.

Interesting.  How did you find this problem?  I don't think this
function can ever be called.

> 
> -Ali
> 
> --- gdb-7.1/gdb/fork-child.c    2009-12-31 23:31:31.000000000 -0800
> +++ gdb-7.1/gdb/fork-child.c 2010-09-16 10:17:25.000000000 -0700
> @@ -52,7 +52,7 @@
>  static void
>  breakup_args (char *scratch, char **argv)
>  {
> -  char *cp = scratch;
> +  char *cp = scratch, *tmp;
> 
>    for (;;)
>      {
> @@ -68,15 +68,16 @@
>        *argv++ = cp;
> 
>        /* Scan for next arg separator.  */
> -      cp = strchr (cp, ' ');
> -      if (cp == NULL)
> -       cp = strchr (cp, '\t');
> -      if (cp == NULL)
> -       cp = strchr (cp, '\n');
> +      tmp = strchr (cp, ' ');
> +      if (tmp == NULL)
> +       tmp = strchr (cp, '\t');
> +      if (tmp == NULL)
> +       tmp = strchr (cp, '\n');
> 
>        /* No separators => end of string => break.  */
> -      if (cp == NULL)
> +      if (tmp == NULL)
>         break;
> +      cp = tmp;
> 
>        /* Replace the separator with a terminator.  */
>        *cp++ = '\0';
> 

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] [trivial] fix NULL deref

[Ali Lakhia]

This problem appears when the following macro is configured in
gdb/interior.h to be:

#define STARTUP_WITH_SHELL 0
#define START_INFERIOR_TRAPS_EXPECTED	1

The reason I set these macros is because forking a shell ends up
resenting some of the state when .cshrc is parsed (env vars like
$PATH, etc.). Thanks,

-Ali

Re: [patch] [trivial] fix NULL deref

[Ali Lakhia]

[-- Attachment #1: Type: text/plain, Size: 122 bytes --]

Haven't heard back from anyone. Just wanted to ping and make sure the
patch did not fall through the crack. Thanks,

-Ali

[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 835 bytes --]

--- gdb-7.1/gdb/fork-child.c	2009-12-31 23:31:31.000000000 -0800
+++ gdb-7.1/gdb/fork-child.c	2010-09-16 10:17:25.000000000 -0700
@@ -52,7 +52,7 @@
 static void
 breakup_args (char *scratch, char **argv)
 {
-  char *cp = scratch;
+  char *cp = scratch, *tmp;
 
   for (;;)
     {
@@ -68,15 +68,16 @@
       *argv++ = cp;
 
       /* Scan for next arg separator.  */
-      cp = strchr (cp, ' ');
-      if (cp == NULL)
-	cp = strchr (cp, '\t');
-      if (cp == NULL)
-	cp = strchr (cp, '\n');
+      tmp = strchr (cp, ' ');
+      if (tmp == NULL)
+	tmp = strchr (cp, '\t');
+      if (tmp == NULL)
+	tmp = strchr (cp, '\n');
 
       /* No separators => end of string => break.  */
-      if (cp == NULL)
+      if (tmp == NULL)
 	break;
+      cp = tmp;
 
       /* Replace the separator with a terminator.  */
       *cp++ = '\0';

Re: [patch] [trivial] fix NULL deref

[Michael Snyder]

Ali Lakhia wrote:
> Haven't heard back from anyone. Just wanted to ping and make sure the
> patch did not fall through the crack. Thanks,
> 
> -Ali

The change looks sound to me.

Can we get a changelog entry?

Re: [patch] [trivial] fix NULL deref

[Joel Brobecker]

> The change looks sound to me.
> Can we get a changelog entry?

In addition, I noticed that the diff was produced against gdb-7.1.
In all likeliness, the patch will still apply against head, but
patches should be submitted against HEAD. Available at:

    http://www.sourceware.org/gdb/current/

(I recommend you use the git mirror, if you are familiar with git).

-- 
Joel

Re: [patch] [trivial] fix NULL deref

[Michael Snyder]

Ali Lakhia wrote:
> Thanks. Yes, I verified that the patch will apply cleanly against
> head. Changelog below:
> 
> gdb/ChangeLog
> 2010-09-30  Ali Lakhia  <lakhia@alumni.utexas.net>
> 
>        * fork-child.c (breakup_args): Fix crash if shell forking is
>        disabled at compile time.

Committed.  Thanks.