[Toybox] mkpasswd crashes in github ci

[enh]

On Tue, Jun 24, 2025 at 6:37 PM Rob Landley <rob at landley.net> wrote:
>
> On 6/6/25 23:05, Rob Landley wrote:
> > On 6/5/25 08:51, enh wrote:
> >> doesn't affect android's build because we don't build mkpasswd, but
> >> the mkpasswd tests are failing on github with sigsegv:
> >
> > Sigh, the updated the ASAN plumbing, didn't they?
> >
> > https://landley.net/notes-2024.html#04-07-2024
> >
> > When you enable ASAN on current gcc toolchains, crypt() drops out and
> > becomes a NULL pointer dereference. I don't know why. I need to put a
> > crypt() implementation in lib/hash.c.
> >
> > If you disable ASAN, the tests pass just fine. ASAN is what breaks them.
>
> If I append this to lib/hash.c:
>
> char *krypt(char *phrase, char *setting)
> {
>    return "boom";
> }
> extern __typeof(crypt) crypt __attribute__((__weak__, __alias__("krypt")));
>
> Doing "ASAN=1 make clean mkpasswd; ./mkpasswd blah" STILL fails with the
> exact same null pointer dereference.
>
> I dunno what gcc's ASAN is doing, but it appears to be failing at
> runtime, not link time. I presumably need a compile time probe in
> scripts/genconfig.sh to detect crypt() failing and swap it out with a
> config symbol and #ifdefs, which is just sad...
>
> Maybe I should just switch the github test to "make toybox tests" which
> will build a toybox binary without ASAN and then test the existing
> binary? (The test builds enable ASAN, but make tests will use the
> existing binary if there is one, so listing both tests without asan...)

or just use clang for asan? probably good to have both clang and gcc
in ci anyway, since you support both...

> The other obvious alternative is just doing a new lib/crypt.c and always
> using that, but I'm probably never implementing "yescrypt" because there
> isn't a spec for it and the source tarball seemed intentionally
> obfuscated when I gave it a quick look. I've got md5, sha1, sha256, and
> sha512. I _could_ do des but... why? (I met solar at gentoo long ago, who
> seems retired now, but I think Openwall's Solar Designer is a different
> person. I suppose I could ask...)
>
> Ok, the REALLY WEIRD part is it's still doing that even if I take
> "crypt" out of the library list in make/make.sh. If it's ONLY using the
> alias to call krypt(), it still dies in a way that smashes the stack and
> forgets where it is. I stuck in a dprintf(2) right before it called
> crypt in toys/*/mkpasswd.c and it printed but the stack trace denied it:
>
> This is the line before calling crypt()
> AddressSanitizer:DEADLYSIGNAL
> =================================================================
> ==13214==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000
> (pc 0x000000000000 bp 0x7ffcc3b4e1d0 sp 0x7ffcc3b4e118 T0)
> ==13214==Hint: pc points to the zero page.
> ==13214==The signal is caused by a READ memory access.
> ==13214==Hint: address points to the zero page.
>      #0 0x0  (<unknown module>)
>      #1 0x56255bfac8cd in main /home/landley/toybox/toybox/main.c:367
>      #2 0x7fbd441f7249  (/lib/x86_64-linux-gnu/libc.so.6+0x27249)
>
> AddressSanitizer can not provide additional info.
> SUMMARY: AddressSanitizer: SEGV (<unknown module>)
> ==13214==ABORTING
>
> There's not a lot I can do with that. It seems to be losing its marbles
> on the FUNCTION PROTOTYPE somehow? Which in /usr/include/crypt.h is:
>
> extern char *crypt (const char *__phrase, const char *__setting)
> __THROW;
>
> Ok, just to be PEDANTIC I'll add the stupid consts to my function it's
> redirecting to (that CAN'T be it, ELF doesn't annotate at that level.)
> Yup, made no difference.
>
> Did ASAN special case the name "crypt" somehow? This seems unlikely...

i don't know about gcc, but a quick look at llvm's interceptors shows
crypt() and crypt_r()...

#if SANITIZER_INTERCEPT_CRYPT
INTERCEPTOR(char *, crypt, char *key, char *salt) {
  void *ctx;
  COMMON_INTERCEPTOR_ENTER(ctx, crypt, key, salt);
  COMMON_INTERCEPTOR_READ_RANGE(ctx, key, internal_strlen(key) + 1);
  COMMON_INTERCEPTOR_READ_RANGE(ctx, salt, internal_strlen(salt) + 1);
  char *res = REAL(crypt)(key, salt);
  if (res != nullptr)
    COMMON_INTERCEPTOR_INITIALIZE_RANGE(res, internal_strlen(res) + 1);
  return res;
}
#define INIT_CRYPT COMMON_INTERCEPT_FUNCTION(crypt);
#else
#define INIT_CRYPT
#endif

> Rob
>
> P.S. I want to switch more of the lib/*.c vs -lblah decisions to weak
> symbols in general, so I can have one "use toybox builtins instead of
> external libraries" config symbol dropping out the "weak" and then the
> existing $LIBRARIES plumbing in scripts/make.sh can do its thing with
> --gc-sections --as-needed dropping them back out again and it's all just
> one codepath... but that's not going to work around a runtime gnu/bug.