Page MenuHomePhabricator

[msan] Add stat-family interceptors on Linux
ClosedPublic

Authored by foobarrior on Oct 18 2021, 4:03 AM.

Details

Summary

Add following interceptors on Linux: stat, lstat, fstat, fstatat.

This fixes use-of-uninitialized value on platforms with GLIBC 2.33+.
In particular: Arch Linux, Ubuntu hirsute/impish.

The tests should have also been failing during the release on the mentioned platforms, but I cannot find any related discussion.

Most likely, the regression was introduced by glibc commit 8ed005daf0ab03e14250032:
all stat-family functions are now exported as shared functions.

Before, some of them (namely stat, lstat, fstat, fstatat) were provided as a part of libc_noshared.a and called their __xstat dopplegangers. This is still true for Debian Sid and earlier Ubuntu's. stat interceptors may be safely provided for them, no problem with that.

Closes https://github.com/google/sanitizers/issues/1452.
See also https://jira.mariadb.org/browse/MDEV-24841

Diff Detail

Event Timeline

foobarrior created this revision.Oct 18 2021, 4:03 AM
foobarrior requested review of this revision.Oct 18 2021, 4:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 18 2021, 4:03 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
foobarrior edited the summary of this revision. (Show Details)Oct 18 2021, 5:08 AM
eugenis added inline comments.Oct 18 2021, 2:04 PM
compiler-rt/lib/msan/msan_interceptors.cpp
711

Shouldn't this be "!SANITIZER_FREEBSD && !SANITIZER_NETBSD" ?

foobarrior updated this revision to Diff 380689.EditedOct 19 2021, 7:22 AM

Fix debian build.

__fxstatat interceptor now resides together with fstatat (just like fstat/__fxstat)

INTERCEPT_FUNCTION -> MSAN_INTERCEPT_FUNC to report interception errors

MSAN_INTERCEPT_FUNC(__fxstatat64)

foobarrior added inline comments.Oct 19 2021, 7:27 AM
compiler-rt/lib/msan/msan_interceptors.cpp
711

yes, stupid mistake, sorry

Please apply the clang-format lint changes in the comments.

compiler-rt/lib/msan/msan_interceptors.cpp
708

Please add _MAYBE_ to the macro name for consistency.

compiler-rt/test/msan/fstat.cpp
1 ↗(On Diff #380690)

why is -fPIC necessary?

Add _MAYBE_ where necessary.
Remove accidentally added -fPIC

Should I fix lint checks from the review? They seem to be incorrect from my point of view

compiler-rt/test/msan/fstat.cpp
1 ↗(On Diff #380690)

That snuck in accidentally, while i was testing. Removing

Reformat the code after eugenis comment is noticed

Reformat the code a bit less (no newlines after if's)

foobarrior edited the summary of this revision. (Show Details)Oct 20 2021, 6:01 AM
eugenis accepted this revision.Oct 20 2021, 4:30 PM

Should I fix lint checks from the review? They seem to be incorrect from my point of view

Yes please. Otherwise they will simply pop up in someone else's patch. We do what clang-format tell us to do :)

LGTM. Do you need me to land this change for you? I can reformat before commit, too.

This revision is now accepted and ready to land.Oct 20 2021, 4:30 PM

Yes, please. I have no commit rights.

This revision was automatically updated to reflect the committed changes.