Skip to contentSkip to conversation

Free IFS state after here document expansion

PATCH In review Jonathan Nieder 2 participants 2 messages Mar 13, 2011 – Mar 15, 2011 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Jonathan Nieder

author
#1 lore raw

PATCH 1/1 Free IFS state after here document expansion

Here's another bug bisecting to f42e443bb ([EXPAND] Fix ifsfirst/ifslastp leak, 2010-09-08). It was found with the following test case, based on the configure script for Tracker:

dash -x -c '
<<-_ACEOF
$@
_ACEOF
exec
' - abcdefgh
+
+ exec �a
exec: 1: : Permission denied

The missing ifsfree call is in expandarg when it returns to openhere during here document expansion.

Reported-by Aurelien Jarno <[email protected]>
Signed-off-by Jonathan Nieder <[email protected]>

---
Hi,

Aurelien Jarno wrote[1]:

Quoted 1 line

https://buildd.debian.org/fetch.cgi?pkg=tracker&arch=amd64&ver=0.10.3-1&stamp=1299968124&file=log&as=raw

[...]

Quoted 2 lines
|  57145 dash     CALL  execve(0x806bf64,0x806ba64,0x806bef4)
|  57145 dash     NAMI  "/usr/local/bincd$e,f4g<hDnLoTpu"

This patch seems to fix it. Thoughts?

[1] http://bugs.debian.org/618023

 src/expand.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
src/expand.c +1 -0
index 7a9b157..eac6c6d 100644
@@ -194,6 +194,7 @@ expandarg(union node *arg, struct arglist *arglist, int flag)
 	p = _STPUTC('\0', expdest);
 	expdest = p - 1;
 	if (arglist == NULL) {
+		ifsfree();
 		return;			/* here document expanded */
 	}
 	p = grabstackstr(p);
Signature · 1 line
1.7.4.1
#2 lore raw
Quoted 14 lines · Jonathan Nieder

Here's another bug bisecting to f42e443bb ([EXPAND] Fix ifsfirst/ifslastp leak, 2010-09-08). It was found with the following test case, based on the configure script for Tracker:

dash -x -c '
<<-_ACEOF
$@
_ACEOF
exec
' - abcdefgh
+
+ exec �a
exec: 1: : Permission denied

Quoted 5 lines

The missing ifsfree call is in expandarg when it returns to openhere during here document expansion.

Reported-by Aurelien Jarno <[email protected]>
Signed-off-by Jonathan Nieder <[email protected]>

Applied with minor rearrangement. Thanks!

Signature · 30 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
commit a58d301405d96fb8ff87cffa3cabe3b110214a31
Author: Jonathan Nieder <[email protected]>
Date:   Tue Mar 15 16:01:34 2011 +0800

    [EXPAND] Free IFS state after here document expansion
    
    Here's another bug bisecting to f42e443bb ([EXPAND] Fix
    ifsfirst/ifslastp leak, 2010-09-08).  It was found with the following
    test case, based on the configure script for Tracker:
    
    	dash -x -c '
    		<<-_ACEOF
    		$@
    		_ACEOF
    		exec
    	' - abcdefgh
    	+
    	+ exec   �a
    	exec: 1: : Permission denied
    
    The missing ifsfree call is in expandarg when it returns to openhere
    during here document expansion.
    
    Reported-by: Aurelien Jarno <[email protected]>
    Signed-off-by: Jonathan Nieder <[email protected]>
    Signed-off-by: Herbert Xu <[email protected]>
ChangeLog +4 -0
index 44e5bdf..5163479 100644
@@ -1,3 +1,7 @@
+2011-03-15  Jonathan Nieder <[email protected]>
+
+	* Free IFS state after here document expansion.
+
 2011-03-15  Harald van Dijk <[email protected]>
 
 	* Let funcnode refer to a function definition, not its first command.
src/expand.c +5 -2
index f155ea0..ce60fe9 100644
@@ -194,7 +194,8 @@ expandarg(union node *arg, struct arglist *arglist, int flag)
 	p = _STPUTC('\0', expdest);
 	expdest = p - 1;
 	if (arglist == NULL) {
-		return;			/* here document expanded */
+		/* here document expanded */
+		goto out;
 	}
 	p = grabstackstr(p);
 	exparg.lastp = &exparg.list;

@@ -212,12 +213,14 @@ expandarg(union node *arg, struct arglist *arglist, int flag)
 		*exparg.lastp = sp;
 		exparg.lastp = &sp->next;
 	}
-	ifsfree();
 	*exparg.lastp = NULL;
 	if (exparg.list) {
 		*arglist->lastp = exparg.list;
 		arglist->lastp = exparg.lastp;
 	}
+
+out:
+	ifsfree();
 }