bug-readline
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Bug-readline] [BUG] a race condition in add_history() leads to SIGSEGV


From: Hong Cho
Subject: [Bug-readline] [BUG] a race condition in add_history() leads to SIGSEGV
Date: Mon, 5 Dec 2016 00:42:33 +0000

This was encountered with bash.

 

- bash: 2.05b

- readline: 4.3

- FreeBSD 8.4 / amd64

 

Yeah... the version of bash is old, but it seems add_history() in history.c hasn’t changed much in the master / devel branches at http://git.savannah.gnu.org/cgit/readline.git.

 

The race condition is that “history_length” gets updated before the “the_history” update is completed.

 

In several cases we’ve encountered, “bash” got a signal (SIGHUP), which interrupted add_history(), leaving “the_history” and “history_length” inconsistent. In the signal handling path, it tried to access “the_history” in history_do_write() in histfile.c, which tried to de-reference a NULL pointer.

 

Here is a proposed patch for add_history() in history.c:

 

------

     const char *string;

{

  HIST_ENTRY *temp;

+  int temp_length = history_length;

 

  if (history_stifled && (history_length == history_max_entries))

    {

[...]

          history_size = DEFAULT_HISTORY_GROW_SIZE;

          the_history = (HIST_ENTRY **)xmalloc (history_size * sizeof (HIST_ENTRY *));

-          history_length = 1;

+          temp_length = 1;

        }

      else

         {

[...]

              the_history = (HIST_ENTRY **)

                xrealloc (the_history, history_size * sizeof (HIST_ENTRY *));

             }

-          history_length++;

+          temp_length++;

        }

     }

 

[...]

 

  temp = alloc_history_entry ((char *)string, hist_inittime ());

 

-  the_history[history_length] = (HIST_ENTRY *)NULL;

-  the_history[history_length - 1] = temp;

+  the_history[temp_length] = (HIST_ENTRY *)NULL;

+  the_history[temp_length - 1] = temp;

+  /* Delay the length update until the history update is complete. */

+  history_length = temp_length;

}

 

/* Change the time stamp of the most recent history entry to STRING. */

------

 

Hong.

 

 


reply via email to

[Prev in Thread] Current Thread [Next in Thread]