[Bug 1422795] Re: bash crashes often if inputrc contains revert-all-at-newline

[Jeffrey Hutzelman]

This was fixed in 4.3-8 and so is already fixed in utopic and later.  It
will require an SRU to fix in trusty.  I've attached a debdiff
containing the needed change.

** Description changed:

  Debian bug: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=747341
  The Debian bug includes complete reproduction case. Basically:
  with .inputrc containing
  set revert-all-at-newline On
  
  Go back in the commandline history, edit a command, then submit a different command (may be empty)
  Such as:
  $ ls something
  $ <UP><CTRL+W><DOWN><ENTER>
  
  Attached diff is confirmed to fix the issue.
  
  ProblemType: Bug
  DistroRelease: Ubuntu 14.04
  Package: bash 4.3-7ubuntu1.5 [origin: goobuntu-trusty-testing-desktop]
  ProcVersionSignature: Ubuntu 3.13.0-44.73-generic 3.13.11-ckt12
  Uname: Linux 3.13.0-44-generic x86_64
  NonfreeKernelModules: nvidia
  ApportVersion: 2.14.1-0ubuntu3.6
  Architecture: amd64
  CurrentDesktop: X-Cinnamon
  Date: Tue Feb 17 15:49:30 2015
  SourcePackage: bash
  UpgradeStatus: No upgrade log present (probably fresh install)
  modified.conffile..etc.bash.bashrc: [modified]
  mtime.conffile..etc.bash.bashrc: 2015-01-27T03:27:18.751405
+ 
+ 
+ [Test Case]
+ 
+ Adapted from the Debian bug report:
+ 
+ 1. echo "set revert-all-at-newline on" > bug.inputrc
+ 2. INPUTRC=bug.inputrc bash
+ 3. echo hello
+ 4. ^P^U^N^M  [Hold down control and type "punm".]
+ 
+ Bash should die immediately with SIGABRT.
+ 
+ 
+ [Regression Potential]
+ 
+ Relatively low.
+ 
+ The change has no effect at all unless _rl_revert_all_lines() is called,
+ which only happens if revert-all-at-newline is set, and then only when a
+ newline is typed.  So, the potential for regression is essentially zero for
+ non-interactive shells and for anyone not using revert-all-at-newline (which
+ is not the default).
+ 
+ Further, this change appeared upstream and in both Debian and Ubuntu over
+ a year ago, so it's had plenty of public testing.
+ 
+ lib/readline/misc.c:_rl_revert_all_lines() contains a loop which iterates
+ over history entries, reverting changes to each history entry.  This patch
+ causes entry->data, which points to the per-entry undo list, to be cleared
+ before reverting edits rather than after.  At first glance, this shouldn't
+ make any difference.  However, it prevents rl_do_undo() from replacing the
+ history entry with one reflecting the change.  Otherwise, the entry gets
+ freed, leaving _rl_revert_all_lines() with an invalid pointer.
+ 
+ _Not_ having an invalid pointer and double-free certainly can't be worse
+ than the current situation.  Since we're avoiding is making the pointer
+ invalid rather than not doing the free, the chance of a new leak is pretty
+ much nonexistent.

** Patch added: "debdiff containing the upstream patch"
   https://bugs.launchpad.net/gnubash/+bug/1422795/+attachment/4497315/+files/bash-readline-revert.debdiff

-- 
You received this bug notification because you are a member of Ubuntu
Foundations Bugs, which is subscribed to bash in Ubuntu.
https://bugs.launchpad.net/bugs/1422795

Title:
  bash crashes often if inputrc contains revert-all-at-newline

Status in Gnu Bash:
  New
Status in bash package in Ubuntu:
  Confirmed
Status in bash package in Debian:
  Unknown

Bug description:
  Debian bug: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=747341
  The Debian bug includes complete reproduction case. Basically:
  with .inputrc containing
  set revert-all-at-newline On

  Go back in the commandline history, edit a command, then submit a different command (may be empty)
  Such as:
  $ ls something
  $ <UP><CTRL+W><DOWN><ENTER>

  Attached diff is confirmed to fix the issue.

  ProblemType: Bug
  DistroRelease: Ubuntu 14.04
  Package: bash 4.3-7ubuntu1.5 [origin: goobuntu-trusty-testing-desktop]
  ProcVersionSignature: Ubuntu 3.13.0-44.73-generic 3.13.11-ckt12
  Uname: Linux 3.13.0-44-generic x86_64
  NonfreeKernelModules: nvidia
  ApportVersion: 2.14.1-0ubuntu3.6
  Architecture: amd64
  CurrentDesktop: X-Cinnamon
  Date: Tue Feb 17 15:49:30 2015
  SourcePackage: bash
  UpgradeStatus: No upgrade log present (probably fresh install)
  modified.conffile..etc.bash.bashrc: [modified]
  mtime.conffile..etc.bash.bashrc: 2015-01-27T03:27:18.751405

  
  [Test Case]

  Adapted from the Debian bug report:

  1. echo "set revert-all-at-newline on" > bug.inputrc
  2. INPUTRC=bug.inputrc bash
  3. echo hello
  4. ^P^U^N^M  [Hold down control and type "punm".]

  Bash should die immediately with SIGABRT.

  
  [Regression Potential]

  Relatively low.

  The change has no effect at all unless _rl_revert_all_lines() is called,
  which only happens if revert-all-at-newline is set, and then only when a
  newline is typed.  So, the potential for regression is essentially zero for
  non-interactive shells and for anyone not using revert-all-at-newline (which
  is not the default).

  Further, this change appeared upstream and in both Debian and Ubuntu over
  a year ago, so it's had plenty of public testing.

  lib/readline/misc.c:_rl_revert_all_lines() contains a loop which iterates
  over history entries, reverting changes to each history entry.  This patch
  causes entry->data, which points to the per-entry undo list, to be cleared
  before reverting edits rather than after.  At first glance, this shouldn't
  make any difference.  However, it prevents rl_do_undo() from replacing the
  history entry with one reflecting the change.  Otherwise, the entry gets
  freed, leaving _rl_revert_all_lines() with an invalid pointer.

  _Not_ having an invalid pointer and double-free certainly can't be worse
  than the current situation.  Since we're avoiding is making the pointer
  invalid rather than not doing the free, the chance of a new leak is pretty
  much nonexistent.

To manage notifications about this bug go to:
https://bugs.launchpad.net/gnubash/+bug/1422795/+subscriptions