|Subject:||[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;
+ 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;
the_history = (HIST_ENTRY **)
xrealloc (the_history, history_size * sizeof (HIST_ENTRY *));
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. */
|[Prev in Thread]||Current Thread||[Next in Thread]|