Skip to content

Commit

Permalink
combining alarm and IFS caused segfault (rhbz#1176670)
Browse files Browse the repository at this point in the history
The undocumented alarm builtin executes actions unsafely so that
'read' with an IFS assignment crashed when an alarm was triggered.

This applies an edited version of a Red Hat patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-alarmifs.patch

Prior discussion:
https://bugzilla.redhat.com/1176670

src/cmd/ksh93/bltins/alarm.c:
- Add a TODO note based on dgk's 2014 email cited in the RH bug.
- When executing the trap function, save and restore the IFS table.

src/cmd/ksh93/sh/init.c: get_ifs():
- Remove now-unnecessary SHOPT_MULTIBYTE preprocessor directives as
  8477d2c lets the compiler optimise out multibyte code if needed.
- Initialise the 0 position of the IFS table to S_EOF. This
  corresponds with the static state tables in data/lexstates.c.

src/cmd/ksh93/tests/builtins.sh:
- Crash test.
  • Loading branch information
McDutchie committed Sep 27, 2020
1 parent f7c3565 commit 18b3f4a
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 8 deletions.
17 changes: 17 additions & 0 deletions src/cmd/ksh93/bltins/alarm.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,18 @@
*
*/

/*
* TODO: 2014 email from David Korn cited at <https://bugzilla.redhat.com/1176670>:
*
* > I never documented the alarm builtin because it is problematic. The
* > problem is that traps can't safely be handled asynchronously. What should
* > happen is that the trap is marked for execution (sh_trapnote) and run after
* > the current command completes. The time trap should wake up the shell if
* > it is blocked and it should return and then handle the trap.
*
* When that is done, the save_ifstable workaround can probably be removed.
*/

#include "defs.h"
#include <error.h>
#include <stak.h>
Expand Down Expand Up @@ -141,7 +153,12 @@ void sh_timetraps(Shell_t *shp)
{
tp->flags &= ~L_FLAG;
if(tp->action)
{
char save_ifstable[256];
memcpy(save_ifstable,shp->ifstable,sizeof(save_ifstable));
sh_fun(tp->action,tp->node,(char**)0);
memcpy(shp->ifstable,save_ifstable,sizeof(save_ifstable));
}
tp->flags &= ~L_FLAG;
if(!tp->flags)
{
Expand Down
10 changes: 2 additions & 8 deletions src/cmd/ksh93/sh/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -503,21 +503,14 @@ static char* get_ifs(register Namval_t* np, Namfun_t *fp)
memset(shp->ifstable,0,(1<<CHAR_BIT));
if(cp=value)
{
#if SHOPT_MULTIBYTE
while(n=mbsize(cp),c= *(unsigned char*)cp)
#else
while(c= *(unsigned char*)cp++)
#endif /* SHOPT_MULTIBYTE */
while(n = mbsize(cp), c = *(unsigned char*)cp++)
{
#if SHOPT_MULTIBYTE
cp++;
if(n>1)
{
cp += (n-1);
shp->ifstable[c] = S_MBYTE;
continue;
}
#endif /* SHOPT_MULTIBYTE */
n = S_DELIM;
if(c== *cp)
cp++;
Expand All @@ -533,6 +526,7 @@ static char* get_ifs(register Namval_t* np, Namfun_t *fp)
shp->ifstable[' '] = shp->ifstable['\t'] = S_SPACE;
shp->ifstable['\n'] = S_NL;
}
shp->ifstable[0] = S_EOF;
}
return(value);
}
Expand Down
22 changes: 22 additions & 0 deletions src/cmd/ksh93/tests/builtins.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1055,5 +1055,27 @@ do case $bltin in
"(expected string containing $(printf %q "$expect"), got $(printf %q "$actual"))"
done 3< <(builtin)
# ======
# The 'alarm' builtin could make 'read' crash due to IFS table corruption caused by unsafe asynchronous execution.
# https://bugzilla.redhat.com/1176670
if (builtin alarm) 2>/dev/null
then got=$( { "$SHELL" -c '
builtin alarm
alarm -r alarm_handler +.001
i=0
function alarm_handler.alarm
{
let "(++i) > 100" && exit
}
while :; do
echo cargo,odds and ends,jetsam,junk,wreckage,castoffs,sea-drift
done | while IFS="," read arg1 arg2 arg3 arg4 junk; do
:
done
'; } 2>&1)
((!(e = $?))) || err_exit 'crash with alarm and IFS' \
"(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))"
fi
# ======
exit $((Errors<125?Errors:125))

0 comments on commit 18b3f4a

Please sign in to comment.