Skip to content

Commit

Permalink
Fix memory leak in defpathinit() (#441)
Browse files Browse the repository at this point in the history
Currently, running ksh under ASan without the ASAN_OPTIONS variable
set to 'detect_leaks=0' usually ends with ASan complaining about a
memory leak in defpathinit() (this leak doesn't grow in a loop, so
no regression test was added to leaks.sh). Reproducer:

 $ ENV=/dev/null arch/*/bin/ksh
 $ cp -?
 cp: invalid option -- '?'
 Try 'cp --help' for more information.
 $ exit

 =================================================================
 ==225132==ERROR: LeakSanitizer: detected memory leaks

 Direct leak of 85 byte(s) in 1 object(s) allocated from:
     #0 0x7f6dab42d459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
     #1 0x5647b77fe144 in sh_calloc /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/init.c:265
     #2 0x5647b788fea9 in path_addcomp /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:1567
     #3 0x5647b78911ed in path_addpath /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:1705
     #4 0x5647b7888e82 in defpathinit /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:442
     #5 0x5647b78869f3 in ondefpath /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:67
 --- cut ---
 SUMMARY: AddressSanitizer: 174 byte(s) leaked in 2 allocation(s).

Analysis: The previous code was leaking memory because
defpathinit() returns a pointer from path_addpath(), which has
memory allocated to it in path_addcomp(). This is the code ASan
traced as having allocated memory:

442:	return(path_addpath((Pathcomp_t*)0,(defpath),PATH_PATH));

In path_addpath():
1705:	first = path_addcomp(first,old,cp,type);
[...]
1729:	return(first);

In path_addcomp():
1567:	pp = sh_newof((Pathcomp_t*)0,Pathcomp_t,1,len+1);

The ondefpath() function doesn't save a reference to the pointer
returned by defpathinit(), which causes the memory leak:

66:	if(!defpath)
67:		defpathinit();

The changes in this commit avoid this problem by setting the
defpath variable without also calling path_addpath().

src/cmd/ksh93/sh/path.c:
- Move the code for allocating defpath from defpathinit() into its
  own dedicated function called std_path(). This function is called
  by defpathinit() and ondefpath() to obtain the current string
  stored in the defpath variable. This bugfix is adapted from a
  fork of ksh2020: l0stman/ksh@db5c83a6
  • Loading branch information
JohnoKing authored and McDutchie committed Jan 30, 2022
1 parent 9e525c5 commit e3a1dda
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 12 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ Any uppercase BUG_* names are modernish shell bug IDs.
shell when suspending (Ctrl+Z) an external command invoked by a dot script
or POSIX shell function, or via 'eval'.

- Fixed a memory leak that occurred when running a command that was found on
the PATH.

2022-01-26:

- On Cygwin, ksh now executes scripts that do not have a #! path itself,
Expand Down
25 changes: 13 additions & 12 deletions src/cmd/ksh93/sh/path.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,21 @@ static int checkdotpaths(Pathcomp_t*,Pathcomp_t*,Pathcomp_t*,int);
static void checkdup(register Pathcomp_t*);
static Pathcomp_t *defpathinit(void);

static const char *defpath; /* default path that finds standard utilities */
static const char *std_path(void)
{
static const char *defpath; /* default path that finds standard utilities */
if(!defpath)
{
if(!(defpath = astconf("PATH",NIL(char*),NIL(char*))))
abort();
defpath = sh_strdup(defpath); /* the value returned by astconf() is short-lived */
}
return(defpath);
}

static int ondefpath(const char *name)
{
const char *cp;
if(!defpath)
defpathinit();
cp = defpath;
const char *cp = std_path();
if(cp)
{
const char *sp;
Expand Down Expand Up @@ -433,13 +440,7 @@ Pathcomp_t *path_nextcomp(register Pathcomp_t *pp, const char *name, Pathcomp_t

static Pathcomp_t* defpathinit(void)
{
if(!defpath)
{
if(!(defpath = astconf("PATH",NIL(char*),NIL(char*))))
abort();
defpath = sh_strdup(defpath); /* the value returned by astconf() is short-lived */
}
return(path_addpath((Pathcomp_t*)0,(defpath),PATH_PATH));
return(path_addpath((Pathcomp_t*)0,std_path(),PATH_PATH));
}

static void pathinit(void)
Expand Down

0 comments on commit e3a1dda

Please sign in to comment.