errno not restore before printing error in exec_builtin

[Emanuele Torre]

In a branch of exec_builtin(), errno is not restored before printing an
error message, and that causes "Success" to be printed.

Example:

$ # bash 5.2.37
$ cd /tmp
$ echo "export env='\''$(echo {1..100500})'\''" > src
$ . ./src
$ exec -- "$BASH"
bash: /bin/bash: Argument list too long
bash: /bin/bash: Success
$ unset -v env # to make the shell usable again

(Thanks to ano on the #bash IRC channel of libera.chat for the reproduce
 commands)

With the patch below, it now prints these error messages:

$ ./bash
$ . /tmp/src
$ exec -- "$BASH"
bash: /home/emanuele6/git/bash/bash: Argument list too long
bash: /home/emanuele6/git/bash/bash: Argument list too long

Note that exec errors caused by the exec builtin are always printed
twice, so maybe the code should be changed instead of just fixing this
"not restoring errno" bug.

$ # bash 5.2.37
$ exec /dev/null
bash: /dev/null: Permission denied
bash: exec: /dev/null: cannot execute: Permission denied

This happens because the shell_execve() function used by the exec
builtin prints errors to stderr on its own, and the exec_builtin code,
just prints it again, sometimes with "exec: " added in the middle.

The patch I have used is at the bottom of the e-mail.

o/
 emanuele6

---
 builtins/exec.def | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtins/exec.def b/builtins/exec.def
index 618882c8..d2d8ad3c 100644
--- a/builtins/exec.def
+++ b/builtins/exec.def
@@ -242,7 +242,10 @@ exec_builtin (WORD_LIST *list)
   exit_value = EX_NOEXEC;  /* As per Posix.2, 3.14.6 */
 }
   else
-file_error (command);
+{
+  errno = opt;
+  file_error (command);
+}
 
 failed_exec:
   FREE (command);
-- 
2.47.0

Re: errno not restore before printing error in exec_builtin

[Emanuele Torre]

On Mon, Oct 28, 2024 at 12:47:53AM +0100, Emanuele Torre wrote:
>  }
>else
> -file_error (command);
> +{
> +  errno = opt;
> +  file_error (command);
> +}
>  

I just thought that perhaps this branch was supposed to print errors for
the executable_file() function call above, so maybe the correct fix is
to change  else  =>  "else if (errno)"  instead of restoring it.

Anyway, that does not address that this if-elseif-else block always
prints duplicate errors. I have not found any case in which after
shell_execve() is called, this code print an error that is not a
duplicate.

o/
 emanuele6