Re: bash support for XDG Base Directory spec (~/.config/bash/)

[Siteshwar Vashisht]

On Fri, May 7, 2021 at 11:48 AM Allison Karlitskaya <
allison.karlitsk...@redhat.com> wrote:

> hello,
>
> Please consider these two patches for inclusion in bash to support
> storing shell initialisation files (profile, bashrc) in a subdirectory
> of ~/.config/ as most programs do these days.
>
> I'm happy to make changes to address any feedback.
>
> Even if you'd prefer not to apply the second patch, applying the first
> patch is a nice cleanup and would make it easier for distributions
> such as Fedora to apply the second patch for themselves.
>
> Thank you very much for your consideration,
>

This issue is also discussed in Red Hat bug[1]. I will leave few notes
about it here.

>From the XDG specification[2]:

- $XDG_CONFIG_HOME defines the base directory relative to which
user-specific configuration files should be stored. If $XDG_CONFIG_HOME is
either not set or empty, a default equal to $HOME/.config should be used.

- $XDG_CONFIG_DIRS defines the preference-ordered set of base directories
to search for configuration files in addition to the $XDG_CONFIG_HOME base
directory. The directories in $XDG_CONFIG_DIRS should be seperated with a
colon ':'. If $XDG_CONFIG_DIRS is either not set or empty, a value equal to
/etc/xdg should be used.

- The order of base directories denotes their importance; the first
directory listed is the most important. When the same information is
defined in multiple places the information defined relative to the more
important base directory takes precedent. The base directory defined by
$XDG_DATA_HOME is considered more important than any of the base
directories defined by $XDG_DATA_DIRS. The base directory defined by
$XDG_CONFIG_HOME is considered more important than any of the base
directories defined by $XDG_CONFIG_DIRS.

- Default configuration files should be installed to
$sysconfdir/xdg/subdir/filename with $sysconfdir defaulting to /etc.

Proposed patch doesn't fully implement this specification:

- It doesn't cover the case when `$XDG_CONFIG_HOME` is defined.
`$HOME/.config` should be only used if `$XDG_CONFIG_HOME` is not defined.
(It's actually mentioned in the patch commit).

- It doesn't cover the case when `$XDG_CONFIG_DIRS` environment variable is
not defined.

- It doesn't cover the case when `$XDG_CONFIG_DIRS` environment variable is
defined.

So even if we want to follow XDG specification, proposed patch has only
partial implementation of it. Also, if it gets merged, it may require a
change from package maintainers side to provide default configuration files
under `$sysconfdir/xdg/subdir/filename`.

On a side note, I believe bash startup configurations are already confusing
for an average user and following XDG specification will just make it more
complicated.


> Allison
>

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1959284
[2]
https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html

read builtin goes into uninterruptible loop if input only contains zeros

[Siteshwar Vashisht]

Configuration Information [Automatically generated, do not change]:
Machine: x86_64
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64' 
-DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='x86_64-unknown-linux-gnu' 
-DCONF_VENDOR='unknown' -DLOCALEDIR='/usr/local/share/locale' -DPACKAGE='bash' 
-DSHELL -DHAVE_CONFIG_H   -I.  -I. -I./include -I./lib   -g -O0 
-Wno-parentheses -Wno-format-security
uname output: Linux localhost.localdomain 4.11.6-201.fc25.x86_64 #1 SMP Tue Jun 
20 20:21:11 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
Machine Type: x86_64-unknown-linux-gnu

Bash Version: 4.4
Patch Level: 12
Release Status: release

Description:
read builtin goes into uninterruptible loop if input comes from 
/dev/zero.

Repeat-By:
read -N 1 _ < /dev/zero

Fix:
Attached patch fixes the bug.


-- 
--
Siteshwar Vashisht
From 8d49a26b2bf9c150d23526da0999d8d4fa38a4f7 Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht 
Date: Fri, 28 Jul 2017 15:51:07 +0200
Subject: [PATCH] Make read builtin interruptible if input contains zeros

---
 builtins/read.def | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/builtins/read.def b/builtins/read.def
index 33821f3..de23ab0 100644
--- a/builtins/read.def
+++ b/builtins/read.def
@@ -609,7 +609,7 @@ read_builtin (list)
 	  break;
 	}
 
-  CHECK_ALRM;
+  check_signals();
 
 #if defined (READLINE)
 	}
@@ -662,7 +662,10 @@ read_builtin (list)
 	break;
 
   if (c == '\0' && delim != '\0')
-	continue;		/* skip NUL bytes in input */
+	{
+	  internal_warning ("read_builtin: ignored null byte in input");
+	  continue;		/* skip NUL bytes in input */
+	}
 
   if ((skip_ctlesc == 0 && c == CTLESC) || (skip_ctlnul == 0 && c == CTLNUL))
 	{
-- 
2.9.4

Re: read builtin goes into uninterruptible loop if input only contains zeros

[Siteshwar Vashisht]

Modified the patch to print warning only once when null byte is ignored.

- Original Message -
> From: "Siteshwar Vashisht" 
> To: bug-bash@gnu.org
> Sent: Friday, July 28, 2017 3:59:15 PM
> Subject: read builtin goes into uninterruptible loop if input only contains   
> zeros
> 
> Configuration Information [Automatically generated, do not change]:
> Machine: x86_64
> OS: linux-gnu
> Compiler: gcc
> Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64'
> -DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='x86_64-unknown-linux-gnu'
> -DCONF_VENDOR='unknown' -DLOCALEDIR='/usr/local/share/locale'
> -DPACKAGE='bash' -DSHELL -DHAVE_CONFIG_H   -I.  -I. -I./include -I./lib   -g
> -O0 -Wno-parentheses -Wno-format-security
> uname output: Linux localhost.localdomain 4.11.6-201.fc25.x86_64 #1 SMP Tue
> Jun 20 20:21:11 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
> Machine Type: x86_64-unknown-linux-gnu
> 
> Bash Version: 4.4
> Patch Level: 12
> Release Status: release
> 
> Description:
> read builtin goes into uninterruptible loop if input comes from
> /dev/zero.
> 
> Repeat-By:
> read -N 1 _ < /dev/zero
> 
> Fix:
> Attached patch fixes the bug.
> 
> 
> --
> --
> Siteshwar Vashisht
> 

-- 
--
Siteshwar Vashisht
From 87f9e379ef4618d434ebc376d7d63a98ad620273 Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht 
Date: Fri, 28 Jul 2017 15:51:07 +0200
Subject: [PATCH] Make read builtin interruptible if input contains zeros

---
 builtins/read.def | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtins/read.def b/builtins/read.def
index 33821f3..4865c36 100644
--- a/builtins/read.def
+++ b/builtins/read.def
@@ -194,6 +194,7 @@ read_builtin (list)
   struct stat tsb;
   SHELL_VAR *var;
   TTYSTRUCT ttattrs, ttset;
+  int nullbyte;
 #if defined (ARRAY_VARS)
   WORD_LIST *alist;
 #endif
@@ -243,6 +244,7 @@ read_builtin (list)
   nr = nchars = input_is_tty = input_is_pipe = unbuffered_read = have_timeout = 0;
   delim = '\n';		/* read until newline */
   ignore_delim = 0;
+  nullbyte = 0;
 
   reset_internal_getopt ();
   while ((opt = internal_getopt (list, "ersa:d:i:n:p:t:u:N:")) != -1)
@@ -609,7 +611,7 @@ read_builtin (list)
 	  break;
 	}
 
-  CHECK_ALRM;
+  check_signals();
 
 #if defined (READLINE)
 	}
@@ -662,7 +664,14 @@ read_builtin (list)
 	break;
 
   if (c == '\0' && delim != '\0')
-	continue;		/* skip NUL bytes in input */
+	{
+	  if (nullbyte == 0)
+	  {
+	internal_warning ("read_builtin: ignored null byte in input");
+	nullbyte = 1;
+	  }
+	  continue;		/* skip NUL bytes in input */
+	}
 
   if ((skip_ctlesc == 0 && c == CTLESC) || (skip_ctlnul == 0 && c == CTLNUL))
 	{
-- 
2.9.4

Re: read builtin goes into uninterruptible loop if input only contains zeros

[Siteshwar Vashisht]

- Original Message -
> From: "Chet Ramey" 
> To: "Siteshwar Vashisht" , bug-bash@gnu.org
> Cc: "chet ramey" 
> Sent: Sunday, July 30, 2017 4:33:09 PM
> Subject: Re: read builtin goes into uninterruptible loop if input only 
> contains zeros
> 
> Thanks for the report. This was fixed at the end of June, and the fix is in
> the devel branch.

Thanks for pointing to the fix, however I see that the fix is missing a warning 
about ignoring null character. You might want to add it.

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

-- 
--
Siteshwar Vashisht

Cleanup compiler warnings

[Siteshwar Vashisht]

Hello,

Attached patch cleans up compiler warnings about unused variables, functions 
etc. while compiling with '-Wall' option in gcc.

-- 
--
Siteshwar Vashisht
From 83394f684ab0b5451c351e37411e8cab3c4ed42a Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht 
Date: Sun, 30 Jul 2017 16:03:51 +0200
Subject: [PATCH] Cleanup compiler warnings

---
 array.c  |  1 -
 arrayfunc.c  |  6 +---
 bashline.c   | 39 +
 braces.c | 10 +++---
 builtins/bind.def|  1 -
 builtins/cd.def  | 14 ++--
 builtins/command.def |  5 ++-
 builtins/declare.def |  5 ++-
 builtins/evalfile.c  |  2 +-
 builtins/help.def|  9 +++--
 builtins/history.def |  6 +++-
 builtins/mapfile.def |  3 +-
 builtins/setattr.def |  2 +-
 builtins/shopt.def   |  2 +-
 builtins/ulimit.def  |  5 ++-
 execute_cmd.c| 20 +--
 expr.c   |  8 +++--
 general.c| 19 +-
 jobs.c   | 17 -
 locale.c | 13 +++
 parse.y  | 56 +++---
 pathexp.c|  5 ++-
 pcomplete.c  |  3 +-
 print_cmd.c  | 10 +++---
 redir.c  | 16 ++---
 shell.c  |  7 ++--
 subst.c  | 98 
 support/man2html.c   | 76 
 trap.c   |  6 ++--
 variables.c  | 23 +---
 xmalloc.c|  2 ++
 31 files changed, 234 insertions(+), 255 deletions(-)

diff --git a/array.c b/array.c
index 34f022e..5f7d72f 100644
--- a/array.c
+++ b/array.c
@@ -384,7 +384,6 @@ array_remove_quoted_nulls(array)
 ARRAY	*array;
 {
 	ARRAY_ELEMENT	*a;
-	char	*t;
 
 	if (array == 0 || array_head(array) == 0 || array_empty(array))
 		return (ARRAY *)NULL;
diff --git a/arrayfunc.c b/arrayfunc.c
index ac4884f..5b41f86 100644
--- a/arrayfunc.c
+++ b/arrayfunc.c
@@ -262,9 +262,6 @@ bind_assoc_variable (entry, name, key, value, flags)
  char *value;
  int flags;
 {
-  SHELL_VAR *dentry;
-  char *newval;
-
   if ((readonly_p (entry) && (flags&ASS_FORCE) == 0) || noassign_p (entry))
 {
   if (readonly_p (entry))
@@ -285,7 +282,7 @@ assign_array_element (name, value, flags)
 {
   char *sub, *vname;
   int sublen;
-  SHELL_VAR *entry, *nv;
+  SHELL_VAR *entry;
 
   vname = array_variable_name (name, (flags & ASS_NOEXPAND) != 0, &sub, &sublen);
 
@@ -458,7 +455,6 @@ expand_compound_array_assignment (var, value, flags)
  int flags;
 {
   WORD_LIST *list, *nlist;
-  WORD_LIST *hd, *tl, *t, *n;
   char *val;
   int ni;
 
diff --git a/bashline.c b/bashline.c
index e156ef8..fb01e22 100644
--- a/bashline.c
+++ b/bashline.c
@@ -175,11 +175,14 @@ static char *quote_word_break_chars __P((char *));
 static void set_filename_bstab __P((const char *));
 static char *bash_quote_filename __P((char *, int, char *));
 
+#ifdef INCLUDE_UNUSED
 #ifdef _MINIX
 static void putx __P((int));
 #else
 static int putx __P((int));
 #endif
+#endif
+
 static int bash_execute_unix_command __P((int, int));
 static void init_unix_command_map __P((void));
 static int isolate_sequence __P((char *, int, int, int *));
@@ -1049,7 +1052,7 @@ bash_forward_shellword (count, key)
  int count, key;
 {
   size_t slen;
-  int sindex, c, p;
+  int c, p;
   DECLARE_MBSTATE;
 
   if (count < 0)
@@ -1158,7 +1161,7 @@ bash_backward_shellword (count, key)
  int count, key;
 {
   size_t slen;
-  int sindex, c, p;
+  int c, p;
   DECLARE_MBSTATE;
 
   if (count < 0)
@@ -1416,7 +1419,7 @@ attempt_shell_completion (text, start, end)
  const char *text;
  int start, end;
 {
-  int in_command_position, ti, saveti, qc, dflags;
+  int in_command_position, ti, qc, dflags;
   char **matches, *command_separator_chars;
 #if defined (PROGRAMMABLE_COMPLETION)
   int have_progcomps, was_assignment;
@@ -1438,7 +1441,7 @@ attempt_shell_completion (text, start, end)
  appears after a character that separates commands.  It cannot be a
  command word if we aren't at the top-level prompt. */
   ti = start - 1;
-  saveti = qc = -1;
+  qc = -1;
 
   while ((ti > -1) && (whitespace (rl_line_buffer[ti])))
 ti--;
@@ -1449,7 +1452,7 @@ attempt_shell_completion (text, start, end)
   if (ti >= 0 && (rl_line_buffer[ti] == '"' || rl_line_buffer[ti] == '\''))
 {
   qc = rl_line_buffer[ti];
-  saveti = ti--;
+  ti--;
   while (ti > -1 && (whitespace (rl_line_buffer[ti])))
 	ti--;
 }
@@ -1799,7 +1802,7 @@ command_word_completion_function (hint_text, state)
   static char *dequoted_hint = (char *)NULL;
   static char *directory_part = (char *)NULL;
   static char **glob_matches = (char **)NULL;
-  static int path_index, hint_len, dequoted_len, istate, igncase;
+  static int path_index, hint_len, istate, igncase;
   static int mapping_over, local_index, searching_path, hint_is_dir;

Integer overflow in command substitution

[Siteshwar Vashisht]

Machine: x86_64
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64' 
-DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='x86_64-redhat-linux-gnu' 
-DCONF_VENDOR='redhat' -DLOCALEDIR='/usr/share/locale' -DPACKAGE='bash' -DSHELL 
-DHAVE_CONFIG_H   -I.  -I. -I./include -I./lib  -D_GNU_SOURCE -DRECYCLES_PIDS 
-DDEFAULT_PATH_VALUE='/usr/local/bin:/usr/bin'  -O2 -g -pipe -Wall 
-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
-fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches 
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic 
-Wno-parentheses -Wno-format-security
uname output: Linux localhost.localdomain 4.13.12-200.fc26.x86_64 #1 SMP Wed 
Nov 8 16:47:26 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
Machine Type: x86_64-redhat-linux-gnu

Bash Version: 4.4
Patch Level: 12
Release Status: release

Repeat-By:
$ bash -c 'true $(yes )'
bash: xrealloc: cannot allocate 18446744071562067968 bytes

Fix:
Attached patch fixes this issue.

-- 
--
Siteshwar Vashisht
From a91b113be8fca1a38b2b7f67be11039f3efd44e3 Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht 
Date: Thu, 16 Nov 2017 12:18:00 +0100
Subject: [PATCH] Avoid integer overflow while allocating memory in
 read_comsub() function


diff --git a/subst.c b/subst.c
index eb855e9d..e48524e5 100644
--- a/subst.c
+++ b/subst.c
@@ -5803,7 +5803,8 @@ read_comsub (fd, quoted, flags, rflag)
  int *rflag;
 {
   char *istring, buf[128], *bufp, *s;
-  int istring_index, istring_size, c, tflag, skip_ctlesc, skip_ctlnul;
+  size_t istring_size, istring_index;
+  int c, tflag, skip_ctlesc, skip_ctlnul;
   ssize_t bufn;
   int nullbyte;
 
-- 
2.13.6

What should be the expected behavior for $_ ?

[Siteshwar Vashisht]

According to bash reference manual[1]:

($_, an underscore.) At shell startup, set to the absolute pathname used to 
invoke the shell or shell script being executed as passed in the environment or 
argument list. Subsequently, expands to the last argument to the previous 
command, after expansion. Also set to the full pathname used to invoke each 
command executed and placed in the environment exported to that command. When 
checking mail, this parameter holds the name of the mail file. 

Now consider following example (with bash-completion package installed):

$ cd /tmp/
$ touch rpmall.txt rpmshort.txt
$ mkdir testdir
$ cp rpmall.txt rpmshort.txt $_ # Use tab completion to complete filenames
cp: target '_filedir' is not a directory

Last command fails because tab completing 'cp' command modifies value of '$_'. 
Shall value of '$_' be modified if a command gets executed in background ?

[1] 
https://www.gnu.org/software/bash/manual/html_node/Special-Parameters.html#Special-Parameters

-- 
--
Siteshwar Vashisht

Re: Bash patches format

[Siteshwar Vashisht]

- Original Message -
> From: "Marty E. Plummer" 
> To: "Chet Ramey" 
> Cc: bash-annou...@gnu.org, bug-bash@gnu.org
> Sent: Thursday, May 31, 2018 3:08:57 AM
> Subject: Re: Bash patches format
> 
> Well, as I said, debian and fedora convert to -p1 unified, as current
> debian packages using the quilt 3.0 format expect -p1; I'm unsure as to
> the rationale for fedora doing it.

I converted them so that I can use autosetup[1] macro, it allows automatic 
patching in spec files. Other than bash upstream patches, all the patches were 
applied through '-p1', so I had to convert upstream patches.

> And gentoo expects -p1 by default for
> patches applied by eapply (though you can specify -p0 in the ebuild, it
> is a deviation from the defaults).
> 
> I don't believe debian actually needs a unified diff, but quilt 3.0
> requires -p1 (see
> https://lists.debian.org/debian-devel/2009/03/msg00368.html).
> 

[1] https://src.fedoraproject.org/rpms/bash/blob/master/f/bash.spec#_125
-- 
--
Siteshwar Vashisht

Bash fails to exit if parent ssh process is killed during command search

[Siteshwar Vashisht]

Configuration Information [Automatically generated, do not change]:
Machine: x86_64
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64' 
-DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='x86_64-redhat-linux-gnu' 
-DCONF_VENDOR='redhat' -DLOCALEDIR='/usr/share/locale' -DPACKAGE='bash' -DSHELL 
-DHAVE_CONFIG_H   -I.  -I. -I./include -I./lib  -D_GNU_SOURCE -DRECYCLES_PIDS 
-DDEFAULT_PATH_VALUE='/usr/local/bin:/usr/bin'  -O2 -g -pipe -Wall 
-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
-fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches 
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic 
-fasynchronous-unwind-tables -Wno-parentheses -Wno-format-security
uname output: Linux localhost.localdomain 4.16.13-200.fc27.x86_64 #1 SMP Wed 
May 30 15:03:53 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Machine Type: x86_64-redhat-linux-gnu

Bash Version: 4.4
Patch Level: 19
Release Status: release

Description:
Bash fails to exit and shows 100% cpu usage if parent sshd process is 
killed while performing command search.

Repeat-By:

1. ssh to localhost with bash as default shell.
2. In the ssh session press 'ESC -p', this will show ':' on command line.
3. From a different terminal, check parent process id for bash running in ssh 
session by executing 'ps fax'. For e.g. on my machine it is 9825:

9822 ?Ss 0:00  \_ sshd: situ [priv]
9825 ?S  0:00  \_ sshd: situ@pts/8
9834 pts/8Ss+0:00  \_ -bash

4. Kill the parent process of bash by executing:

kill -hup 9825

5. Bash gets stuck in a loop and shows 100% cpu usage.

Fix:
See attached patch. Credits to Paulo Andrade  for 
finding and fixing the bug.


-- 
--
Siteshwar Vashisht
diff --git a/lib/readline/search.c b/lib/readline/search.c
--- a/lib/readline/search.c
+++ b/lib/readline/search.c
@@ -367,7 +367,7 @@ noninc_search (dir, pchar)
 {
   c = _rl_search_getchar (cxt);
 
-  if (c == 0)
+  if (c <= 0)
 	break;
 
   r = _rl_nsearch_dispatch (cxt, c);

Re: What should be the expected behavior for $_ ?

[Siteshwar Vashisht]

- Original Message -
> From: "L A Walsh" 
> To: "chet ramey" 
> Cc: bug-bash@gnu.org, "Siteshwar Vashisht" 
> Sent: Wednesday, April 4, 2018 2:03:05 AM
> Subject: Re: What should be the expected behavior for $_ ?
> 
> 
> 
> Chet Ramey wrote:
> > On 4/3/18 10:03 AM, Siteshwar Vashisht wrote:
> >   
> >> $ mkdir testdir
> >> $ cp rpmall.txt rpmshort.txt $_ # Use tab completion to complete filenames
> >> cp: target '_filedir' is not a directory
> >>
> >> Last command fails because tab completing 'cp' command modifies value of
> >> '$_'. Shall value of '$_' be modified if a command gets executed in
> >> background ?
> >> 
> Well -- two things -- 1, "use tab completion to complete filenames" --
> WHAT filenames?  Did you really mean to look for other files?  Usually
> '$_' is rather ephemeral -- doing anything between the last command
> and the use of '$_', would seem to be perilous if you want to to make
> use of '$_'.  And 2nd -- it's not really the case that command completion
> is done in background (exactly), but more "behind the scenes".
> 
> Seems more like you are making an argument for not relying on the value
> of '$_' in interactive use.  Maybe in a script -- where interactive
> things might not be happening butas more automations get added
> to your shell (whether from bash or some addon package), using $_
> could possibly lose its value in other ways.
> 
> 
> * Maybe yet-another-option -- to have '$_' be equal to the last arg
> of the last command executed in the same "context" -- i.e. if
> interactive, then from the last interactive command, or if in a script,
> from last arg of previously executed line...
> 
> I'd prefer to see that^^, than to change the current behavior, as I'd
> be too concerned about unforeseen consequences, though I'm not sure
> how common the problem is vs. work involved in changing it.
> 
> >
> > It's an interesting question. You want $_ to expand to the last argument
> > (or last word) of the previous history entry when the shell is interactive,
> > which is available as !$, instead of the last command executed by the
> > current shell instance.
> >
> > Should the command line know about shell functions and commands executed in
> > the foreground on its behalf? What should the behavior be in a
> > non-interactive shell? What do folks think?

Can we at least document this behavior in man page if we can't change it ?

> >   
> 
> Suppose some shell func/cmd executed deliberately changes the value of
> $_ for some reason?  I can't think of a good reason why, off hand, but
> others might have a better imagination ;-)  Is there a common
> entry point for something like bash_completion to save such a var if
> it wants to (I don't know).
> 
> Besides, even relying on '$_' being the last word of '!$'
> wouldn't work when histexpand is off or disabled -- I have too many
> surprises with '!' expansion.
> 
> 
> 
> 
> 
> 

-- 
--
Siteshwar Vashisht

Race condition in handling SIGHUP

[Siteshwar Vashisht]

Hello, 

Recently we came across a bug in bash which was introduced by below patch : 

http://git.savannah.gnu.org/cgit/bash.git/commit/?id=d5d00961 

In bash 4.2 this could lead to a race condition. 'yy_readline_get ()' function 
sets 'terminate_immediately' to 1 before calling readline() at [1]. 

If shell receives SIGHUP and executes 'termsig_handler ()' before setting 
'terminate_immediately' back to 0 [2], we will end up at [3] (considering 
'RL_ISSTATE (RL_STATE_READCMD)' evaluates to 0 when readline is not waiting to 
read command from user), and ~/.bash_history will not be updated. 

We started seeing this bug after above mentioned patch was backported to RHEL 
6. Sometimes ~/.bash_history is not updated when user executes 'reboot' command 
in a ssh session. This is caused by race condition in handling SIGHUP. 

While this issue was fixed by backporting somes changes (See attached patch) 
from [4] to bash-4.2 or older versions, there is still a corner case which may 
cause race condition in handling SIGHUP in current upstream. 

'bash_tilde_expand ()' function sets 'terminate_immediately' to 1 at [5] 

If SIGHUP is received and termsig_handler () gets executed before reaching [6], 
~/.bash_history will not be updated. This can happen in a scenario where user 
is running ssh session and requests for expansion for '~', if an admin issues 
'reboot' command at the same time then ~/.bash_history may not be updated. 

I have 2 questions about how current upstream handles this condition : 

1. Why 'terminate_immediately' is set to 1 at [7]? 

2. Why 'RL_ISSTATE (RL_STATE_READCMD)' is being checked at [8]? This will 
evaluate to 0 if readline is not waiting to read a command from user. I believe 
this check can be removed. 

 
[1] http://git.savannah.gnu.org/cgit/bash.git/tree/parse.y?id=bash-4.2#n1441 
[2] http://git.savannah.gnu.org/cgit/bash.git/tree/parse.y?id=bash-4.2#n1446 
[3] 
http://git.savannah.gnu.org/cgit/bash.git/tree/sig.c?id=d5d0096115d0d484fd669ad170498962ea45e841#n513
 
[4] 
http://git.savannah.gnu.org/cgit/bash.git/commit/?id=ac50fbac377e32b98d2de396f016ea81e8ee9961
 
[5] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n994 
[6] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n1004 
[7] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n994 
[8] http://git.savannah.gnu.org/cgit/bash.git/tree/sig.c#n524 

-- 
Siteshwar Vashisht 
Software Engineer 
From 1ecc0ff050d74aef548677fb97b2113b0920c711 Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht 
Date: Mon, 25 Apr 2016 14:04:10 +0530
Subject: [PATCH] Do not set terminate_immediately while reading input

---
 builtins/read.def | 3 +--
 parse.y   | 5 +
 y.tab.c   | 5 +
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/builtins/read.def b/builtins/read.def
index 1ef9142..2dcaf35 100644
--- a/builtins/read.def
+++ b/builtins/read.def
@@ -455,7 +455,6 @@ read_builtin (list)
  of the unwind-protect stack after the realloc() works right. */
   add_unwind_protect (xfree, input_string);
   interrupt_immediately++;
-  terminate_immediately++;
 
   unbuffered_read = (nchars > 0) || (delim != '\n') || input_is_pipe;
 
@@ -512,6 +511,7 @@ read_builtin (list)
 
   if (retval <= 0)
 	{
+	  CHECK_TERMSIG;
 	  eof = 1;
 	  break;
 	}
@@ -616,7 +616,6 @@ add_char:
 zsyncfd (fd);
 
   interrupt_immediately--;
-  terminate_immediately--;
   discard_unwind_frame ("read_builtin");
 
   retval = eof ? EXECUTION_FAILURE : EXECUTION_SUCCESS;
diff --git a/parse.y b/parse.y
index 9a78d0c..0b9f9ff 100644
--- a/parse.y
+++ b/parse.y
@@ -1440,12 +1440,11 @@ yy_readline_get ()
 	  old_sigint = (SigHandler *)set_signal_handler (SIGINT, sigint_sighandler);
 	  interrupt_immediately++;
 	}
-  terminate_immediately = 1;
 
   current_readline_line = readline (current_readline_prompt ?
   	  current_readline_prompt : "");
 
-  terminate_immediately = 0;
+  CHECK_TERMSIG;
   if (signal_is_ignored (SIGINT) == 0 && old_sigint)
 	{
 	  interrupt_immediately--;
@@ -1606,13 +1605,11 @@ yy_stream_get ()
   if (interactive)
 	{
 	  interrupt_immediately++;
-	  terminate_immediately++;
 	}
   result = getc_with_restart (bash_input.location.file);
   if (interactive)
 	{
 	  interrupt_immediately--;
-	  terminate_immediately--;
 	}
 }
   return (result);
diff --git a/y.tab.c b/y.tab.c
index d702554..563d312 100644
--- a/y.tab.c
+++ b/y.tab.c
@@ -3757,12 +3757,11 @@ yy_readline_get ()
 	  old_sigint = (SigHandler *)set_signal_handler (SIGINT, sigint_sighandler);
 	  interrupt_immediately++;
 	}
-  terminate_immediately = 1;
 
   current_readline_line = readline (current_readline_prompt ?
   	  current_readline_prompt : "");
 
-  terminate_immediately = 0;
+ 

Race condition in handling SIGHUP

[Siteshwar Vashisht]

Hello, 

Recently we came across a bug in bash which was introduced by below patch : 

http://git.savannah.gnu.org/cgit/bash.git/commit/?id=d5d00961 

In bash 4.2 this could lead to a race condition. 'yy_readline_get ()' function 
sets 'terminate_immediately' to 1 before calling readline() at [1]. 

If shell receives SIGHUP and executes 'termsig_handler ()' before setting 
'terminate_immediately' back to 0 [2], we will end up at [3] (considering 
'RL_ISSTATE (RL_STATE_READCMD)' evaluates to 0 when readline is not waiting to 
read command from user), and ~/.bash_history will not be updated. 

We started seeing this bug after above mentioned patch was backported to RHEL 
6. Sometimes ~/.bash_history is not updated when user executes 'reboot' command 
in a ssh session. This is caused by race condition in handling SIGHUP. 

While this issue was fixed by backporting somes changes (See attached patch) 
from [4] to bash-4.2 or older versions, there is still a corner case which may 
cause race condition in handling SIGHUP in current upstream. 

'bash_tilde_expand ()' function sets 'terminate_immediately' to 1 at [5] 

If SIGHUP is received and termsig_handler () gets executed before reaching [6], 
~/.bash_history will not be updated. This can happen in a scenario where user 
is running ssh session and requests for expansion for '~', if an admin issues 
'reboot' command at the same time then ~/.bash_history may not be updated. 

I have 2 questions about how current upstream handles this condition : 

1. Why 'terminate_immediately' is set to 1 at [7]? 

2. Why 'RL_ISSTATE (RL_STATE_READCMD)' is being checked at [8]? This will 
evaluate to 0 if readline is not waiting to read a command from user. I believe 
this check can be removed. 

 
[1] http://git.savannah.gnu.org/cgit/bash.git/tree/parse.y?id=bash-4.2#n1441 
[2] http://git.savannah.gnu.org/cgit/bash.git/tree/parse.y?id=bash-4.2#n1446 
[3] 
http://git.savannah.gnu.org/cgit/bash.git/tree/sig.c?id=d5d0096115d0d484fd669ad170498962ea45e841#n513
 
[4] 
http://git.savannah.gnu.org/cgit/bash.git/commit/?id=ac50fbac377e32b98d2de396f016ea81e8ee9961
 
[5] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n994 
[6] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n1004 
[7] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n994 
[8] http://git.savannah.gnu.org/cgit/bash.git/tree/sig.c#n524 



-- 

-- 

From 1ecc0ff050d74aef548677fb97b2113b0920c711 Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht 
Date: Mon, 25 Apr 2016 14:04:10 +0530
Subject: [PATCH] Do not set terminate_immediately while reading input

---
 builtins/read.def | 3 +--
 parse.y   | 5 +
 y.tab.c   | 5 +
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/builtins/read.def b/builtins/read.def
index 1ef9142..2dcaf35 100644
--- a/builtins/read.def
+++ b/builtins/read.def
@@ -455,7 +455,6 @@ read_builtin (list)
  of the unwind-protect stack after the realloc() works right. */
   add_unwind_protect (xfree, input_string);
   interrupt_immediately++;
-  terminate_immediately++;
 
   unbuffered_read = (nchars > 0) || (delim != '\n') || input_is_pipe;
 
@@ -512,6 +511,7 @@ read_builtin (list)
 
   if (retval <= 0)
 	{
+	  CHECK_TERMSIG;
 	  eof = 1;
 	  break;
 	}
@@ -616,7 +616,6 @@ add_char:
 zsyncfd (fd);
 
   interrupt_immediately--;
-  terminate_immediately--;
   discard_unwind_frame ("read_builtin");
 
   retval = eof ? EXECUTION_FAILURE : EXECUTION_SUCCESS;
diff --git a/parse.y b/parse.y
index 9a78d0c..0b9f9ff 100644
--- a/parse.y
+++ b/parse.y
@@ -1440,12 +1440,11 @@ yy_readline_get ()
 	  old_sigint = (SigHandler *)set_signal_handler (SIGINT, sigint_sighandler);
 	  interrupt_immediately++;
 	}
-  terminate_immediately = 1;
 
   current_readline_line = readline (current_readline_prompt ?
   	  current_readline_prompt : "");
 
-  terminate_immediately = 0;
+  CHECK_TERMSIG;
   if (signal_is_ignored (SIGINT) == 0 && old_sigint)
 	{
 	  interrupt_immediately--;
@@ -1606,13 +1605,11 @@ yy_stream_get ()
   if (interactive)
 	{
 	  interrupt_immediately++;
-	  terminate_immediately++;
 	}
   result = getc_with_restart (bash_input.location.file);
   if (interactive)
 	{
 	  interrupt_immediately--;
-	  terminate_immediately--;
 	}
 }
   return (result);
diff --git a/y.tab.c b/y.tab.c
index d702554..563d312 100644
--- a/y.tab.c
+++ b/y.tab.c
@@ -3757,12 +3757,11 @@ yy_readline_get ()
 	  old_sigint = (SigHandler *)set_signal_handler (SIGINT, sigint_sighandler);
 	  interrupt_immediately++;
 	}
-  terminate_immediately = 1;
 
   current_readline_line = readline (current_readline_prompt ?
   	  current_readline_prompt : "");
 
-  terminate_immediately = 0;
+  CHECK_TERMSIG;
   if (

Re: Race condition in handling SIGHUP

[Siteshwar Vashisht]

- Original Message -
> From: "Chet Ramey" 
> To: "Siteshwar Vashisht" , bug-bash@gnu.org
> Cc: dkas...@redhat.com, "chet ramey" 
> Sent: Thursday, April 28, 2016 6:19:17 PM
> Subject: Re: Race condition in handling SIGHUP
> 
> On 4/27/16 6:04 AM, Siteshwar Vashisht wrote:
> 
> > While this issue was fixed by backporting somes changes (See attached
> > patch) from [4]  to bash-4.2 or older versions, there is still a corner
> > case which may cause race condition in handling SIGHUP in current upstream.
> > 
> > 'bash_tilde_expand ()' function sets 'terminate_immediately' to 1 at [5]
> > 
> > If SIGHUP is received and termsig_handler () gets executed before reaching
> > [6], ~/.bash_history will not be updated. This can happen in a scenario
> > where user is running ssh session and requests for expansion for '~', if an
> > admin issues 'reboot' command at the same time then ~/.bash_history may not
> > be updated.
> > 
> > I have 2 questions about how current upstream handles this condition :
> > 
> > 1. Why 'terminate_immediately' is set to 1 at [7]?
> 
> Because systems using a networked password database can hang at a priority
> that doesn't interrupt the system call when a SIGHUP arrives.  The handler
> gets run, but the system call gets restarted.  Setting
> terminate_immediately was a way to get around that.
> 
> That's probably not as necessary as it once was, so we can try removing
> that code from bash_tilde_expand.
> 
> > 2. Why 'RL_ISSTATE (RL_STATE_READCMD)' is being checked at [8]? This will
> > evaluate to 0 if readline is not waiting to read a command from user. I
> > believe this check can be removed.
> 
> The checks have to handle all the places terminate_immediately is being
> set.  However, you need to notice how terminate_immediately can be set if
> a terminating signal arrives twice before it's handled.  You don't want to
> try and save the history, which involves memory allocation and multiple
> system and C library calls, from a signal handler context.
> 
> That code is written the way it is to accommodate the much more common
> case of users exiting a shell by hitting the `close' button on their
> terminal window, which causes the terminal emulator to send one or more
> SIGHUPs to the shell process, usually while readline is active.  You want
> shell to try and save the history in this case, since that's what users
> expect.

if (interactive_shell == 0 || interactive == 0 || (sig != SIGHUP && sig != 
SIGTERM) || no_line_editing || (RL_ISSTATE (RL_STATE_READCMD) == 0))

This condition means if shell is waiting for a command to finish execution 
(readline is not waiting for user input) and the terminating signal arrives 
more than once (may be through hitting 'close' button in terminal) 
~/.bash_history will not be saved.

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

-- 
--
Siteshwar Vashisht
Software Engineer

Re: Autocompletion problems with nounset and arrays

[Siteshwar Vashisht]

- Original Message -
> From: "Chet Ramey" 
> To: "Eric Pruitt" , bug-bash@gnu.org, 
> b...@packages.debian.org
> Cc: "chet ramey" 
> Sent: Tuesday, April 26, 2016 11:24:12 PM
> Subject: Re: Autocompletion problems with nounset and arrays
> 
> Thanks for the report.  I will fix this before bash-4.4 is released.

I have built bash from devel branch and I am still able to reproduce this bug. 
Also, this bug is reproducible if I try to complete path names instead of 
arrays.

# ls /home/situ/Desbash: !ref: unbound variable

I can only relate one change from devel branch with this bug, however it does 
not handle all the cases :

@@ -3140,7 +3147,12 @@ bash_filename_stat_hook (dirname)
   if (should_expand_dirname)  
 {
   new_dirname = savestring (local_dirname);
+  /* no error messages, and expand_prompt_string doesn't longjmp so we 
don't
+have to worry about restoring this setting. */
+  global_nounset = unbound_vars_is_error;
+  unbound_vars_is_error = 0;
   wl = expand_prompt_string (new_dirname, 0, W_NOCOMSUB);  /* does the 
right thing */
+  unbound_vars_is_error = global_nounset;
   if (wl)

nounset should be turned off for all completions and not just for one specific 
case.

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

-- 
--
Siteshwar Vashisht

Re: bug in [ -f file ] test

[Siteshwar Vashisht]

- Original Message -
> From: "László Házy" 
> To: "Greg Wooledge" 
> Cc: bug-bash@gnu.org
> Sent: Thursday, July 28, 2016 2:18:55 AM
> Subject: Re: bug in [ -f file ] test
> 
> I had disabled SELinux, and got the same results. So it is not SELinux.

How did you turn off SELinux ? Was it turned off before bash started ? I would 
suggest you to boot with selinux=0 kernel parameter and check if this issue 
reproduces.

I had investigated similar bug couple of months back where MLS policies were 
affecting behavior of file permission checks. You can read the bug report 
here[1]. faccessat() function in glibc does not correctly emulate AT_EACCESS 
and AT_SYMLINK_NOFOLLOW flags and there is a bug[2] filed against glibc to fix 
this behavior. While the issue I investigated was specific to root user, it's 
possible it might be affecting non-root users too.


> I am afraid Chet did not set the permissions to /home/user1 as per the
> command list I had given. As I said before, it does not affect cd, ls
> nor cat on my system since /home/user1 and file have the required
> permissions for the group.
> 

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1329691#c0
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1333764

-- 
--
Siteshwar Vashisht

Bash crashes while handling very long string in parameter expansion

[Siteshwar Vashisht]

Configuration Information [Automatically generated, do not change]:
Machine: x86_64
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64' 
-DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='x86_64-unknown-linux-gnu' 
-DCONF_VENDOR='unknown' -DLOCALEDIR='/usr/local/share/locale' -DPACKAGE='bash' 
-DSHELL -DHAVE_CONFIG_H -DDEBUG -DMALLOC_DEBUG -I.  -I. -I./include -I./lib   
-g -O2 -Wno-parentheses -Wno-format-security
uname output: Linux localhost.localdomain 4.7.0-0.rc7.git4.1.fc25.x86_64 #1 SMP 
Mon Jul 18 15:59:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
Machine Type: x86_64-unknown-linux-gnu

Bash Version: 4.4
Patch Level: 0
Release Status: rc2

Description:
Bash crashes while handling very long string in parameter expansion.

Repeat-By:
Configure bash to compile with '--with-bash-malloc=no' parameter and 
install it :

> ./configure --with-bash-malloc=no

Generate file with very long string by executing below commands :

> for i in $(seq 0 1023); do echo -n .; done > data1k
> for i in $(seq 0 1023); do cat data1k; done > data1m
> for i in $(seq 0 1023); do cat data1m; done > data1g

Script to reproduce the crash :

> cat test.sh
#!/bin/bash

_INPUT_LOG_FILE=$1

echo "Starting..."

CMD="cat ${_INPUT_LOG_FILE}"
OUT=`${CMD} 2>&1`

echo "${CMD} completed..."

echo "Command Output : ${CMD} ${OUT}" > /dev/null

exit 1

    Execute the script :

/usr/local/bin/bash test.sh data1g

Result:
Bash crashes with segmentation fault

-- 
--
Siteshwar Vashisht

Re: Bash crashes while handling very long string in parameter expansion

[Siteshwar Vashisht]

- Original Message -
> From: "Chet Ramey" 
> To: "Siteshwar Vashisht" , bug-bash@gnu.org
> Cc: "chet ramey" 
> Sent: Tuesday, August 9, 2016 7:43:54 PM
> Subject: Re: Bash crashes while handling very long string in parameter
> expansion
> 
> You exceed the hard resource limit for your data segment size, and either
> the kernel kills the process or malloc fails and xmalloc() aborts the
> process.  If malloc fails and returns 0, the shell will attempt to print
> an explanatory message.  If that's not happening, the kernel is killing it.

Bash is not crashing due to exhausting system limits or by kernel OOM killer. 
This bug reproduces on systems that have more than 4 GB RAM. 

Bash uses signed int variable to store return value of strlen() function.  
While doing parameter expansion when string length goes beyond signed int 
limits, it causes a crash with below backtrace :

(gdb) bt
#0  __memmove_sse2_unaligned_erms () at 
../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:364
#1  0x00455a4a in sub_append_string (
source=0x7ffef75de010 
"\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001."...,
 target=0x74aad0 "\001C\001o\001m\001m\001a\001n\001d\001 
\001O\001u\001t\001p\001u\001t\001 :\001 \001c\001a\001t\001 
\001d\001a\001t\001a\001\061\001g\001 ", 
indx=0x7fffdd30, size=0x7fffdd34) at subst.c:722
#2  0x00467bd0 in expand_word_internal (word=0x74cdc0, quoted=1, 
isexp=0, contains_dollar_at=0x7fffded0, expanded_something=0x0) at 
subst.c:9068
#3  0x00468db6 in expand_word_internal (word=0x74d0a0, quoted=0, 
isexp=0, contains_dollar_at=0x7fffe078, expanded_something=0x7fffe07c) 
at subst.c:9387
#4  0x0046ac21 in shell_expand_word_list (tlist=0x74d080, eflags=31) at 
subst.c:10490
#5  0x0046af10 in expand_word_list_internal (list=0x74a7c0, eflags=31) 
at subst.c:10613
#6  0x0046a137 in expand_words (list=0x74a7c0) at subst.c:10135
#7  0x0043d7ff in execute_simple_command (simple_command=0x74a7a0, 
pipe_in=-1, pipe_out=-1, async=0, fds_to_close=0x74cc60) at execute_cmd.c:4153
#8  0x00437a4e in execute_command_internal (command=0x74cd70, 
asynchronous=0, pipe_in=-1, pipe_out=-1, fds_to_close=0x74cc60) at 
execute_cmd.c:802
#9  0x00437030 in execute_command (command=0x74cd70) at 
execute_cmd.c:405
#10 0x00422045 in reader_loop () at eval.c:180
#11 0x0041fd8a in main (argc=3, argv=0x7fffe458, 
env=0x7fffe478) at shell.c:792

(gdb) frame 1
#1  0x00455a4a in sub_append_string (
source=0x7ffef75de010 
"\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001.\001."...,
 target=0x74aad0 "\001C\001o\001m\001m\001a\001n\001d\001 
\001O\001u\001t\001p\001u\001t\001 :\001 \001c\001a\001t\001 
\001d\001a\001t\001a\001\061\001g\001 ", 
indx=0x7fffdd30, size=0x7fffdd34) at subst.c:722
722   FASTCOPY (source, target + *indx, srclen);


(gdb) l 713,722
713
714   srclen = STRLEN (source);
715   if (srclen >= (int)(*size - *indx))
716 {
717   n = srclen + *indx;
718   n = (n + DEFAULT_ARRAY_SIZE) - (n % DEFAULT_ARRAY_SIZE);
719   target = (char *)xrealloc (target, (*size = n));
720 }
721
722   FASTCOPY (source, target + *indx, srclen);

(gdb) p srclen
$4 = -2147483648

I can see that bash uses signed int variables through out it's code to store 
return value of strlen() function, so such issues may crop up at other places 
too.

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

-- 
--
Siteshwar Vashisht

Bash leaks heredoc fd to child processes

[Siteshwar Vashisht]

Hello,

Bash leaks heredoc fd to child processes if heredoc string contains command 
substitution.

Reproducer code :

$ cat test.sh 
#!/bin/bash

cat <From 6b7970ee787cf042182f8f93bf25c6e6453a8aef Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht 
Date: Tue, 17 Jan 2017 10:28:34 +0100
Subject: [PATCH] Do not leak heredoc fd to child processes

---
 lib/sh/tmpfile.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/sh/tmpfile.c b/lib/sh/tmpfile.c
index e41e45b..15fcb5c 100644
--- a/lib/sh/tmpfile.c
+++ b/lib/sh/tmpfile.c
@@ -42,7 +42,7 @@
 extern int errno;
 #endif
 
-#define BASEOPENFLAGS	(O_CREAT | O_TRUNC | O_EXCL | O_BINARY)
+#define BASEOPENFLAGS	(O_CREAT | O_TRUNC | O_EXCL | O_BINARY | O_CLOEXEC)
 
 #define DEFAULT_TMPDIR		"."	/* bogus default, should be changed */
 #define DEFAULT_NAMEROOT	"shtmp"
@@ -195,7 +195,7 @@ sh_mktmpfd (nameroot, flags, namep)
 
 #ifdef USE_MKSTEMP
   sprintf (filename, "%s/%s.XX", tdir, lroot);
-  fd = mkstemp (filename);
+  fd = mkostemp (filename, O_CLOEXEC);
   if (fd < 0 || namep == 0)
 {
   free (filename);
-- 
2.9.3

Re: Bash leaks heredoc fd to child processes

[Siteshwar Vashisht]

- Original Message -
> From: "John McKown" 
> To: "Chester Ramey" 
> Cc: bug-bash@gnu.org, "Siteshwar Vashisht" 
> Sent: Tuesday, January 17, 2017 5:14:44 PM
> Subject: Re: Bash leaks heredoc fd to child processes
> 
> ​Probably a part of LVM (Logical Volume Manager),
> 
> VS(8)
> System Manager's Manual
>  PVS(8)
> 
> NAME
>pvs — report information about physical volumes

Yes, it's the utility used in my reproducer. I used it because it was verbose 
about the descriptor leak.

For reference, this is what I was working on 
https://bugzilla.redhat.com/show_bug.cgi?id=1413676
 
> 
> ​
> 
> 
> --
> There’s no obfuscated Perl contest because it’s pointless.
> 
> —Jeff Polk
> 
> Maranatha! <><
> John McKown
> 

-- 
--
Siteshwar Vashisht

Re: Bash leaks heredoc fd to child processes

[Siteshwar Vashisht]

- Original Message -
> From: "Chet Ramey" 
> To: "Siteshwar Vashisht" , bug-bash@gnu.org
> Cc: "chet ramey" 
> Sent: Tuesday, January 17, 2017 7:42:07 PM
> Subject: Re: Bash leaks heredoc fd to child processes
> 
> Thanks for the report. Here's a simpler patch.

Looks good to me. Thanks!

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

-- 
--
Siteshwar Vashisht

read() may fail due to nonblocking stdin

[Siteshwar Vashisht]

If a child process sets stdin to non-blocking and does not set it back to 
blocking before exiting, other processes may fail to read from stdin.

Reproducer steps :

$ cat set_nonblock.c 
#include 
#include 
#include 

int main() {
char buff[256];
int flags = fcntl (0, F_GETFL);
if (fcntl(0, F_SETFL, flags | O_NONBLOCK)) {
perror("fcntl failed");
}
if (read(0, buff, 256) == -1) {
perror("read failed");
}
}

$ cc -o set_nonblock set_nonblock.c
$ cat test.sh 
#!/bin/bash

./set_nonblock

cat

$ ./test.sh 
read failed: Resource temporarily unavailable
cat: -: Resource temporarily unavailable

Attached patch sets standard file descriptors to blocking before child process 
starts.

-- 
--
Siteshwar Vashisht
From f4fd3c17cb15f87fc8733b44bd477e32ff2d002a Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht 
Date: Sun, 22 Jan 2017 08:25:23 +0100
Subject: [PATCH] Make stdin blocking when command starts

---
 execute_cmd.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/execute_cmd.c b/execute_cmd.c
index 00389df..d63f6e5 100644
--- a/execute_cmd.c
+++ b/execute_cmd.c
@@ -531,6 +531,22 @@ async_redirect_stdin ()
 internal_error (_("cannot redirect standard input from /dev/null: %s"), strerror (errno));
 }
 
+/* Make stdin,stdout and stderr blocking */
+static void
+set_standard_fds_blocking() {
+  int flags = fcntl (0, F_GETFL);
+  if (fcntl(0, F_SETFL, flags & ~O_NONBLOCK))
+sys_error (_("Failed to make stdin blocking"));
+
+  flags = fcntl (1, F_GETFL);
+  if (fcntl(1, F_SETFL, flags & ~O_NONBLOCK))
+sys_error (_("Failed to make stdout blocking"));
+
+  flags = fcntl (2, F_GETFL);
+  if (fcntl(2, F_SETFL, flags & ~O_NONBLOCK))
+sys_error (_("Failed to make stderr blocking"));
+}
+
 #define DESCRIBE_PID(pid) do { if (interactive) describe_pid (pid); } while (0)
 
 extern int rpm_requires;
@@ -594,6 +610,8 @@ execute_command_internal (command, asynchronous, pipe_in, pipe_out,
 
   exec_result = EXECUTION_SUCCESS;
 
+  set_standard_fds_blocking();
+
   /* If a command was being explicitly run in a subshell, or if it is
  a shell control-structure, and it has a pipe, then we do the command
  in a subshell. */
-- 
2.9.3

Re: read() may fail due to nonblocking stdin

[Siteshwar Vashisht]

- Original Message -
> From: "Chet Ramey" 
> To: "Siteshwar Vashisht" , bug-bash@gnu.org
> Cc: "chet ramey" 
> Sent: Monday, January 23, 2017 8:01:38 PM
> Subject: Re: read() may fail due to nonblocking stdin
> 
> Something like this, for instance.

Thanks for the patch. Do you plan to include it upstream ?

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

-- 
--
Siteshwar Vashisht

Make syslog history configurable

[Siteshwar Vashisht]

Hello,

Bash history is logged to syslog if SYSLOG_HISTORY macro is defined in 
config-top.h. There is no option to enable/disable it at runtime. I am adding a 
shell option 'syshist' that can be used to configure logging bash history to 
syslog at runtime.


-- 
--
Siteshwar Vashisht
From c6ec0d751ded75188f64d1d1ac9916c44153c305 Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht 
Date: Tue, 24 Jan 2017 17:28:14 +0100
Subject: [PATCH] Make syslog history configurable

---
 bashhist.c   | 3 ++-
 builtins/set.def | 9 +
 config-top.h | 4 ++--
 flags.c  | 7 +++
 flags.h  | 4 
 5 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/bashhist.c b/bashhist.c
index 9979f99..e6127c8 100644
--- a/bashhist.c
+++ b/bashhist.c
@@ -851,7 +851,8 @@ bash_add_history (line)
 really_add_history (line);
 
 #if defined (SYSLOG_HISTORY)
-  bash_syslog_history (line);
+  if (syslog_history)
+bash_syslog_history (line);
 #endif
 
   using_history ();
diff --git a/builtins/set.def b/builtins/set.def
index 8122361..718ba39 100644
--- a/builtins/set.def
+++ b/builtins/set.def
@@ -116,6 +116,9 @@ Options:
operation differs from the Posix standard to
match the standard
   privileged   same as -p
+#if defined (SYSLOG_HISTORY)
+  syshist  same as -S
+#endif
   verbose  same as -v
 #if defined (READLINE)
   vi   use a vi-style line editing interface
@@ -141,6 +144,9 @@ Options:
 #endif /* BANG_HISTORY */
   -P  If set, do not resolve symbolic links when executing commands
   such as cd which change the current directory.
+#if defined (SYSLOG_HISTORY)
+  -S  If set, log history to syslog
+#endif
   -T  If set, the DEBUG and RETURN traps are inherited by shell functions.
   --  Assign any remaining arguments to the positional parameters.
   If there are no remaining arguments, the positional parameters
@@ -231,6 +237,9 @@ const struct {
   { "pipefail",  '\0', &pipefail_opt, (setopt_set_func_t *)NULL, (setopt_get_func_t *)NULL  },
   { "posix", '\0', &posixly_correct, set_posix_mode, (setopt_get_func_t *)NULL },
   { "privileged", 'p', (int *)NULL, (setopt_set_func_t *)NULL, (setopt_get_func_t *)NULL  },
+#if defined (SYSLOG_HISTORY)
+  { "syshist", 'S', (int *)NULL, (setopt_set_func_t *)NULL, (setopt_get_func_t *)NULL  },
+#endif
   { "verbose",	  'v', (int *)NULL, (setopt_set_func_t *)NULL, (setopt_get_func_t *)NULL  },
 #if defined (READLINE)
   { "vi",'\0', (int *)NULL, set_edit_mode, get_edit_mode },
diff --git a/config-top.h b/config-top.h
index cb0e002..ae8d124 100644
--- a/config-top.h
+++ b/config-top.h
@@ -114,8 +114,8 @@
 /* #define NOTFOUND_HOOK "command_not_found_handle" */
 
 /* Define if you want each line saved to the history list in bashhist.c:
-   bash_add_history() to be sent to syslog(). */
-/* #define SYSLOG_HISTORY */
+   If syshist shell option is set, bash_add_history() to be sent to syslog(). */
+#define SYSLOG_HISTORY
 #if defined (SYSLOG_HISTORY)
 #  define SYSLOG_FACILITY LOG_USER
 #  define SYSLOG_LEVEL LOG_INFO
diff --git a/flags.c b/flags.c
index 4b94fb0..cc16221 100644
--- a/flags.c
+++ b/flags.c
@@ -137,6 +137,10 @@ int history_expansion = 1;
 #  endif
 #endif /* BANG_HISTORY */
 
+#if defined (SYSLOG_HISTORY)
+int syslog_history = 0;
+#endif 
+
 /* Non-zero means that we allow comments to appear in interactive commands. */
 int interactive_comments = 1;
 
@@ -215,6 +219,9 @@ const struct flags_alist shell_flags[] = {
 #endif /* BANG_HISTORY */
   { 'I', &no_invisible_vars },
   { 'P', &no_symbolic_links },
+#if defined (SYSLOG_HISTORY)
+  { 'S', &syslog_history},
+#endif /* SYSLOG_HISTORY */
   { 'T', &function_trace_mode },
   {0, (int *)NULL}
 };
diff --git a/flags.h b/flags.h
index d5ed334..7fe25d5 100644
--- a/flags.h
+++ b/flags.h
@@ -62,6 +62,10 @@ extern int brace_expansion;
 extern int history_expansion;
 #endif /* BANG_HISTORY */
 
+#if defined (SYSLOG_HISTORY)
+extern int syslog_history;
+#endif
+
 #if defined (RESTRICTED_SHELL)
 extern int restricted;
 extern int restricted_shell;
-- 
2.9.3

Re: Make syslog history configurable

[Siteshwar Vashisht]

- Original Message -
> From: "Siteshwar Vashisht" 
> To: bug-bash@gnu.org
> Sent: Wednesday, January 25, 2017 1:39:12 PM
> Subject: Make syslog history configurable
> 
> Hello,
> 
> Bash history is logged to syslog if SYSLOG_HISTORY macro is defined in
> config-top.h. There is no option to enable/disable it at runtime. I am
> adding a shell option 'syshist' that can be used to configure logging bash
> history to syslog at runtime.

I modified the patch to use shopt instead of set to enable/disable syslog 
history.
> 
> 
> --
> --
> Siteshwar Vashisht
> 

-- 
--
Siteshwar Vashisht
From 49d2435aefca79b08d19f67c139d2091634853f5 Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht 
Date: Wed, 25 Jan 2017 15:58:59 +0100
Subject: [PATCH] Make syslog history configurable with shopt

---
 bashhist.c |  5 +++--
 builtins/shopt.def | 11 +++
 config-top.h   |  2 +-
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/bashhist.c b/bashhist.c
index 9979f99..7ab6486 100644
--- a/bashhist.c
+++ b/bashhist.c
@@ -749,7 +749,7 @@ check_add_history (line, force)
 #define SYSLOG_MAXLEN 600
 
 extern char *shell_name;
-
+int syslog_history = 0;
 #ifndef OPENLOG_OPTS
 #define OPENLOG_OPTS 0
 #endif
@@ -851,7 +851,8 @@ bash_add_history (line)
 really_add_history (line);
 
 #if defined (SYSLOG_HISTORY)
-  bash_syslog_history (line);
+  if (syslog_history)
+bash_syslog_history (line);
 #endif
 
   using_history ();
diff --git a/builtins/shopt.def b/builtins/shopt.def
index 2febb7e..848bab3 100644
--- a/builtins/shopt.def
+++ b/builtins/shopt.def
@@ -118,6 +118,10 @@ extern char *shell_name;
 extern int debugging_mode;
 #endif
 
+#if defined (SYSLOG_HISTORY)
+extern int syslog_history;
+#endif
+
 static void shopt_error __P((char *));
 
 static int set_shellopts_after_change __P((char *, int));
@@ -223,6 +227,9 @@ static struct {
 #endif
   { "shift_verbose", &print_shift_error, (shopt_set_func_t *)NULL },
   { "sourcepath", &source_uses_path, (shopt_set_func_t *)NULL },
+#if defined (SYSLOG_HISTORY)
+  { "syslog_history", &syslog_history, (shopt_set_func_t *)NULL },
+#endif
   { "xpg_echo", &xpg_echo, (shopt_set_func_t *)NULL },
   { (char *)0, (int *)0, (shopt_set_func_t *)NULL }
 };
@@ -345,6 +352,10 @@ reset_shopt_options ()
   command_oriented_history = 1;
 #endif
 
+#if defined (SYSLOG_HISTORY)
+  syslog_history = 0;
+#endif
+
 #if defined (READLINE)
   complete_fullquote = 1;
   force_fignore = 1;
diff --git a/config-top.h b/config-top.h
index cb0e002..8051fad 100644
--- a/config-top.h
+++ b/config-top.h
@@ -115,7 +115,7 @@
 
 /* Define if you want each line saved to the history list in bashhist.c:
bash_add_history() to be sent to syslog(). */
-/* #define SYSLOG_HISTORY */
+#define SYSLOG_HISTORY
 #if defined (SYSLOG_HISTORY)
 #  define SYSLOG_FACILITY LOG_USER
 #  define SYSLOG_LEVEL LOG_INFO
-- 
2.9.3

'history -n' should re-read history file if history is cleared

[Siteshwar Vashisht]

Reproducer steps :

> PROMPT_COMMAND="history -a; history -c; history -n"
> history

Actual output :
2  history

Expected output:
  All the older history items from bash history file should be re-read.

I am providing a test patch that resolves this issue.

-- 
--
Siteshwar Vashisht
diff --git a/bashhist.c b/bashhist.c
index 9979f99..5ccf9db 100644
--- a/bashhist.c
+++ b/bashhist.c
@@ -332,7 +332,7 @@ bash_clear_history ()
 {
   clear_history ();
   history_lines_this_session = 0;
-  /* XXX - reset history_lines_read_from_file? */
+  history_lines_in_file = history_lines_read_from_file = 0;
 }
 
 /* Delete and free the history list entry at offset I. */

Re: 'history -n' should re-read history file if history is cleared

[Siteshwar Vashisht]

- Original Message -
> From: "Chet Ramey" 
> To: "Siteshwar Vashisht" , bug-bash@gnu.org
> Cc: "chet ramey" 
> Sent: Tuesday, April 18, 2017 5:36:05 PM
> Subject: Re: 'history -n' should re-read history file if history is cleared
> 
> Why should explicitly clearing old items from the history cause them to
> be treated as new?

I was just a bit skeptical about behavior of 'history -n' if history is 
cleared. On a second thought, I think the current behavior is fine and may not 
worth the change.

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

-- 
--
Siteshwar Vashisht

test builtin does not check for nanoseconds while comparing timestamps

[Siteshwar Vashisht]

Configuration Information [Automatically generated, do not change]:
Machine: x86_64
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64' 
-DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='x86_64-redhat-linux-gnu' 
-DCONF_VENDOR='redhat' -DLOCALEDIR='/usr/share/locale' -DPACKAGE='bash' -DSHELL 
-DHAVE_CONFIG_H   -I.  -I. -I./include -I./lib  -D_GNU_SOURCE -DRECYCLES_PIDS 
-DDEFAULT_PATH_VALUE='/usr/local/bin:/usr/bin'  -O2 -g -pipe -Wall 
-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
-fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches 
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic
uname output: Linux localhost.localdomain 4.10.17-200.fc25.x86_64 #1 SMP Mon 
May 22 18:12:57 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
Machine Type: x86_64-redhat-linux-gnu

Bash Version: 4.3
Patch Level: 43
Release Status: release

Description:
If the modified timestamp of 2 files differ by less than 1 second, test 
builtin sometimes returns incorrect result. It happens because 
HAVE_STRUCT_STAT_ST_ATIM_TV_NSEC is not defined in config.h.

Repeat-By:
while true; do touch a; sleep 0.1; touch b; stat a b|grep Modify; [ b 
-nt a ] && echo test_cmd_ok || echo test_cmd_failed; echo; done


Fix:
Attached patch fixes this issue.

-- 
--
Siteshwar Vashisht
From 2648afeb4e2bae4e250700c8fb37e2aa64c82b11 Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht 
Date: Sat, 3 Jun 2017 01:37:08 +0200
Subject: [PATCH] Fix macro check for struct stat.st_atim.tv_nsec

---
 config.h.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.h.in b/config.h.in
index b5c35c3..1e4b71d 100644
--- a/config.h.in
+++ b/config.h.in
@@ -452,6 +452,7 @@
 #undef SYS_TIME_H_DEFINES_STRUCT_TIMESPEC
 #undef PTHREAD_H_DEFINES_STRUCT_TIMESPEC
 
+#undef HAVE_STRUCT_STAT_ST_ATIM_TV_NSEC
 #undef TYPEOF_STRUCT_STAT_ST_ATIM_IS_STRUCT_TIMESPEC
 #undef HAVE_STRUCT_STAT_ST_ATIMESPEC_TV_NSEC
 #undef HAVE_STRUCT_STAT_ST_ATIMENSEC
-- 
2.9.4