Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Potential Memory Leak in mmk_stub_create_static Function #36

Open
RiversJin opened this issue Jun 26, 2023 · 0 comments
Open

Potential Memory Leak in mmk_stub_create_static Function #36

RiversJin opened this issue Jun 26, 2023 · 0 comments

Comments

@RiversJin
Copy link

Hey there,

I think I've found a memory leak in mmk_stub_create_static. It looks like name isn't getting freed when the function returns an "ENOENT" error.

This isn't a big deal for a test framework since tests don't run for long, but it does trigger a leak report when using tools like Address Sanitizer.

I think it's an easy fix and I'd like to submit a PR if that's okay with you.

Thanks!

RiversJin
@RiversJin RiversJin mentioned this issue Jun 26, 2023
Closed
ahcorde pushed a commit to ahcorde/Mimick that referenced this issue Jul 24, 2024
@clalancette
Find the symbols via DT_GNU_HASH instead of DT_HASH. (Snaipe#36)
4dbd495 
* Switch to CMake 3.5.

This avoids a warning when building with modern
CMake.

Signed-off-by: Chris Lalancette <[email protected]>

* Find the symbols via DT_GNU_HASH instead of DT_HASH.

Since glibc 2.36 (released in August 2022), builds of libc.so.6
are built with the default value of --hash-style on all platforms.
The immediate effect of this is that linker no longer generates
a DT_HASH section, which is what Mimick uses to detect vital
functions like vfprintf and abort.

It turns out that Ubuntu and Debian specifically override this
behavior on amd64 and i386, since there are some proprietary
applications on those platforms that depend on this.  However,
this override is *not* applied on aarch64, so there is no DT_HASH.
This explains the discrepancy we see when running CI on amd64
(where Mimick tests succeed) and aarch64 (where Mimick tests fail).

It also turns out that DT_HASH is "deprecated", and has been for
about 15 years.  Thus, all of our platforms (going back to RHEL-8)
support this construct.

Thus, this commit implements getting symbols from DT_GNU_HASH instead
of from DT_HASH.  Note that it also changes it so that we *prefer*
to get the data from DT_GNU_HASH, as someday DT_HASH may go away
entirely.

I should note that I borrowed heavily from
https://flapenguin.me/elf-dt-gnu-hash and
https://sourceware.org/git/?p=glibc.git;a=blob;f=elf/dl-lookup.c;h=3d2369dbf2b7ca219eaf80a820e2a8e1329fbf50;hb=HEAD#l350
to implement this, though I made a bunch of changes to fix
warnings and better integrate into the Mimick source code.

Signed-off-by: Chris Lalancette <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@RiversJin