Various small leaks

[Grisha Levit]

Found mostly by normal usage running a no-bash-malloc build with clang's
LeakSanitizer enabled. So far seems to provide very accurate results with
little overhead.

* arrayfunc.c
- quote_compound_array_word: make sure to free VALUE
- bind_assoc_var_internal: if assigning to a dynamic variable, make sure
  to free the key (usually assoc_insert would do it)

* bashline.c
- bash_command_name_stat_hook: free original *NAME if we are going to
  change what it points to (what the callers seem to expect)

* builtins/evalstring.c
- parse_and_execute: make sure to dispose of the parsed command
  resulting from a failed function import attempt
- open_redir_file: if we did not get a pointer to pass back the expanded
  filename, make sure to free the name

* examples/loadables/kv.c
- kvsplit: bind_assoc_variable does not free its VALUE argument so do
  not pass a copy in the call calling

* examples/loadables/stat.c
- loadstat: bind_assoc_variable does not free its VALUE argument so make
  sure to do it

* subst.c
- param_expand: free temp1 value for codepaths that don't do it
---
 arrayfunc.c   | 6 +-
 bashline.c| 1 +
 builtins/evalstring.c | 4 
 examples/loadables/kv.c   | 2 +-
 examples/loadables/stat.c | 1 +
 subst.c   | 2 ++
 6 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arrayfunc.c b/arrayfunc.c
index 28aee54b..ea7a559d 100644
--- a/arrayfunc.c
+++ b/arrayfunc.c
@@ -193,7 +193,10 @@ bind_assoc_var_internal (SHELL_VAR *entry, HASH_TABLE
*hash, char *key, const ch
   newval = make_array_variable_value (entry, 0, key, value, flags);

   if (entry->assign_func)
-(*entry->assign_func) (entry, newval, 0, key);
+{
+  (*entry->assign_func) (entry, newval, 0, key);
+  FREE (key);
+}
   else
 assoc_insert (hash, key, newval);

@@ -945,6 +948,7 @@ quote_compound_array_word (char *w, int type)
   if (t != w+ind)
free (t);
   strcpy (nword + i, value);
+  free (value);

   return nword;
 }
diff --git a/bashline.c b/bashline.c
index 6adc548e..c14d465f 100644
--- a/bashline.c
+++ b/bashline.c
@@ -1923,6 +1923,7 @@ bash_command_name_stat_hook (char **name)
   result = search_for_command (cname, 0);
   if (result)
 {
+  FREE (*name);
   *name = result;
   return 1;
 }
diff --git a/builtins/evalstring.c b/builtins/evalstring.c
index 11785de1..f4f62b2d 100644
--- a/builtins/evalstring.c
+++ b/builtins/evalstring.c
@@ -468,6 +468,8 @@ parse_and_execute (char *string, const char *from_file,
int flags)
should_jump_to_top_level = 0;
last_result = last_command_exit_value = EX_BADUSAGE;
set_pipestatus_from_exit (last_command_exit_value);
+   dispose_command(command);
+   global_command = (COMMAND *)NULL;
reset_parser ();
break;
  }
@@ -766,6 +768,8 @@ open_redir_file (REDIRECT *r, char **fnp)

   if (fnp)
 *fnp = fn;
+  else
+free (fn);
   return fd;
 }

diff --git a/examples/loadables/kv.c b/examples/loadables/kv.c
index 1dfceb6a..ee6dc7f0 100644
--- a/examples/loadables/kv.c
+++ b/examples/loadables/kv.c
@@ -71,7 +71,7 @@ kvsplit (SHELL_VAR *v, char *line, char *dstring)
   else
 value = "";

-  return (bind_assoc_variable (v, name_cell (v), savestring (key),
savestring (value), 0) != 0);
+  return (bind_assoc_variable (v, name_cell (v), savestring (key), value,
0) != 0);
 }

 int
diff --git a/examples/loadables/stat.c b/examples/loadables/stat.c
index 1093f7b0..d58f07e5 100644
--- a/examples/loadables/stat.c
+++ b/examples/loadables/stat.c
@@ -330,6 +330,7 @@ loadstat (char *vname, SHELL_VAR *var, char *fname, int
flags, char *fmt, struct
   key = savestring (arraysubs[i]);
   value = statval (i, fname, flags, fmt, sp);
   v = bind_assoc_variable (var, vname, key, value, ASS_FORCE);
+  free (value);
 }
   return 0;
 }
diff --git a/subst.c b/subst.c
index b93374b8..0ecb4cf1 100644
--- a/subst.c
+++ b/subst.c
@@ -10850,6 +10850,7 @@ comsub:
  {
chk_atstar (temp, quoted, pflags, quoted_dollar_at_p,
contains_dollar_at);
tdesc = parameter_brace_expand_word (temp, SPECIAL_VAR (temp, 0),
quoted, pflags, 0);
+   free (temp1);
if (tdesc == &expand_wdesc_error || tdesc == &expand_wdesc_fatal)
  return (tdesc);
ret = tdesc;
@@ -10862,6 +10863,7 @@ comsub:
  {
set_exit_status (EXECUTION_FAILURE);
report_error (_("%s: invalid variable name for name reference"),
temp);
+   free (temp1);
return (&expand_wdesc_error); /* XXX */
  }
else
From 892cb679195298496a8fc86c36825b20b0a8e07c Mon Sep 17 00:00:00 2001
From: Grisha Levit 
Date: Sat, 3 Jun 2023 16:51:26 -0400
Subject: [PATCH] various leaks

Found mostly by normal usage running a no-bash-malloc build with clang's
LeakSanitizer enabled. So far seems to provide very accurate results.

* arrayfunc.c
- quote_compound_array_word: make sure to free VALUE
- bind_assoc_var_internal: if assigning to a dynamic variable, make s

[PATCH] completion display interrupt leak handling

[Grisha Levit]

When readline's display of completion matches is interrupted, the match
list is only freed for the '?' completion type but not for '!' and '@'
types.  I couldn't find a specific reason for that to be the case so
hope this patch is an OK fix.

Also add freeing saved_line_buffer from rl_complete_internal to the
signal handler and freeing of matches in one spot where menu completion
doesn't do it.


* lib/readline/complete.c
- complete_sigcleanarg_t: new struct to hold _rl_complete_sigcleanup
  argument
- _rl_complete_sigcleanup: use new struct instead of *matches pointer,
  now also handles freeing saved_line
- rl_complete_internal: set up _rl_sigcleanup whenever display_matches
  is called so we no longer leak matches and saved_line_buffer when
  getting a SIGINT
- rl_menu_complete: free contents of matches instead of just pointer
  when completion-query-items exceeded
---
 lib/readline/complete.c | 61 ++---
 1 file changed, 39 insertions(+), 22 deletions(-)

diff --git a/lib/readline/complete.c b/lib/readline/complete.c
index bf7cc856..f8f08aa6 100644
--- a/lib/readline/complete.c
+++ b/lib/readline/complete.c
@@ -494,12 +494,19 @@ _rl_reset_completion_state (void)
   rl_completion_quote_character = 0;
 }

+typedef struct {
+char **matches;
+char *saved_line;
+} complete_sigcleanarg_t;
+
 static void
 _rl_complete_sigcleanup (int sig, void *ptr)
 {
+  complete_sigcleanarg_t *arg = ptr;
   if (sig == SIGINT) /* XXX - for now */
 {
-  _rl_free_match_list ((char **)ptr);
+  _rl_free_match_list (arg->matches);
+  FREE (arg->saved_line);
   _rl_complete_display_matches_interrupt = 1;
 }
 }
@@ -2008,7 +2015,8 @@ rl_complete_internal (int what_to_do)
   int start, end, delimiter, found_quote, i, nontrivial_lcd;
   char *text, *saved_line_buffer;
   char quote_char;
-  int tlen, mlen, saved_last_completion_failed;
+  int tlen, mlen, saved_last_completion_failed, do_display;
+  complete_sigcleanarg_t cleanarg;

   RL_SETSTATE(RL_STATE_COMPLETING);

@@ -2088,6 +2096,8 @@ rl_complete_internal (int what_to_do)
   if (matches && matches[0] && *matches[0])
 last_completion_failed = 0;

+  do_display = 0;
+
   switch (what_to_do)
 {
 case TAB:
@@ -2121,13 +2131,13 @@ rl_complete_internal (int what_to_do)
  {
if (what_to_do == '!')
  {
-   display_matches (matches);
+   do_display = 1;
break;
  }
else if (what_to_do == '@')
  {
if (nontrivial_lcd == 0)
- display_matches (matches);
+ do_display = 1;
break;
  }
else if (rl_editing_mode != vi_mode)
@@ -2151,23 +2161,7 @@ rl_complete_internal (int what_to_do)
append_to_match (matches[0], delimiter, quote_char, nontrivial_lcd);
break;
  }
-
-  if (rl_completion_display_matches_hook == 0)
- {
-   _rl_sigcleanup = _rl_complete_sigcleanup;
-   _rl_sigcleanarg = matches;
-   _rl_complete_display_matches_interrupt = 0;
- }
-  display_matches (matches);
-  if (_rl_complete_display_matches_interrupt)
-{
-  matches = 0; /* already freed by rl_complete_sigcleanup */
-  _rl_complete_display_matches_interrupt = 0;
-   if (rl_signal_event_hook)
- (*rl_signal_event_hook) (); /* XXX */
-}
-  _rl_sigcleanup = 0;
-  _rl_sigcleanarg = 0;
+  do_display = 1;
   break;

 default:
@@ -2180,6 +2174,29 @@ rl_complete_internal (int what_to_do)
   return 1;
 }

+  if (do_display)
+{
+  if (rl_completion_display_matches_hook == 0)
+ {
+   _rl_sigcleanup = _rl_complete_sigcleanup;
+   cleanarg.matches = matches;
+   cleanarg.saved_line = saved_line_buffer;
+   _rl_sigcleanarg = &cleanarg;
+   _rl_complete_display_matches_interrupt = 0;
+ }
+  display_matches (matches);
+  if (_rl_complete_display_matches_interrupt)
+ {
+   matches = 0; /* already freed by rl_complete_sigcleanup */
+   saved_line_buffer = 0;
+   _rl_complete_display_matches_interrupt = 0;
+   if (rl_signal_event_hook)
+ (*rl_signal_event_hook) (); /* XXX */
+ }
+  _rl_sigcleanup = 0;
+  _rl_sigcleanarg = 0;
+}
+
   _rl_free_match_list (matches);

   /* Check to see if the line has changed through all of this
manipulation. */
@@ -2864,7 +2881,7 @@ rl_menu_complete (int count, int ignore)
if (rl_completion_query_items > 0 && match_list_size >=
rl_completion_query_items)
  {
rl_ding ();
-   FREE (matches);
+   _rl_free_match_list (matches);
matches = (char **)0;
full_completion = 1;
return (0);
From 8eef36cd74caae425e536ead84b1a8cb1cad44b7 Mon Sep 17 00:00:00 2001
From: Grisha Levit 
Date: Sat, 3 Jun 2023 23:31:16 -0400
Subject: [PATCH] completion display interrupt leak handling

When readline's display of completion matches is interrupted, the match
list is only freed for the '?' completion type but not for '!' and '@'
types.  I couldn't find a specific reason for that to be the case so
hope this patch is an OK fix.

Also add freeing saved_l

Re: New array_expand_once option

[alex xmb ratchev]

On Thu, Jun 15, 2023, 20:40 alex xmb ratchev  wrote:

>
>
> On Thu, Jun 15, 2023, 15:06 Chet Ramey  wrote:
>
>> On 6/14/23 5:18 PM, alex xmb ratchev wrote:
>>
>> > [[ -v a["$subscript"] ]]
>>
>
a small question about that .. do the quotes matter ?

>
>> > is already an arithmetic expansion error in bash-5.2, but not in
>> bash-5.1.
>> >
>> >
>> > hello there ..
>> >
>> > i dont get much
>> > it d be an arith err if array wasnt -A
>> > .. ?
>>
>> Yes, because only one expansion is performed, and so the subscript is the
>> command substitution, which is not a valid arithmetic expression.
>>
>
> very good
> i like now it works like this , . the old was bugged
>
> >
>> > so i shopt -s array_expand_once
>> > and [[ -v foo["$var"] ]]
>> > .. is the right way ? ( for the change , the future )
>>
>> It's not as important for compound commands like [[, but for builtins like
>> test/[, yes, this is the way to use it.
>>
>
> alright , thanks
>
> greets
>
> --
>> ``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/
>>
>>

Re: New array_expand_once option

[Dennis Williamson]

On Sat, Jun 17, 2023, 6:59 AM alex xmb ratchev  wrote:

> On Thu, Jun 15, 2023, 20:40 alex xmb ratchev  wrote:
>
> >
> >
> > On Thu, Jun 15, 2023, 15:06 Chet Ramey  wrote:
> >
> >> On 6/14/23 5:18 PM, alex xmb ratchev wrote:
> >>
> >> > [[ -v a["$subscript"] ]]
> >>
> >
> a small question about that .. do the quotes matter ?
>
>


Try it without the quotes and with an empty or unset  variable.

>

Re: New array_expand_once option

[alex xmb ratchev]

On Sat, Jun 17, 2023, 14:46 Dennis Williamson 
wrote:

>
>
> On Sat, Jun 17, 2023, 6:59 AM alex xmb ratchev  wrote:
>
>> On Thu, Jun 15, 2023, 20:40 alex xmb ratchev  wrote:
>>
>> >
>> >
>> > On Thu, Jun 15, 2023, 15:06 Chet Ramey  wrote:
>> >
>> >> On 6/14/23 5:18 PM, alex xmb ratchev wrote:
>> >>
>> >> > [[ -v a["$subscript"] ]]
>> >>
>> >
>> a small question about that .. do the quotes matter ?
>>
>>
>
>
> Try it without the quotes and with an empty or unset  variable.
>

well .. will do .. dont have -dev yet ..
peace

>

Fwd: Crashing the Linux System

[LitHack]

-- Forwarded message -
From: LitHack 
Date: Sat, 17 Jun 2023 at 08:52
Subject: Crashing the Linux System
To: 


Running the yes command in command substitution will crash the linux shell.
According to me inside command the substitution it is creating multiple
process(fork).  Command: `yes` or $(yes)

Here is the bug report:
Bug report 

Re: Crashing the Linux System

[alex xmb ratchev]

On Sat, Jun 17, 2023, 15:06 LitHack  wrote:

> -- Forwarded message -
> From: LitHack 
> Date: Sat, 17 Jun 2023 at 08:52
> Subject: Crashing the Linux System
> To: 
>
>
> Running the yes command in command substitution will crash the linux shell.
> According to me inside command the substitution it is creating multiple
> process(fork).  Command: `yes` or $(yes)
>

maybe it outmaxes the mem limit ( its endless yes\n text ) and mix with
many procs result in bad ..
forkbomb style
try setting some ulimit s , .. im not much aware of them .. -u says user
processes , -T says threads max , ..

the forkbomb

:() { : | : ; } ; :

crashes sys es .. unproportionally unrespondingly slow


Here is the bug report:
> Bug report 
>

Re: Crashing the Linux System

[Lawrence Velázquez]

> On Jun 16, 2023, at 11:39 PM, LitHack  wrote:
> 
> -- Forwarded message -
> From: LitHack 
> Date: Sat, 17 Jun 2023 at 08:52
> Subject: Crashing the Linux System
> To: 
> 
> 
> Running the yes command in command substitution will crash the linux shell.
> According to me inside command the substitution it is creating multiple
> process(fork).  Command: `yes` or $(yes)
> 
> Here is the bug report:
> Bug report 


You already posted this exact "bug report" to zsh-work...@zsh.org,
and the answer you got there applies here too: This is user error
and not a bug.


Begin forwarded message:

> From: Bart Schaefer 
> Subject: Re: Crashing the Linux System
> Date: June 17, 2023 at 12:43:02 AM EDT
> To: LitHack 
> Cc: zsh-work...@zsh.org
> 
> On Fri, Jun 16, 2023 at 8:38 PM LitHack  wrote:
>> 
>> Running the yes command in command substitution will crash the linux shell. 
>> According to me inside command the substitution it is creating multiple 
>> process(fork).  Command: `yes` or $(yes)
> 
> The "yes" command is defined to produce an unending stream of output.
> The $(...) substitution is defined to capture all the output from a
> command and substitute it as a string.  "All the output" of "yes" is
> impossible to capture in finite memory.  The error I get from zsh is
> the expected one:
> zsh: fatal error: out of heap memory
> There definitely are not multiple forks happening.
> 
> This is not a bug except in the sense that it was user error to use
> $(yes) in the first place.  It's no different than deliberately
> writing an infinite loop such as $(while true; do echo y; done).



-- 
vq

Re: Fwd: Crashing the Linux System

[Greg Wooledge]

On Sat, Jun 17, 2023 at 09:09:23AM +0530, LitHack wrote:
> Running the yes command in command substitution will crash the linux shell.

Not surprising, really.  You're trying to capture and store an
infinite number of bytes in memory.  Eventually, either the malloc is
going to fail due to lack of virtual memory, or the Linux OOM (Out Of
Memory) Killer is going to step in and start killing random processes.

> According to me inside command the substitution it is creating multiple
> process(fork).  Command: `yes` or $(yes)

The command substitution only creates one new process, but that process
will try to consume an infinite amount of memory.

Re: New array_expand_once option

[alex xmb ratchev]

On Tue, Jun 13, 2023, 22:21 Chet Ramey  wrote:

> The latest push to the devel branch extends the assoc_expand_once
> semantics to indexed array variables. This means that a construct
> like
>
> export subscript='$(uname >&2 ; echo 0)'
> shopt -s assoc_expand_once
> printf -v a["$subscript"] %s hi
> declare -p a
>
> will no longer run `uname' and assign "hi" to subscript 0. This affects
> shell builtins that take array references as arguments, which undergo one
> round of word expansion before the builtin performs the assignment.
>
> The affected builtins are primarily
>
> printf
> read
> wait
>
> since those assign values to variable names passed as arguments.
>
> The option also affects
>
> declare
> let
> local
> typeset
> test
> [
> unset
>
> For example, the following does not run `uname':
>
> export subscript='$(uname >&2 ; echo 0)'
> shopt -s assoc_expand_once
> let a["$subscript"]+=1
>
> To better describe its functionality, I renamed assoc_expand_once to
> array_expand_once. The old name is still accepted.
>

its an opt that have manually to be enabled ?
or is by default on

When the next version of bash is released, the old bash-5.2 behavior (that
> runs `uname' in the above examples) will be available if the shell
> compatibility level is set to 52 or lower.
>
> 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/
>
>