1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
|
From 11bd374ea216b561deaf9be675ddd8941b80a487 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Nogueira?= <erico.erc@gmail.com>
Date: Sun, 29 Aug 2021 01:07:27 -0300
Subject: [PATCH] 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.
---
src/textstream.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/textstream.h b/src/textstream.h
index 161ce7fe1..38027ec07 100644
--- a/src/textstream.h
+++ b/src/textstream.h
@@ -67,11 +67,13 @@ class TextStream final
{
flush();
m_s = s;
+ m_f = nullptr;
}
void setFile(FILE *f)
{
flush();
+ m_s = nullptr;
m_f = f;
}
|
