|
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. |
[Prev in Thread] | Current Thread | [Next in Thread] |