New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tracking issue for the various test failures under ASan #518
Labels
Comments
The buffer overflow in expand.sh is fixed as follows: expand.sh failure patch v1diff --git a/src/cmd/ksh93/sh/lex.c b/src/cmd/ksh93/sh/lex.c
index db0cefcff..1be19c596 100644
--- a/src/cmd/ksh93/sh/lex.c
+++ b/src/cmd/ksh93/sh/lex.c
@@ -1187,7 +1187,7 @@ int sh_lex(Lex_t* lp)
if(lp->lex.reservok && state[n]==S_BREAK && isfirst)
break;
#if SHOPT_BRACEPAT
- if(sh_isoption(SH_BRACEEXPAND) && c==LBRACE && !assignment && state[n]!=S_BREAK
+ if(sh_isoption(SH_BRACEEXPAND) && c==LBRACE && !assignment && n>0 && state[n]!=S_BREAK
&& !lp->lex.incase && !lp->lex.intest
&& !lp->lex.skipword)
{
Come to think of it, since the rest of the code really does nothing in that case until the expand.sh failure patch v2diff --git a/src/cmd/ksh93/sh/lex.c b/src/cmd/ksh93/sh/lex.c
index db0cefcff..5af15b49d 100644
--- a/src/cmd/ksh93/sh/lex.c
+++ b/src/cmd/ksh93/sh/lex.c
@@ -1181,7 +1181,7 @@ int sh_lex(Lex_t* lp)
break;
if(n>0)
fcseek(-LEN);
- else if(lp->lex.reservok)
+ else
break;
/* check for reserved word { or } */
if(lp->lex.reservok && state[n]==S_BREAK && isfirst) No regression test failures are caused by either patch on my end. edit: The code can be further simplified like this: expand.sh failure patch v3diff --git a/src/cmd/ksh93/sh/lex.c b/src/cmd/ksh93/sh/lex.c
index db0cefcff..4d0ad60be 100644
--- a/src/cmd/ksh93/sh/lex.c
+++ b/src/cmd/ksh93/sh/lex.c
@@ -1175,14 +1175,12 @@ int sh_lex(Lex_t* lp)
goto do_reg;
}
isfirst = (lp->lexd.first&&fcseek(0)==lp->lexd.first+1);
- fcgetc(n);
+ if(fcgetc(n)<=0)
+ break;
/* check for {} */
if(c==LBRACE && n==RBRACE)
break;
- if(n>0)
- fcseek(-LEN);
- else if(lp->lex.reservok)
- break;
+ fcseek(-LEN);
/* check for reserved word { or } */
if(lp->lex.reservok && state[n]==S_BREAK && isfirst)
break; |
McDutchie
added a commit
that referenced
this issue
Aug 19, 2022
This macro expansion in lex.c may assign -1 to n if EOF is reached: 1178: fcgetc(n); As a result, n may be -1 when this code is reached: 1190: if(sh_isoption(SH_BRACEEXPAND) && c==LBRACE && !assignment && state[n]!=S_BREAK 'state[n]' is a buffer overflow if n==-1. src/cmd/ksh93/sh/lex.c: sh_lex(): case S_BRACE: - Apart from the buffer overflow, if n<=0, none of the code following fcget(n) does anything until 'break' on line 1199 is reached. So, if fcget(n) yields <=0, just break. This allows some code simplification. Progresses: #518
McDutchie
added a commit
that referenced
this issue
Aug 19, 2022
This macro expansion in lex.c may assign -1 to n if EOF is reached: 1178: fcgetc(n); As a result, n may be -1 when this code is reached: 1190: if(sh_isoption(SH_BRACEEXPAND) && c==LBRACE && !assignment && state[n]!=S_BREAK 'state[n]' is a buffer overflow if n==-1. src/cmd/ksh93/sh/lex.c: sh_lex(): case S_BRACE: - Apart from the buffer overflow, if n<=0, none of the code following fcget(n) does anything until 'break' on line 1199 is reached. So, if fcget(n) yields <=0, just break. This allows some code simplification. Progresses: #518
McDutchie
added
help wanted
Extra attention is needed
regressfail
Regression test failure
labels
Aug 24, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently,
shtests
counts 12 errors under ASan when the regression tests are run with theASAN_OPTIONS
variable set todetect_leaks=0
. Below is a regression test log from running the tests under ASan (last updated 2022-09-30):ASan test results
I should note that the
pty
tests freeze under ASan, so in order to get those tests to finish the frozenpty
process was killed withSIGKILL
. Additionally, if ASan's memory leak detection is left enabled, then there are many more test failures due to a multitude of small memory leaks.The text was updated successfully, but these errors were encountered: