Skip to content

Commit 11bd374

Browse files
committed
Fix memory corruption in TextStream.
This resulted in observable crashes with musl libc due to corruption in malloc, but could also be observed in valgrind, when building GNU Radio's doxygen docs. One of the valgrind errors is reproduced below. ==26== Invalid read of size 4 ==26== at 0x405A62D: fwrite (fwrite.c:32) ==26== by 0x47154D: flush (textstream.h:181) ==26== by 0x47154D: setFile (textstream.h:74) ==26== by 0x47154D: OutputGenerator::startPlainFile(QCString const&) (outputgen.cpp:69) ==26== by 0x2EFF57: HtmlGenerator::startFile(QCString const&, QCString const&, QCString const&, int) (htmlgen.cpp:1121) ==26== by 0x3083EC: forall<OutputGenerator, const QCString&, const QCString&, const QCString&, int, const QCString&, const QCString&, const QCString&, int&> (outputlist.h:512) ==26== by 0x3083EC: startFile (outputlist.h:91) ==26== by 0x3083EC: startFile(OutputList&, QCString const&, QCString const&, QCString const&, HighlightedItem, bool, QCString const&) (index.cpp:240) ==26== by 0x296807: FileDefImpl::writeSourceHeader(OutputList&) (filedef.cpp:1110) ==26== by 0x276751: generateFileSources() (doxygen.cpp:7971) ==26== by 0x277793: generateOutput() (doxygen.cpp:11992) ==26== by 0x21173E: main (main.cpp:38) ==26== Address 0x130bb62c is 140 bytes inside a block of size 1,264 free'd ==26== at 0x48BAA4B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==26== by 0x40585D0: fclose (fclose.c:35) ==26== by 0x4DCE5D7: std::__basic_file<char>::close() (in /usr/lib/libstdc++.so.6.0.28) ==26== by 0x4E1D676: std::basic_filebuf<char, std::char_traits<char> >::close() (in /usr/lib/libstdc++.so.6.0.28) ==26== by 0x4B055C: ~basic_filebuf (fstream:249) ==26== by 0x4B055C: ~basic_ofstream (fstream:863) ==26== by 0x4B055C: writeJavaScriptSearchIndex() (searchindex.cpp:923) ==26== by 0x2776F0: generateOutput() (doxygen.cpp:11957) ==26== by 0x21173E: main (main.cpp:38) ==26== Block was alloc'd at ==26== at 0x48B981F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==26== by 0x40578FE: fdopen (__fdopen.c:21) ==26== by 0x405943E: fopen (fopen.c:26) ==26== by 0x4DCE55F: std::__basic_file<char>::open(char const*, std::_Ios_Openmode, int) (in /usr/lib/libstdc++.so.6.0.28) ==26== by 0x4E1E19A: std::basic_filebuf<char, std::char_traits<char> >::open(char const*, std::_Ios_Openmode) (in /usr/lib/libstdc++.so.6.0.28) ==26== by 0x4B0470: open (fstream:332) ==26== by 0x4B0470: open (fstream:962) ==26== by 0x4B0470: basic_ofstream (fstream:830) ==26== by 0x4B0470: writeJavaScriptSearchIndex() (searchindex.cpp:923) ==26== by 0x2776F0: generateOutput() (doxygen.cpp:11957) ==26== by 0x21173E: main (main.cpp:38) What was happening is that the TextStream object is reused for multiple different files. Therefore, when one calls setStream(nullptr) or setFile(nullptr), m_f or m_s will, respectively, still contain a pointer to the previous member, which is used when one calls setFile() or setStream() again, since these methods will call flush(). For example, a program doing s.setFile(f1); s.setStream(nullptr); fclose(f1); s.setFile(f2); will call fwrite(f1, ...). This pattern can be observed in many parts of Doxygen, so fixing it in TextStream itself by always zeroing the other pointer is the simplest fix.
1 parent 81adb94 commit 11bd374

File tree

1 file changed

+2
-0
lines changed

1 file changed

+2
-0
lines changed

src/textstream.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,13 @@ class TextStream final
6767
{
6868
flush();
6969
m_s = s;
70+
m_f = nullptr;
7071
}
7172

7273
void setFile(FILE *f)
7374
{
7475
flush();
76+
m_s = nullptr;
7577
m_f = f;
7678
}
7779

0 commit comments

Comments
 (0)