bugGNU patch - Bugs: bug #58361, stack overrun when safe_rename()...

 
 

bug #58361: stack overrun when safe_rename() fails

Submitter:  None
Submitted:  Tue 12 May 2020 09:04:37 PM UTC
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  Works For Me
Privacy:  Public Assigned to:  None
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       No canned response available

 

Thu 04 Jun 2020 06:54:36 AM UTC, comment #5: 
Elan Ruusamäe <glen>
Wed 13 May 2020 08:23:54 AM UTC, comment #4: 

is the fix part of any release ?

I see there is bug #57883 that says last release was 2 years ago.

Anonymous
Tue 12 May 2020 09:56:59 PM UTC, comment #3: 

This has been fixed here:

commit b7b028a77bd855f6f56b17c8837fc1cca77b469d (refs/bisect/bad)
Author: Andreas Gruenbacher <agruen@gnu.org>
Date:   Fri Jun 28 00:30:25 2019 +0200

    Abort when cleaning up fails

   

    When a fatal error triggers during cleanup, another attempt will be made to
    clean up, which will likely lead to the same fatal error.  So instead, bail out
    when that happens.
    src/patch.c (cleanup): Bail out when called recursively.
    (main): There is no need to call output_files() before cleanup() as cleanup()
    already does that.

Andreas Gruenbacher <agruen>
Project Administrator
Tue 12 May 2020 09:07:55 PM UTC, comment #2: 

If you need more info feel free to reach me at https://github.com/vladak

Anonymous
Tue 12 May 2020 09:06:31 PM UTC, comment #1: 

Additional reference: this was with patch version 2.7.6 however I believe this is problem with the latest greatest as well.

Anonymous
Tue 12 May 2020 09:04:37 PM UTC, original submission:  

There is a stack overrun when safe_rename() fails, backup is in effect and the patch is question is git patch:

mkdir foo
echo "foo bar" > foo/file.txt
cat << EOF > file.patch
diff --git a/src/pch.c b/src/pch.c
index ff9ed2c..bc6278c 100644
--- a/file.txt
+++ b/file.txt
@@ -1 +1 @@
-foo bar
+foo foo
EOF

When run like so:

$ patch -d foo --strip=1 --backup --version-control=numbered < file.patch

and there is some renameat() failure, this enters infinite (sic!) loop which ends up by blowing the stack.

In my case I simulated error injection (was too lazy to fire up Dtrace Error Injection suite or LD_PRELOAD a custom library that would override the function in question) by changing safe_rename() function definition into this:

/* Replacement for rename() */
int safe_rename (const char oldpath, const char newpath)
{
  int olddirfd, newdirfd;
  int ret;

  errno = EIO;
  return -1;
}

this produces the following output:

patching file file.txt
./build/amd64/src/patch: **** Can't rename file file.txt to file.txt.~2~ : I/O error
./build/amd64/src/patch: **** Can't rename file file.txt to file.txt.~2~ : I/O error
./build/amd64/src/patch: **** Can't rename file file.txt to file.txt.~2~ : I/O error
./build/amd64/src/patch: **** Can't rename file file.txt to file.txt.~2~ : I/O error

.. many lines omitted

Segmentation Fault (core dumped)

The (blown) stack looks like this:

patch:core> $c ! head -30
libc.so.1`_ndoprnt_s+0x1a()
libc.so.1`_ndoprnt+0x12()
libc.so.1`fprintf+0x124()
pfatal+0x83()
move_file+0x4b0()
output_files.constprop.5+0xbf()
fatal_exit+0x10()
pfatal+0xec()
move_file+0x4b0()
output_files.constprop.5+0xbf()
fatal_exit+0x10()
pfatal+0xec()
move_file+0x4b0()
output_files.constprop.5+0xbf()
fatal_exit+0x10()
pfatal+0xec()
move_file+0x4b0()
output_files.constprop.5+0xbf()
fatal_exit+0x10()
pfatal+0xec()
move_file+0x4b0()
output_files.constprop.5+0xbf()
fatal_exit+0x10()
pfatal+0xec()
move_file+0x4b0()
output_files.constprop.5+0xbf()
fatal_exit+0x10()
pfatal+0xec()
move_file+0x4b0()
output_files.constprop.5+0xbf()

patch:core> $c ! tail -30
pfatal+0xec()
move_file+0x4b0()
output_files.constprop.5+0xbf()
fatal_exit+0x10()
pfatal+0xec()
move_file+0x4b0()
output_files.constprop.5+0xbf()
fatal_exit+0x10()
pfatal+0xec()
move_file+0x4b0()
output_files.constprop.5+0xbf()
fatal_exit+0x10()
pfatal+0xec()
move_file+0x4b0()
output_files.constprop.5+0xbf()
fatal_exit+0x10()
pfatal+0xec()
move_file+0x4b0()
output_files.constprop.5+0xbf()
fatal_exit+0x10()
pfatal+0xec()
move_file+0x4b0()
output_files.constprop.5+0xbf()
fatal_exit+0x10()
pfatal+0xec()
create_backup+0x597()
move_file+0x23a()
output_files.constprop.5+0xbf()
main+0x2e82()
_start+0x43()

The number of call frames is obviously highly dependent on system/architecture/settings. In my case (x86, 64-bit program) there were 10264 pfatal frames recorded in the core file.

This is because a recursion: main() calls output_files() (in the above stack trace called output_files.constprop.5 for some reason) which eventually calls pfatal() via create_backup() because safe_rename() failed.

Now, pfatal() calls fatal_exit() that calls cleanup() that calls output_files(NULL) and the cycle repeats (assuming the error condition is permanent).

For reference, this was hit on a Solaris 11.4 system when running the "prep" phase of https://github.com/oracle/solaris-ips that contains some heavy use of patches.

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by glen (Posted a comment)
  • -email is unavailable- added by agruen (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

     

     

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-05-12 agruen StatusNone Works For Me
        Open/ClosedOpen Closed

    Back to the top


    Powered by Savane 3.9