Skip to contentSkip to conversation

Crash in expandarg

PATCH In review Johannes Altmanninger 2 participants 3 messages Sep 13, 2024 – Nov 17, 2024

lore
Share & tools
lore
#0 lore raw

I'm hitting an easily reproducible crash.
It bisects to c5bf970 (expand: Add multi-byte support to pmatch, 2024-06-02).

After bisecting I reduced it to this example (probably not minimal)

	echo \\ | dash -c 'foo=$(cat; printf .); foo=${foo%.}'

stacktrace from gdb:

	Program terminated with signal SIGSEGV, Segmentation fault.
	#0  0x000061d927e57bbd in expandarg (arg=arg@entry=0x61d927e6dce00000, arglist=arglist@entry=0x7fff2ea3bbe0, flag=flag@entry=4) at expand.c:228
	228		argbackq = arg->narg.backquote;
	(gdb) bt
	#0  0x000061d927e57bbd in expandarg (arg=arg@entry=0x61d927e6dce00000, arglist=arglist@entry=0x7fff2ea3bbe0, flag=flag@entry=4) at expand.c:228
	#1  0x000061d927e53168 in evalcommand (cmd=0x61d927e6dd10 <stackbase+400>, flags=1) at eval.c:865
	#2  0x000061d927e522e7 in evaltree (n=0x61d927e6dd10 <stackbase+400>, flags=1) at eval.c:305
	#3  0x000061d927e522e7 in evaltree (n=0x61d927e6dd10 <stackbase+400>, n@entry=0x61d927e6dd30 <stackbase+432>, flags=1) at eval.c:305
	#4  0x000061d927e52cf3 in evalstring (s=0x61d927e6db88 <stackbase+8> "foo=$(cat; printf .); foo=${foo%.}", flags=1) at eval.c:190
	#5  0x000061d927e506f5 in main (argc=3, argv=0x7fff2ea3bec8) at main.c:176
#1 lore raw

PATCH 1/1 expand: Fix scanleft/right for !FNMATCH_IS_ENABLED && quotes

Quoted 6 lines · Johannes Altmanninger

I'm hitting an easily reproducible crash.
It bisects to c5bf970 (expand: Add multi-byte support to pmatch, 2024-06-02).

After bisecting I reduced it to this example (probably not minimal)

       echo \\ | dash -c 'foo=$(cat; printf .); foo=${foo%.}'

Thanks for the report. This patch should fix the problem:

---8<---
When our own pmatch is used, loc2 is unused in scanleft/right when quotes is true. However, it is still needed when quotes is false.

Fix the scanleft/right code so that loc2 is always updated (so it will be garbage when quotes is true) but only returned depending on the value of quotes.

Fixes c5bf9702ea11 ("expand: Add multi-byte support to pmatch")
src/expand.c +6 -11
index d73f29c..7a30648 100644
@@ -621,18 +621,15 @@ static char *scanleft(char *startp, char *endp, char *rmesc, char *rmescend,
 		match = pmatch(str, s);
 		*(FNMATCH_IS_ENABLED ? loc2 : loc) = c;
 		if (match)
-			return FNMATCH_IS_ENABLED && quotes ? loc : loc2;
+			return quotes ? loc : loc2;
 
 		if (!c)
 			break;
 
 		mb = mbnext(loc);
 		loc += (mb & 0xff) + (mb >> 8);
-		if (unlikely(FNMATCH_IS_ENABLED || !quotes)) {
-			ml = (mb >> 8) > 3 ? (mb >> 8) - 2 : 1;
-			loc2 += ml;
-		} else
-			loc2 = loc;
+		ml = (mb >> 8) > 3 ? (mb >> 8) - 2 : 1;
+		loc2 += ml;
 	} while (1);
 	return 0;
 }
@@ -645,8 +642,7 @@ static char *scanright(char *startp, char *endp, char *rmesc, char *rmescend,
 	char *loc;
 	char *loc2;
 
-	for (loc = endp, loc2 = rmescend;;
-	     FNMATCH_IS_ENABLED ? loc2-- : (loc2 = loc)) {
+	for (loc = endp, loc2 = rmescend;; loc2--) {
 		char *s = FNMATCH_IS_ENABLED ? loc2 : loc;
 		char c = *s;
 		unsigned ml;
@@ -659,7 +655,7 @@ static char *scanright(char *startp, char *endp, char *rmesc, char *rmescend,
 		match = pmatch(str, s);
 		*(FNMATCH_IS_ENABLED ? loc2 : loc) = c;
 		if (match)
-			return FNMATCH_IS_ENABLED && quotes ? loc : loc2;
+			return quotes ? loc : loc2;
 		if (--loc < startp)
 			break;
 		if (!esc--)
@@ -676,8 +672,7 @@ static char *scanright(char *startp, char *endp, char *rmesc, char *rmescend,
 		loc -= ml + 2;
 		if (*loc == (char)CTLESC)
 			loc--;
-		if (FNMATCH_IS_ENABLED)
-			loc2 -= ml - 1;
+		loc2 -= ml - 1;
 	}
 	return 0;
 }
Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#2 lore raw

Re: [PATCH] expand: Fix scanleft/right for !FNMATCH_IS_ENABLED && quotes

Quoted 9 lines · Herbert Xu
Quoted 6 lines · Johannes Altmanninger

I'm hitting an easily reproducible crash.
It bisects to c5bf970 (expand: Add multi-byte support to pmatch, 2024-06-02).

After bisecting I reduced it to this example (probably not minimal)

       echo \\ | dash -c 'foo=$(cat; printf .); foo=${foo%.}'

Thanks for the report. This patch should fix the problem:

thanks this works for me. I've been using master for the trap fix.