This is an archive of the discontinued LLVM Phabricator instance.

sanitizers: Disable crypt, crypt_r interceptors for glibc
Needs RevisionPublic

Authored by fweimer on Feb 14 2023, 11:56 PM.


Group Reviewers
Restricted Project

On GNU/Linux (glibc), the crypt and crypt_r functions are not part of the main shared object (, but libcrypt (with multiple possible sonames). The sanitizer libraries do not depend on libcrypt, so it can happen that during sanitizer library initialization, no real implementation will be found because the crypt, crypt_r functions are not present in the process image (yet). If its interceptors are called nevertheless, this results in a call through a null pointer when the sanitizer library attempts to forward the call to the real implementation.

Many distributions have already switched to libxcrypt, a library that is separate from glibc and that can be build with sanitizers directly (avoiding the need for interceptors). This patch disables building the interceptor for glibc targets.

I could not get the test harness to skip the crypt_r test for glibc, so I'm removing it. It is Linux-specific anyway, Android was already excluded, so it has limited scope anyway.


Diff Detail

Event Timeline

fweimer created this revision.Feb 14 2023, 11:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 14 2023, 11:56 PM
Herald added a subscriber: Enna1. · View Herald Transcript
fweimer requested review of this revision.Feb 14 2023, 11:56 PM

Looks like it was added for glibc Linux in D68431 for fix a problem.

Looks like it was added for glibc Linux in D68431 for fix a problem.

I think it was just done to increase coverage? These days, it's possible to build libxcrypt with sanitizers enabled instead, so there is a clear alternative.

Anyway, given the bugs introduced by the current interceptors, could we please move this forward? Thanks.

MaskRay accepted this revision.EditedApr 26 2023, 12:53 PM

I have tried to search for some crypt/crypt_r use cases but cannot find significant users. I think removing the interceptor will very unlikely cause issues for users with old glibc. musl still provides crypt/crypt_r. If it moves away, we can possibly remove the interceptor implementation.

This revision is now accepted and ready to land.Apr 26 2023, 12:53 PM
thesamesam accepted this revision.Apr 26 2023, 12:54 PM

Given glibc plans to remove its crypt() anyway, this seems fine.

@vitalybuka any objections?

vitalybuka requested changes to this revision.Apr 26 2023, 6:43 PM

I don't have objection in general. It's not clear what was broken when original patch was landed.

However if we keep the code, we should keep the test.

I propose to revert D68431.

This revision now requires changes to proceed.Apr 26 2023, 6:43 PM