[klibc] [EXPAND] Fix ifsfirst/ifslastp leak

As it stands expandarg may return with a non-NULL ifslastp which
then confuses any subsequent ifsbreakup user that doesn't clear
it directly.

What's worse, if we get interrupted before we hit ifsfree in
expandarg we will leak memory.

This patch fixes this by always calling ifsfree in expandarg
thus ensuring that ifslastp is always NULL on the normal path.
It also adds an ifsfree call to the RESET path to ensure that
memory isn't leaked.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: maximilian attems <max@stro.at>

3 files changed, 21 insertions, 12 deletions

diff --git a/usr/dash/expand.c b/usr/dash/expand.c
index d5d11d5c39cd8..c5fb30a603b1f 100644
--- a/usr/dash/expand.c
+++ b/usr/dash/expand.c
@@ -117,7 +117,6 @@ STATIC char *evalvar(char *, int);
 STATIC size_t strtodest(const char *, const char *, int);
 STATIC void memtodest(const char *, size_t, const char *, int);
 STATIC ssize_t varvalue(char *, int, int);
-STATIC void ifsfree(void);
 STATIC void expandmeta(struct strlist *, int);
 #ifdef HAVE_GLOB
 STATIC void addglob(const glob_t *);
@@ -191,8 +190,6 @@ expandarg(union node *arg, struct arglist *arglist, int flag)
 
 	argbackq = arg->narg.backquote;
 	STARTSTACKSTR(expdest);
-	ifsfirst.next = NULL;
-	ifslastp = NULL;
 	argstr(arg->narg.text, flag);
 	p = _STPUTC('\0', expdest);
 	expdest = p - 1;
@@ -215,8 +212,7 @@ expandarg(union node *arg, struct arglist *arglist, int flag)
 		*exparg.lastp = sp;
 		exparg.lastp = &sp->next;
 	}
-	if (ifsfirst.next)
-		ifsfree();
+	ifsfree();
 	*exparg.lastp = NULL;
 	if (exparg.list) {
 		*arglist->lastp = exparg.list;
@@ -1108,22 +1104,25 @@ add:
 	arglist->lastp = &sp->next;
 }
 
-STATIC void
-ifsfree(void)
+void ifsfree(void)
 {
-	struct ifsregion *p;
+	struct ifsregion *p = ifsfirst.next;
+
+	if (!p)
+		goto out;
 
 	INTOFF;
-	p = ifsfirst.next;
 	do {
 		struct ifsregion *ifsp;
 		ifsp = p->next;
 		ckfree(p);
 		p = ifsp;
 	} while (p);
-	ifslastp = NULL;
 	ifsfirst.next = NULL;
 	INTON;
+
+out:
+	ifslastp = NULL;
 }
 
 
@@ -1678,7 +1677,6 @@ casematch(union node *pattern, char *val)
 	setstackmark(&smark);
 	argbackq = pattern->narg.backquote;
 	STARTSTACKSTR(expdest);
-	ifslastp = NULL;
 	argstr(pattern->narg.text, EXP_TILDE | EXP_CASE);
 	STACKSTRNUL(expdest);
 	result = patmatch(stackblock(), val);
@@ -1718,3 +1716,13 @@ varunset(const char *end, const char *var, const char *umsg, int varflags)
 	}
 	sh_error("%.*s: %s%s", end - var - 1, var, msg, tail);
 }
+
+#ifdef mkinit
+
+INCLUDE "expand.h"
+
+RESET {
+	ifsfree();
+}
+
+#endif
diff --git a/usr/dash/expand.h b/usr/dash/expand.h
index 2c67c57d61d35..214c172374841 100644
--- a/usr/dash/expand.h
+++ b/usr/dash/expand.h
@@ -73,6 +73,7 @@ int casematch(union node *, char *);
 void recordregion(int, int, int);
 void removerecordregions(int);
 void ifsbreakup(char *, struct arglist *);
+void ifsfree(void);
 
 /* From arith.y */
 intmax_t arith(const char *);
diff --git a/usr/dash/miscbltin.c b/usr/dash/miscbltin.c
index c6c5d55c531d5..d67fd90927d49 100644
--- a/usr/dash/miscbltin.c
+++ b/usr/dash/miscbltin.c
@@ -90,7 +90,7 @@ readcmd_handle_line(char *line, char **ap, size_t len)
 
 	ifsbreakup(s, &arglist);
 	*arglist.lastp = NULL;
-	removerecordregions(0);
+	ifsfree();
 
 	for (sl = arglist.list; sl; sl = sl->next) {
 		/* remaining fields present, but no variables left. */