All of lore.kernel.org
 help / color / mirror / Atom feed
* Crash in expandarg
@ 2024-09-13 22:05 Johannes Altmanninger
  2024-11-17  2:25 ` [PATCH] expand: Fix scanleft/right for !FNMATCH_IS_ENABLED && quotes Herbert Xu
  0 siblings, 1 reply; 3+ messages in thread
From: Johannes Altmanninger @ 2024-09-13 22:05 UTC (permalink / raw)
  To: dash

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] expand: Fix scanleft/right for !FNMATCH_IS_ENABLED && quotes
  2024-09-13 22:05 Crash in expandarg Johannes Altmanninger
@ 2024-11-17  2:25 ` Herbert Xu
  2024-11-17  6:55   ` Johannes Altmanninger
  0 siblings, 1 reply; 3+ messages in thread
From: Herbert Xu @ 2024-11-17  2:25 UTC (permalink / raw)
  To: Johannes Altmanninger; +Cc: dash

Johannes Altmanninger <aclopte@gmail.com> wrote:
> 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")
Reported-by: Johannes Altmanninger <aclopte@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/src/expand.c b/src/expand.c
index d73f29c..7a30648 100644
--- a/src/expand.c
+++ b/src/expand.c
@@ -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;
 }
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] expand: Fix scanleft/right for !FNMATCH_IS_ENABLED && quotes
  2024-11-17  2:25 ` [PATCH] expand: Fix scanleft/right for !FNMATCH_IS_ENABLED && quotes Herbert Xu
@ 2024-11-17  6:55   ` Johannes Altmanninger
  0 siblings, 0 replies; 3+ messages in thread
From: Johannes Altmanninger @ 2024-11-17  6:55 UTC (permalink / raw)
  To: Herbert Xu; +Cc: dash

On Sun, Nov 17, 2024 at 10:25:03AM +0800, Herbert Xu wrote:
> Johannes Altmanninger <aclopte@gmail.com> wrote:
> > 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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-11-17  6:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-13 22:05 Crash in expandarg Johannes Altmanninger
2024-11-17  2:25 ` [PATCH] expand: Fix scanleft/right for !FNMATCH_IS_ENABLED && quotes Herbert Xu
2024-11-17  6:55   ` Johannes Altmanninger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.