public inbox for glibc-bugs@sourceware.org help / color / mirror / Atom feed
From: "skissane at gmail dot com" <sourceware-bugzilla@sourceware.org> To: glibc-bugs@sourceware.org Subject: [Bug libc/30101] New: Memory corruption when incorrectly calling _mcleanup/__monstartup/moncontrol repeatedly or in wrong order Date: Thu, 09 Feb 2023 13:17:37 +0000 [thread overview] Message-ID: <bug-30101-131@http.sourceware.org/bugzilla/> (raw) https://sourceware.org/bugzilla/show_bug.cgi?id=30101 Bug ID: 30101 Summary: Memory corruption when incorrectly calling _mcleanup/__monstartup/moncontrol repeatedly or in wrong order Product: glibc Version: unspecified Status: UNCONFIRMED Severity: normal Priority: P2 Component: libc Assignee: unassigned at sourceware dot org Reporter: skissane at gmail dot com CC: drepper.fsp at gmail dot com Target Milestone: --- Reproduced in latest Git master branch. I wrote a patch which I will send to the mailing list shortly. It is possible to corrupt memory by calling the _mcleanup, __monstartup, and moncontrol APIs an incorrect number of times, or in an incorrect order. While such incorrect use of these APIs is of course unsupported, it still should not corrupt memory or crash. And, I think it is relatively simple to fix these bugs, so why not. ##### Issue 1: Calling _mcleanup twice in a row crashes This one is the easiest to reproduce. The second call will crash due to a double free. Additionally, prior to the double free, it attempts to read deallocated memory (although that is only obvious under valgrind), and will write out a gmon.out file containing (possibly meaningless) data read from that deallocated memory. Test case: 1. Create file test_mcleanup.c with following contents: #include <sys/gmon.h> int main(int argc, char **argv) { _mcleanup(); return 0; } 2. Build and run it: gcc -pg -o test_mcleanup test_mcleanup.c && ./test_mcleanup 3. Observe the crash: double free or corruption (top) Aborted (core dumped) This happens because _mcleanup is called once manually (from main), and then a second time (as an atexit handler installed by __gmon_start__). _mcleanup calls free on the tos pointer, but doesn't set it to NULL, so the second call results in a double free. To fix this, zero the _gmonparam buffer at end of _mcleanup. While setting tos=NULL would be a minimal fix, zeroing the buffer is safest and helps prevent other issues. To ensure we don't read any unallocated memory, we also need to skip calling write_gmon() when the tos pointer is NULL (since write_gmon is written to assume tos!=NULL). ##### Issue 2: Calling __monstartup repeatedly leaks memory This issue is only visible under valgrind. __monstartup allocates a buffer, the address of which is placed in _gmonparam.tos. If you call __monstartup again (without calling _mcleanup first), it allocates a new buffer, and overwrites the address of the old one, which is never freed. I think the simplest solution is to check that tos == NULL at start of __monstartup, and just return in that case, turning improper repeated calls to __monstartup into no-ops. The other option to fix this would be to free any existing buffer at the start of __monstartup; before doing that, we must also call moncontrol(0), in case the kernel is writing to it due to the __profil system call. That would make a second call to __monstartup override the first, instead of being a no-op; while this is a reasonable solution, I prefer the first option since it is simpler. ##### Issue 3: Calling moncontrol(1) after _mcleanup corrupts memory This issue is hypothetical, it would be difficult to reproduce, so I have not attempted to do so. A buggy program calls _mcleanup(), then some time later incorrectly calls moncontrol(1), without an intervening call to __monstartup. Prior to the fix for to issue (1) above, _mcleanup() did not zero out _gmonparam.kcount, which would now point to freed memory, which may have since been reallocated for some unrelated purpose. That pointer to freed memory would then be passed to the __profil system call, which would then cause the kernel to overwrite that unrelated data structure with profiling data. With zeroing of _gmonparam at end of _mcleanup to fix (1), this issue can no longer occur. Instead something less drastic goes wrong – the _gmonparam.state gets incorrectly changed to GMON_PROF_ON, despite the fact that profiling is actually off. From my reading of the mcount code, it seems plausible that might cause memory corruption or a crash, but I haven't been able to reproduce any such issue. To be safest, ensure the state remains GMON_PROF_OFF if tos==NULL ##### Issue 4: moncontrol(0) doesn't disable profiling in error state This is also a hypothetical issue I haven't been able to reproduce. It relates to the if branch at start of moncontrol, which makes the operation a no-op when in GMON_PROF_ERROR state. There are two ways we could end up in GMON_PROF_ERROR state: memory allocation failure (out of memory) in __monstartup, or an overflow error in the mcount function. Suppose we call _mcleanup(), which in turn calls moncontrol(0). If state=GMON_PROF_ERROR due to memory allocation failure, we will have tos=NULL, and we never will have enabled profiling, so even though moncontrol(0) won't disable profiling, it shouldn't be on to begin with, so we are fine. However, suppose we had an overflow error in mcount. Now state=GMON_PROF_ERROR, but profiling is on, and tos!=NULL. We call _mcleanup(), which in turn calls moncontrol(0) – but since state=GMON_PROF_ERROR, moncontrol does not make the __profil system call to turn profiling off. Now _mcleanup() frees the tos buffer, but the kernel goes on writing the profiling data into the now-deallocated memory, so this is another variant on issue 3. Unlike issue (3), zeroing the buffer in _mcleanup (to fix (1)) makes no difference here, since the address of the freed buffer is saved in kernel memory. Aside from the difficulty in reproducing issue (3), I'm also not sure how to trigger an overflow error in mcount. However, the fix is simple: have mcontrol() still disable profiling even if called in error state. But make sure we keep the current behaviour that mcontrol(0) doesn't clear any error condition. One change in behaviour I would propose, is make mcontrol(1) in error condition turn into a stop instead of a nop; I think doing it that way makes the code a little simpler, and leaving kernel profiling on in the error state is pointless (since we won't write the file). -- You are receiving this mail because: You are on the CC list for the bug.
next reply other threads:[~2023-02-09 13:17 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-02-09 13:17 skissane at gmail dot com [this message] 2023-02-09 13:23 ` [Bug libc/30101] " skissane at gmail dot com 2023-02-10 22:00 ` skissane at gmail dot com 2023-02-10 22:04 ` skissane at gmail dot com
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=bug-30101-131@http.sourceware.org/bugzilla/ \ --to=sourceware-bugzilla@sourceware.org \ --cc=glibc-bugs@sourceware.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).