[libcxx-commits] [PATCH] D64979: [libc++] Set __file_ to 0 in basic_filebuf::close() even if fclose fails

[Petr Hosek via Phabricator via libcxx-commits]

phosek created this revision.
phosek added reviewers: ldionne, EricWF, mclow.lists.
Herald added subscribers: libcxx-commits, dexonsmith, christof.

This issue was detected by ASan in one of our tests. This test manually
invokes basic_filebuf::cloe(). fclose(__h.release() returned a non-zero
exit status, so __file_ wasn't set to 0. Later when basic_filebuf
destructor ran, we would enter the if (__file_) block again leading to
heap-use-after-free error.

The POSIX specification for fclose says that independently of the return
value, fclose closes the underlying file descriptor and any further
access (including another call to fclose()) to the stream results in
undefined behavior. This is exactly what happened in our test case.

To avoid this issue, we have to always set __file_ to 0 independently of
the fclose return value.


Repository:
  rCXX libc++

https://reviews.llvm.org/D64979

Files:
  libcxx/include/fstream


Index: libcxx/include/fstream
===================================================================
--- libcxx/include/fstream
+++ libcxx/include/fstream
@@ -697,10 +697,9 @@
         unique_ptr<FILE, int(*)(FILE*)> __h(__file_, fclose);
         if (sync())
             __rt = 0;
-        if (fclose(__h.release()) == 0)
-            __file_ = 0;
-        else
+        if (fclose(__h.release()))
             __rt = 0;
+        __file_ = 0;
         setbuf(0, 0);
     }
     return __rt;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D64979.210761.patch
Type: text/x-patch
Size: 490 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190719/e3b082db/attachment.bin>