Skip to content

Commit

Permalink
Fix annoying silent crash in builtins.sh under ASan
Browse files Browse the repository at this point in the history
Under ASan the builtins.sh script silently exits with exit status 1
and no backtrace. After isolating the bug into a simplified script,
I managed to reproduce it with an actual stacktrace. Reproducer:
   set -o xtrace  # Shows how far the script goes before crashing
                  # (which is inconsistent and system-dependent
                  # AFAICT; ALL_LIBCMD can affect it).
   bltin='/opt/ast/bin/basename
   /opt/ast/bin/cat
   /opt/ast/bin/cp
   /opt/ast/bin/cut
   /opt/ast/bin/dirname
   /opt/ast/bin/getconf
   /opt/ast/bin/ln
   /opt/ast/bin/mktemp
   /opt/ast/bin/mv'  # Feel free to expand this with other commands
                     # from libcmd if you with.
   for i in ${bltin}
   do ({ PATH=/opt/ast/bin; "${bltin##*/}" --this-option-does-not-exist; } 2>&1)
   done
Stacktrace obtained after much effort:
=================================================================
==116622==ERROR: AddressSanitizer: heap-use-after-free on address 0x502000000e50 at pc 0x72ad1bb52b7f bp 0x7ffc8b5a0cd0 sp 0x7ffc8b5a0478
READ of size 3 at 0x502000000e50 thread T0
    #0 0x72ad1bb52b7e in memcpy /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc:115
    ksh93#1 0x5bb0a922f3de in synthesize /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:466
    ksh93#2 0x5bb0a92305cd in initialize /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:591
    ksh93#3 0x5bb0a9230fac in format /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:632
    ksh93#4 0x5bb0a923ba4b in astgetconf /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:1382
    ksh93#5 0x5bb0a923d5c5 in astconf /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:1472
    ksh93#6 0x5bb0a924e07b in initconformance /home/johno/GitRepos/KornShell/ksh/src/lib/libast/misc/conformance.c:50
    ksh93#7 0x5bb0a924eff2 in conformance /home/johno/GitRepos/KornShell/ksh/src/lib/libast/misc/conformance.c:122
    ksh93#8 0x5bb0a9288b8e in b_cp /home/johno/GitRepos/KornShell/ksh/src/lib/libcmd/cp.c:706
    <CUT>

src/lib/libast/port/astconf.c:
- If fp->value and value pointed to the same allocated memory
  before realloc, avoid memcpy as value now points to freed memory.
- Produce an obvious panic if memory allocation fails.

src/cmd/ksh93/sh/init.c:
- For correctness, prevent memory leaks by freeing memory in sh_realloc
  upon failure.
  • Loading branch information
JohnoKing committed Dec 29, 2024
1 parent bb83575 commit 885258f
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 5 deletions.
4 changes: 4 additions & 0 deletions src/cmd/ksh93/sh/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,11 @@ void *sh_realloc(void *ptr, size_t size)
void *cp;
cp = realloc(ptr, size);
if(!cp)
{
if(ptr)
free(ptr);
nomemory(size);
}
return cp;
}

Expand Down
23 changes: 18 additions & 5 deletions src/lib/libast/port/astconf.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ synthesize(Feature_t* fp, const char* path, const char* value)
char* d;
char* v;
char* p;
int docpy;
int n;

#if DEBUG_astconf
Expand Down Expand Up @@ -458,14 +459,26 @@ synthesize(Feature_t* fp, const char* path, const char* value)
fp->value = 0;
if (n == 1 && (*value == '0' || *value == '-'))
n = 0;
if (!(fp->value = newof(fp->value, char, n, 1)))
fp->value = null;
docpy = fp->value != value;
if (!fp->value && !(fp->value = calloc(1, n + 1)))
{
error(ERROR_SYSTEM|ERROR_PANIC,"out of memory");
UNREACHABLE();
}
else
{
fp->flags |= CONF_ALLOC;
memcpy(fp->value, value, n);
fp->value[n] = 0;
char *tofree = fp->value;
if (!(fp->value = realloc(fp->value, n + 1)))
{
free(tofree);
error(ERROR_SYSTEM|ERROR_PANIC,"out of memory");
UNREACHABLE();
}
}
fp->flags |= CONF_ALLOC;
if(docpy)
memcpy(fp->value, value, n);
fp->value[n] = 0;
return fp->value;
}

Expand Down

0 comments on commit 885258f

Please sign in to comment.