Re: read reads from stale buffer after interrupt

[Chet Ramey]

On 5/28/24 1:53 AM, Oğuz wrote:

See:

$ while read; do :; done 

Thanks for the report. Since this occurs when read(2) returns a partial
buffer on an interrupt, I think we can handle it in read_builtin().

--
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: [PATCH] bind_assoc_variable: free key if cannot assign

[Chet Ramey]

On 5/30/24 5:30 PM, Grisha Levit wrote:

Avoid leaking expansion of `x' in `declare -Ar A; A[x]='


Thanks for the report. I think a more appropriate place to fix this is
assign_array_element_internal(), since that's where the key is allocated.

Chet

--
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: read reads from stale buffer after interrupt

[Oğuz]

On Friday, May 31, 2024, Chet Ramey  wrote:
>
> Thanks for the report. Since this occurs when read(2) returns a partial
> buffer on an interrupt, I think we can handle it in read_builtin().
>

Okay, handle how though? Leave the file offset at the last byte read before
the interrupt or where the last successful read command left it?


-- 
Oğuz

Re: read reads from stale buffer after interrupt

[Chet Ramey]

On 5/31/24 11:57 AM, Oğuz wrote:
On Friday, May 31, 2024, Chet Ramey > wrote:


Thanks for the report. Since this occurs when read(2) returns a partial
buffer on an interrupt, I think we can handle it in read_builtin().


Okay, handle how though? Leave the file offset at the last byte read before 
the interrupt or where the last successful read command left it?


Reinitialize the buffer state before we longjmp. Since we were interrupted,
we're going to abandon this invocation of read, discarding what we read.

--
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



OpenPGP_signature.asc
Description: OpenPGP digital signature

[PATCH] cond expr: cleanup on errors

[Grisha Levit]

Two minor leak fixes for conditional command error conditions:

If a WORD token is read when COND_AND, COND_OR, COND_END, or a binary
operator are expected, the allocated WORD_DESC is leaked.

If a conditional command has a syntax error, the allocated COMMAND is
leaked.
---
 parse.y | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/parse.y b/parse.y
index 1349480b..d39d6737 100644
--- a/parse.y
+++ b/parse.y
@@ -3525,6 +3525,7 @@ read_token (int command)
   if (cond_token != COND_END)
{
  cond_error ();
+ dispose_command (yylval.command);
  return (-1);
}
   token_to_read = COND_END;
@@ -5003,6 +5004,9 @@ cond_skip_newlines (void)
 #define COND_RETURN_ERROR() \
   do { cond_token = COND_ERROR; return ((COND_COM *)NULL); } while (0)
 
+#define COND_TERM_DONE() \
+  do { if (cond_skip_newlines () == WORD) dispose_word (yylval.word); } while 
(0)
+
 static COND_COM *
 cond_term (void)
 {
@@ -5037,12 +5041,12 @@ cond_term (void)
  COND_RETURN_ERROR ();
}
   term = make_cond_node (COND_EXPR, (WORD_DESC *)NULL, term, (COND_COM 
*)NULL);
-  (void)cond_skip_newlines ();
+  COND_TERM_DONE ();
 }
   else if (tok == BANG || (tok == WORD && (yylval.word->word[0] == '!' && 
yylval.word->word[1] == '\0')))
 {
   if (tok == WORD)
-   dispose_word (yylval.word); /* not needed */
+   dispose_word (yylval.word);
   term = cond_term ();
   if (term)
term->flags ^= CMD_INVERT_RETURN;
@@ -5069,7 +5073,7 @@ cond_term (void)
  COND_RETURN_ERROR ();
}
 
-  (void)cond_skip_newlines ();
+  COND_TERM_DONE ();
 }
   else if (tok == WORD)/* left argument to binary operator */
 {
@@ -5118,6 +5122,8 @@ cond_term (void)
  else
parser_error (line_number, _("conditional binary operator 
expected"));
  dispose_cond_node (tleft);
+ if (tok == WORD)
+   dispose_word (yylval.word);
  COND_RETURN_ERROR ();
}
 
@@ -5153,7 +5159,7 @@ cond_term (void)
  COND_RETURN_ERROR ();
}
 
-  (void)cond_skip_newlines ();
+  COND_TERM_DONE ();
 }
   else
 {
-- 
2.45.1

[PATCH] coproc: do not leak name

[Grisha Levit]

When a named coproc is created, the name string and associated WORD_DESC
are leaked.
---
 parse.y | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/parse.y b/parse.y
index d39d6737..fe5038be 100644
--- a/parse.y
+++ b/parse.y
@@ -1103,6 +1103,7 @@ coproc:   COPROC shell_command
{
  $$ = make_coproc_command ($2->word, $3);
  $$->flags |= CMD_WANT_SUBSHELL|CMD_COPROC_SUBSHELL;
+ dispose_word ($2);
}
|   COPROC WORD shell_command redirection_list
{
@@ -1120,6 +1121,7 @@ coproc:   COPROC shell_command
tc->redirects = $4;
  $$ = make_coproc_command ($2->word, $3);
  $$->flags |= CMD_WANT_SUBSHELL|CMD_COPROC_SUBSHELL;
+ dispose_word ($2);
}
|   COPROC simple_command
{
-- 
2.45.1

[PATCH] expand_word_internal: fix leak with W_NOSPLIT2

[Grisha Levit]

Free temporary list allocated when exapnding `$@' in

bash -c 'IFS=:; : ${_+$@}' _ X
---
 subst.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/subst.c b/subst.c
index 3faa4068..c56d2434 100644
--- a/subst.c
+++ b/subst.c
@@ -12144,6 +12144,7 @@ finished_with_string:
 return, we expect to be able to split the results, but the
 space separation means the right split doesn't happen. */
  tword->word = string_list (list); 
+ dispose_words (list);
}
  else
tword->word = istring;
-- 
2.45.1

[PATCH] exec: free args on failed exec

[Grisha Levit]

The comment describing why this wasn't done has been there since the
start of the repo's history and AFAICT it is not accurate anymore, as
shell_execve only calls realloc when it's going to longjmp rather than
return.

Fixes leak in

bash -O execfail -c 'exec /; :'
---
 builtins/exec.def | 4 
 1 file changed, 4 deletions(-)

diff --git a/builtins/exec.def b/builtins/exec.def
index 3ca7c4f8..618882c8 100644
--- a/builtins/exec.def
+++ b/builtins/exec.def
@@ -230,10 +230,6 @@ exec_builtin (WORD_LIST *list)
   exit_value = shell_execve (command, args, env);
   opt = errno;
 
-  /* We have to set this to NULL because shell_execve has called realloc()
- to stuff more items at the front of the array, which may have caused
- the memory to be freed by realloc().  We don't want to free it twice. */
-  args = (char **)NULL;
   if (cleanenv == 0)
 adjust_shell_level (1);
 
-- 
2.45.1