mainThe GNU Bourne-Again SHell - Support: sr #110763, Regression: bash does not give an...

 
 

sr #110763: Regression: bash does not give an error if it cannot read the remainder of the script

Submitter:  None
Submitted:  Wed 09 Nov 2022 03:25:13 PM UTC
   
 
Category:  None Priority:  5 - Normal
Severity:  4 - Important Status:  In Progress
Privacy:  Public Assigned to:  None
Originator Email:  -email is unavailable- Open/Closed:  Open
Operating System:  GNU/Linux
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sat 04 Feb 2023 06:09:17 PM UTC, comment #11: 

The POSIX group is discussing this. I think the initial consensus is to make it an error, but there's existing practice to consider.

Chet Ramey <chet>
Group administrator
Wed 11 Jan 2023 03:41:24 PM UTC, comment #10: 


> Follow-up Comment #9, sr #110763 (project bash):
>
> Sorry I've been unable to respond earlier. I'm the original filer of this sr
> #110763 here.
>
> To answer your comment, yes, this issue still exists in Bash 5.2, compiled
> from source.
>
> I found that if the ESTALE occurs on the very first read (before the dup2()
> call) then it will be reported as an error and exit with code 126.


Yes. That's a read the shell issues to determine whether or not the script
file is a binary file. Getting a read error there, before trying to parse
and execute any commands, means there's little sense in continuing. The 126
status makes the most sense based on the POSIX description.

> If the ESTALE occurs from any read on fd 255 (the result of the dup2() call)
> it will not be reported as an error.


That's where you get the EOF, when you're trying to parse and execute
commands.

> That means you can get an exit code of 0 having not run any of the script at
> all.


The exit status for an empty script, or a script composed entirely of blank
lines and comments, where you certainly don't run any commands, is 0.

>
> In terms of why wasn't this caught earlier, at least for Ubuntu it turns out
> that they've been patching it all this time. I downloaded the Ubuntu 20.04 LTS
> sources and patches from https://packages.ubuntu.com/focal/shells/bash and in
> debian/patches/input-err.diff (inside bash_5.0-6ubuntu1.2.debian.tar.xz) is
> the error handling with the error text you didn't recognize as being in your
> code:


That's the wrong fix -- you should be able to execute commands you read
before getting the read error. For instance, if you read

echo a; echo b;

and get a read error, you should execute those two commands instead of
exiting immediately.


> We still feel strongly that an error (not eof) reading the script needs to
> become a nonzero exit code and error message reported to the user. Too many
> situations rely on an exit code of 0 meaning the entire script ran
> successfully, which should be a valid assumption.


Based on existing shell behavior -- and historical shell behavior -- it
seems like that doesn't match existing practice.

I'm interested in seeing what the POSIX group has to say, if anything.
Thorsten put the question out there.

Chet Ramey <chet>
Group administrator
Mon 09 Jan 2023 05:27:25 PM UTC, comment #9: 

Sorry I've been unable to respond earlier. I'm the original filer of this sr #110763 here.

To answer your comment, yes, this issue still exists in Bash 5.2, compiled from source.

I found that if the ESTALE occurs on the very first read (before the dup2() call) then it will be reported as an error and exit with code 126.

If the ESTALE occurs from any read on fd 255 (the result of the dup2() call) it will not be reported as an error.

That means you can get an exit code of 0 having not run any of the script at all.

In terms of why wasn't this caught earlier, at least for Ubuntu it turns out that they've been patching it all this time. I downloaded the Ubuntu 20.04 LTS sources and patches from https://packages.ubuntu.com/focal/shells/bash and in debian/patches/input-err.diff (inside bash_5.0-6ubuntu1.2.debian.tar.xz) is the error handling with the error text you didn't recognize as being in your code:


 --- a/input.c
 +++ b/input.c
 @@ -517,7 +517,7 @@ b_fill_buffer (bp)
        if (nr == 0)
         bp->b_flag |= B_EOF;
        else
-       bp->b_flag |= B_ERROR;*{color}
+       fatal_error("error reading input file: %s", strerror(errno));*{color}
        return (EOF);
      }


We still feel strongly that an error (not eof) reading the script needs to become a nonzero exit code and error message reported to the user. Too many situations rely on an exit code of 0 meaning the entire script ran successfully, which should be a valid assumption.

Anonymous
Sat 07 Jan 2023 09:38:35 PM UTC, comment #8: 

It doesn't matter for the shell. We're not talking about one of the specific enumerated exit values in POSIX; those conditions don't match here.

But the real question is whether this condition overrides the requirement to exit with the last command exit status. In that case, you can choose anything between 1 and 125.

Chet Ramey <chet>
Group administrator
Sat 07 Jan 2023 09:29:50 PM UTC, comment #7: 

I beg to differ. The errorlevel does matter; for some programs (diff, grep, unifdef, cURL, …) it does signal something.

Thorsten Glaser <mirabilos>
Sat 07 Jan 2023 09:10:29 PM UTC, comment #6: 


> Follow-up Comment #3, sr #110763 (project bash):
>
> This was also reported against mksh in
> https://bugs.launchpad.net/mksh/+bug/2002044
>
> I wonder why this wasn’t caught in so many shells.


Because the shell exits with the status of the last command executed, and
a read error, which does not cause command execution, can't change $?.

> In the sense of cross-shell coordination, what errorlevel do you suggest? I
> have a slight dislike for the small numbers here, but arguments could be held
> that the large numbers are more likely to have special meaning for the
> scripts, too.


If you choose to go that way, the exit status doesn't matter as long as
it's non-zero.

> In fact, interrupting by being unable to read the next part of the script is a
> rather severe issue. I’m tempted to argue that killing myself with a
> suitable signal (PIPE? BUS? HUP?) and, if ignored, simulating the
> corresponding 128+signo exit would be better.


This is a bad idea. Generating a signal when you didn't receive one is
misleading, and trying to fool the user by exiting with a status reserved
for being killed by a signal is worse.

Chet Ramey <chet>
Group administrator
Sat 07 Jan 2023 08:41:39 PM UTC, comment #5: 

So the question is whether or not the standard means read errors when it says "including but not limited to syntax, redirection, or variable assignment errors."

If you don't think the standard means read errors above, then what happens when the shell successfully executes a command and then gets a read error the next time it goes back to read from the script file? The shell is required to exit with the exit
status of the last command executed.

The only shell I've found that reacts to this is yash. All the rest treat read returning -1 the same as 0 (EOF) and exit with the status of the last command.

Chet Ramey <chet>
Group administrator
Fri 06 Jan 2023 02:15:16 PM UTC, comment #4: 

Is treating a read error the same as EOF “good”, then?

Probably suitable for the standard as historical practice, but I doubt anyone relies on that at the moment…

Thorsten Glaser <mirabilos>
Fri 06 Jan 2023 02:13:51 PM UTC, comment #3: 

This was also reported against mksh in https://bugs.launchpad.net/mksh/+bug/2002044

I wonder why this wasn’t caught in so many shells.

In the sense of cross-shell coordination, what errorlevel do you suggest? I have a slight dislike for the small numbers here, but arguments could be held that the large numbers are more likely to have special meaning for the scripts, too.

In fact, interrupting by being unable to read the next part of the script is a rather severe issue. I’m tempted to argue that killing myself with a suitable signal (PIPE? BUS? HUP?) and, if ignored, simulating the corresponding 128+signo exit would be better.

Thorsten Glaser <mirabilos>
Wed 09 Nov 2022 07:19:48 PM UTC, comment #2: 

I will note several things.

1. The shell is required to exit with the status of the last-executed command; unless

2. There is a parse error while reading a command, in which case most shells exit with status 2 or 127 (among other errors caused by word expansions or execution, none of which matter here); and

3. EOF will cause a parse error if the shell is attempting to parse a compound command; and

4. Read errors are treated the same as EOF; however

5. If the shell gets an EOF while reading a simple command, it generally just tries to execute that simple command and exits with that command's exit status.

POSIX allows some wiggle room if the shell "detects an error," though read errors are not mentioned explicitly.

I would be surprised to see a shell that behaved otherwise.

As to your "regression," that error message doesn't appear anywhere in the bash-4.3 source I distributed. Maybe debian/ubuntu added a patch, in which case that's the place to ask.

Chet Ramey <chet>
Group administrator
Wed 09 Nov 2022 04:47:53 PM UTC, comment #1: 

Bash-4.4 is three versions behind and has not been supported for a long time. Please check using bash-5.2 and report those results.

Chet Ramey <chet>
Group administrator
Wed 09 Nov 2022 03:25:13 PM UTC, original submission:  

A customer had a situation where their shared storage (GPFS) failed. A script they were running on said storage did not complete, but did not report the expected ESTALE error and in fact exited with a 0 code.

I did some experimenting and found that various versions of bash/ksh read the shell script in 8k or 64k chunks. If one of those subsequent reads other than the initial fails, then bash exits without having finished running the script, does not display an error, and the exit code is whatever was the last exit status within the script.

I originally reproduced this using a Python-based FUSE filesystem driver (a simple passthrough of https://github.com/skorokithakis/python-fuse-sample using the fuse.py package from https://github.com/fusepy/fusepy) to mount /tmp/fuse-actual onto /mnt (requires root). By having a shell script:


#!/bin/bash
echo 'Sleeping...'
sleep 10
# A thousand comment lines taking up
# over 64K of space.
...
#
echo 'Okay, we made it here'
exit 1


and hitting ^C on the Python process mounting the filesystem while the script is sleeping will provoke ENOTCONN, but then bash exits with 0:


read(10, "#!/bin/bash\n\necho \"Sleeping...\"\n"..., 65536) = 65536
stat(".", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0
lseek(10, 42, SEEK_SET)                 = 42
write(1, "Sleeping...\n", 12)           = 12
rt_sigaction(SIGALRM, {0x41aa90, [], SA_RESTORER|SA_INTERRUPT, 0x7fbdfd3cbcb0}, {SIG_DFL, [], 0}, 8) = 0
poll(0x7ffda485e7e0, 0, 5000)           = 0 (Timeout)
stat(".", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0
lseek(10, 65536, SEEK_SET)              = 65536
read(10, 0x7fbdfde18050, 65536)         = -1 ENOTCONN (Transport endpoint is not connected)
read(10, 0x7fbdfde18050, 65536)         = -1 ENOTCONN (Transport endpoint is not connected)
close(10)                               = -1 ENOTCONN (Transport endpoint is not connected)
close(10)                               = -1 EBADF (Bad file descriptor)
fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 7), ...}) = 0
fstat(2, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 7), ...}) = 0
exit_group(0)                           = ?


If you put the line
/bin/false
immediately after the sleep and before the comments, then the shell will exit_group(1), so it appears to be exiting prematurely, but with the last command status.

After determining this using the FUSE filesystem, a colleague figured out how to reproduce the issue more easily (and without root) using strace's fault injection capability. By first tracing a success one can count and determine which syscall should get a synthetic error. Use the following script to create big.sh:


#! /bin/sh
# Write a script big.sh with an initial echo, then a bunch of comments,
# then a final echo just after the 64KB mark.
target_size=${1:-65536}
target=big.sh
readonly eol_nbytes=1           # count the \n character
exec > $target
chmod +x $target
nbytes=0
str='echo Starting ...'
while [ $nbytes -lt $target_size ]; do
    echo $str
    nbytes=$((nbytes + ${#str} + eol_nbytes))
    str='# filler'
done
echo 'echo ... done.'


Then trace it:

strace -o strace.out bash ./big.sh


Grep the trace to count all the calls to read():

% grep '^read(' /tmp/strace.out | cat -n
     1        read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0 \351\0\0\0\0\0\0"..., 832) = 832
     2        read(3, "\4\0\0\0\20\0\0\0\5\0\0\0GNU\0\2\0\0\300\4\0\0\0\3\0\0\0\0\0\0\0", 32) = 32
     3        read(3, "\4\0\0\0\20\0\0\0\5\0\0\0GNU\0\2\0\0\300\4\0\0\0\3\0\0\0\0\0\0\0", 32) = 32
     4        read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\300\20\0\0\0\0\0\0"..., 832) = 832
     5        read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\2405\2\0\0\0\0\0"..., 832) = 832
     6        read(3, "\4\0\0\0\20\0\0\0\5\0\0\0GNU\0\2\0\0\300\4\0\0\0\3\0\0\0\0\0\0\0", 32) = 32
     7        read(3, "# Locale name alias data base.\n#"..., 4096) = 2997
     8        read(3, "", 4096)                       = 0
     9        read(3, "echo Starting ...\n# filler\n# fil"..., 80) = 80
    10        read(255, "echo Starting ...\n# filler\n# fil"..., 8192) = 8192
    11        read(255, "filler\n# filler\n# filler\n# fille"..., 8192) = 8192
    12        read(255, "ller\n# filler\n# filler\n# filler\n"..., 8192) = 8192
    13        read(255, "er\n# filler\n# filler\n# filler\n# "..., 8192) = 8192
    14        read(255, "\n# filler\n# filler\n# filler\n# fi"..., 8192) = 8192
    15        read(255, " filler\n# filler\n# filler\n# fill"..., 8192) = 8192
    16        read(255, "iller\n# filler\n# filler\n# filler"..., 8192) = 8192
    17        read(255, "ler\n# filler\n# filler\n# filler\n#"..., 8192) = 8192
    18        read(255, "r\necho ... done.\n", 8192)  = 17
    19        read(255, "", 8192)


Okay, this is RHEL8's bash and it reads in 8k not 64k chunks. No worries, you can still see that the 18th read is the first one past the 64k boundary. Inject an error into that read:


% strace -o /tmp/strace.out -e inject=read:error=ESTALE:when=18+1 bash ./big.sh
Starting ...


Note that it never finishes the script, we don't see "echo ... done." read or performed, but bash exits with success:

read(255, "ler\n# filler\n# filler\n# filler\n#"..., 8192) = 8192
read(255, 0x56385755aed0, 8192)         = -1 ESTALE (Stale file handle) (INJECTED)
read(255, 0x56385755aed0, 8192)         = -1 ESTALE (Stale file handle) (INJECTED)
rt_sigprocmask(SIG_BLOCK, [CHLD], [], 8) = 0
rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
exit_group(0)                           = ?
+++ exited with 0 +++


I found this behavior in RHEL 8 with bash 4.4.19(1). I also found it with ksh93 (93u+ 2012-08-01) and mksh (R56 2018/01/14) though of course those aren't your problem and I'll file those appropriately.

Importantly with Bash, though, it's a regression! It used to handle this situation correctly. On an Ubuntu 14.04 system with Bash 4.3.11 you get (with the python example; Ubuntu 14.04 is older than strace's injection ability):


/mnt/script: error reading input file: Transport endpoint is not connected
<returns with exit code 2>


While this issue might seem obscure due to the methods used to reproduce the problem, getting ESTALE from network shared storage (e.g. GPFS or NFS) is quite possible, it's just hard to provoke in a test. And as noted, for Bash it's a regression.

Thanks for taking a look.

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mirabilos (Posted a comment)
  • -email is unavailable- added by chet (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-01-11 chet StatusNeed Info In Progress
    2022-11-09 chet StatusNone Need Info

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code