sh: Don't assume EINTR means SIGALRM

While waiting for input in the read builtin, if select() is interrupted
but there is no pending signal, we act like we timed out, and return the
same status as i

sh: Don't assume EINTR means SIGALRM

While waiting for input in the read builtin, if select() is interrupted
but there is no pending signal, we act like we timed out, and return the
same status as if we had been interrupted by SIGALRM, instead of looping
until we actually do time out.

* Replace the single select() call with a ppoll() loop.

* Improve validation of the timeout value. We now accept things like
"1h30m15s", which we used to silently truncate to "1h". The flip side
is that we no longer accept things like "1hour" or "5sec".

* Modify the existing `read -t 0` test case to verify that read returns
immediately when there is input and fails immediately when there isn't.

* Add a second test case which performs the same tests with a non-zero
timeout value.

PR: 290844
MFC after: 1 week
Fixes: c4539460e3a4 ("sh: Improve error handling in read builtin:")
Reviewed by: jilles, bdrewery
Differential Revision: https://reviews.freebsd.org/D53761

show more ...

sh: Fix a double free in a rare scenario with pipes

The command
sh -c 'sleep 3 | sleep 2 & sleep 3 & kill %1; wait %1'
crashes (with appropriate sanitization such as putting
MALLOC_CONF=abort:true

sh: Fix a double free in a rare scenario with pipes

The command
sh -c 'sleep 3 | sleep 2 & sleep 3 & kill %1; wait %1'
crashes (with appropriate sanitization such as putting
MALLOC_CONF=abort:true,junk:true in the environment or compiling with
-fsanitize=address).

What happens here is that waitcmdloop() calls dowait() with a NULL job
pointer, instructing dowait() to freejob() if it's a non-interactive
shell and $! was not and cannot be referenced for it. However,
waitcmdloop() then uses fields possibly freed by freejob() and calls
freejob() again.

This only occurs if the job being waited for is identified via % syntax
($! has never been referenced for it), it is a pipeline with two or more
elements and another background job has been started before the wait
command. That seems special enough for a bug to remain. Test scripts
written by Jilles would almost always use $! and not % syntax.

We can instead make waitcmdloop() pass its job pointer to dowait(),
fixing up things for that (waitcmdloop() will have to call deljob() if
it does not call freejob()).

The crash from
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=290330#c2 appears to
be the same bug.

PR: 290330
Reported by: bdrewery
Reviewed by: bdrewery
Differential Revision: https://reviews.freebsd.org/D53773

show more ...

sh(1): Do not interpret chdir to "" as equivalent to chdir with no argument

A script that does the following:

cd "${dir}" || exit 1

would incorrectly remain in the current directory when `${di

sh(1): Do not interpret chdir to "" as equivalent to chdir with no argument

A script that does the following:

cd "${dir}" || exit 1

would incorrectly remain in the current directory when `${dir}` is
an empty string under the current implementation. This behavior,
while historical, is potentially dangerous, as it is likely not
what the script author intended.

Change the command to treat an empty string as an error and emit a
diagnostic message to standard error, as required by
IEEE Std 1003.1-2024.

PR: standards/287440
Test Plan: kyua test bin/sh
Relnotes: yes
Differential Revision: https://reviews.freebsd.org/D50974

show more ...

Remove residual blank line at start of Makefile

This is a residual of the $FreeBSD$ removal.

MFC After: 3 days (though I'll just run the command on the branches)
Sponsored by: Netflix

Remove $FreeBSD$: one-line sh pattern

Remove /^\s*#[#!]?\s*\$FreeBSD\$.*$\n/

sh: accept fc options grouped behind one '-'

As per Utility Syntax Guidelines, accept both forms: -l -n and -ln.

To do that, anticipate the source string for the next option that will
be parsed by

sh: accept fc options grouped behind one '-'

As per Utility Syntax Guidelines, accept both forms: -l -n and -ln.

To do that, anticipate the source string for the next option that will
be parsed by nextopt(). It's not always *argptr, sometimes it is
nextopt_optptr.

To simplify the check for not_fcnumber, slightly modify nextopt() to
always nullify nextopt_optptr in cases where it would have been set
to point to a NUL character.

Reviewed by: jilles
Differential Revision: https://reviews.freebsd.org/D35836

show more ...

read builtin: Empty variables on timeout

This matches how a non-timeout error is handled.

Reviewed by: jilles
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D31876

sh: Write absolute path in command -vV and type

POSIX is pretty clear that command -v, command -V and type shall write
absolute pathnames. Therefore, we need to prepend the current directory's
name

sh: Write absolute path in command -vV and type

POSIX is pretty clear that command -v, command -V and type shall write
absolute pathnames. Therefore, we need to prepend the current directory's
name to relative pathnames.

This can happen either when PATH contains a relative pathname or when the
operand contains a slash but is not an absolute pathname.

show more ...

sh: Don't assume EINTR means SIGALRM

While waiting for input in the read builtin, if select() is interrupted
but there is no pending signal, we act like we timed out, and return the
same status as i

sh: Don't assume EINTR means SIGALRM

While waiting for input in the read builtin, if select() is interrupted
but there is no pending signal, we act like we timed out, and return the
same status as if we had been interrupted by SIGALRM, instead of looping
until we actually do time out.

* Replace the single select() call with a ppoll() loop.

* Improve validation of the timeout value. We now accept things like
"1h30m15s", which we used to silently truncate to "1h". The flip side
is that we no longer accept things like "1hour" or "5sec".

* Modify the existing `read -t 0` test case to verify that read returns
immediately when there is input and fails immediately when there isn't.

* Add a second test case which performs the same tests with a non-zero
timeout value.

PR: 290844
MFC after: 1 week
Fixes: c4539460e3a4 ("sh: Improve error handling in read builtin:")
Reviewed by: jilles, bdrewery
Differential Revision: https://reviews.freebsd.org/D53761

show more ...

sh: Fix a double free in a rare scenario with pipes

The command
sh -c 'sleep 3 | sleep 2 & sleep 3 & kill %1; wait %1'
crashes (with appropriate sanitization such as putting
MALLOC_CONF=abort:true

sh: Fix a double free in a rare scenario with pipes

The command
sh -c 'sleep 3 | sleep 2 & sleep 3 & kill %1; wait %1'
crashes (with appropriate sanitization such as putting
MALLOC_CONF=abort:true,junk:true in the environment or compiling with
-fsanitize=address).

What happens here is that waitcmdloop() calls dowait() with a NULL job
pointer, instructing dowait() to freejob() if it's a non-interactive
shell and $! was not and cannot be referenced for it. However,
waitcmdloop() then uses fields possibly freed by freejob() and calls
freejob() again.

This only occurs if the job being waited for is identified via % syntax
($! has never been referenced for it), it is a pipeline with two or more
elements and another background job has been started before the wait
command. That seems special enough for a bug to remain. Test scripts
written by Jilles would almost always use $! and not % syntax.

We can instead make waitcmdloop() pass its job pointer to dowait(),
fixing up things for that (waitcmdloop() will have to call deljob() if
it does not call freejob()).

The crash from
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=290330#c2 appears to
be the same bug.

PR: 290330
Reported by: bdrewery
Reviewed by: bdrewery
Differential Revision: https://reviews.freebsd.org/D53773

show more ...

sh(1): Do not interpret chdir to "" as equivalent to chdir with no argument

A script that does the following:

cd "${dir}" || exit 1

would incorrectly remain in the current directory when `${di

sh(1): Do not interpret chdir to "" as equivalent to chdir with no argument

A script that does the following:

cd "${dir}" || exit 1

would incorrectly remain in the current directory when `${dir}` is
an empty string under the current implementation. This behavior,
while historical, is potentially dangerous, as it is likely not
what the script author intended.

Change the command to treat an empty string as an error and emit a
diagnostic message to standard error, as required by
IEEE Std 1003.1-2024.

PR: standards/287440
Test Plan: kyua test bin/sh
Relnotes: yes
Differential Revision: https://reviews.freebsd.org/D50974

show more ...

Remove residual blank line at start of Makefile

This is a residual of the $FreeBSD$ removal.

MFC After: 3 days (though I'll just run the command on the branches)
Sponsored by: Netflix

Remove $FreeBSD$: one-line sh pattern

Remove /^\s*#[#!]?\s*\$FreeBSD\$.*$\n/

sh: accept fc options grouped behind one '-'

As per Utility Syntax Guidelines, accept both forms: -l -n and -ln.

To do that, anticipate the source string for the next option that will
be parsed by

sh: accept fc options grouped behind one '-'

As per Utility Syntax Guidelines, accept both forms: -l -n and -ln.

To do that, anticipate the source string for the next option that will
be parsed by nextopt(). It's not always *argptr, sometimes it is
nextopt_optptr.

To simplify the check for not_fcnumber, slightly modify nextopt() to
always nullify nextopt_optptr in cases where it would have been set
to point to a NUL character.

Reviewed by: jilles
Differential Revision: https://reviews.freebsd.org/D35836

show more ...

read builtin: Empty variables on timeout

This matches how a non-timeout error is handled.

Reviewed by: jilles
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D31876

sh: Write absolute path in command -vV and type

POSIX is pretty clear that command -v, command -V and type shall write
absolute pathnames. Therefore, we need to prepend the current directory's
name

sh: Write absolute path in command -vV and type

POSIX is pretty clear that command -v, command -V and type shall write
absolute pathnames. Therefore, we need to prepend the current directory's
name to relative pathnames.

This can happen either when PATH contains a relative pathname or when the
operand contains a slash but is not an absolute pathname.

show more ...

sh: Don't treat % specially in CDPATH

sh: Don't have [ match any [[:class:]]

Submitted by: Robert Elz
MFC after: 3 days

MFhead@r320180

Merge ^/head r319973 through 321382.

Merge ^/head r320042 through r320397.

sh: Ignore error when cd writes the directory actually switched to.

If CDPATH is used non-trivially or the operand is "-", cd writes the
directory actually switched to. (We currently do this only in

sh: Ignore error when cd writes the directory actually switched to.

If CDPATH is used non-trivially or the operand is "-", cd writes the
directory actually switched to. (We currently do this only in interactive
shells, but POSIX requires this in non-interactive shells as well.)

As mentioned in Austin group bug #1045, cd shall not return an error while
leaving the current directory changed. Therefore, ignore any write error.

show more ...

Merge ^/head r317971 through r318379.

sh: Fix '-' from quoted arithmetic in case/glob pattern range.

It does not make much sense to generate the '-' in a pattern bracket
expression using arithmetic expansion, but it does not make sense

sh: Fix '-' from quoted arithmetic in case/glob pattern range.

It does not make much sense to generate the '-' in a pattern bracket
expression using arithmetic expansion, but it does not make sense to forbid
it either.

Try to avoid reprocessing the string if it is unnecessary.

show more ...

sh: Add test for arithmetic expansion in [x-y] pattern range.

It does not make much sense to generate the '-' in a pattern bracket
expression using arithmetic expansion, but it does not make sense t

sh: Add test for arithmetic expansion in [x-y] pattern range.

It does not make much sense to generate the '-' in a pattern bracket
expression using arithmetic expansion, but it does not make sense to forbid
it either.

This test already passes.

show more ...