Skip to content

Commit

Permalink
Fix possible crash due to failure to update shell FD state
Browse files Browse the repository at this point in the history
This applies ksh-20100621-fdstatus.patch from Red Hat. Not very
much information is available, so this one is more or less taken
on faith. But it seems to make sense on the face of it: calling
sh_fcntl() instead of fcntl(2) directly makes the shell update its
internal file descriptor state more frequently.

It claims to fix Red Hat bug 924440. The report is currently closed
to the public: https://bugzilla.redhat.com/show_bug.cgi?id=924440

However, Kamil Dudka at Red Hat writes:
#67 (comment)
| Yes, the summary of RHBZ#924440 is "crash in bestreclaim() after
| traversing a memory block with a very large size". We did not have
| any in house reproducer for the bug. The mentioned patch was
| provided and verified by a customer.

...and Marc Wilson dug up a Red Hat erratum containing this info:
https://download.rhn.redhat.com/errata/RHBA-2013-1599.html
| Previously, the ksh shell did not resize the file descriptor list
| every time it was necessary. This could lead to memory corruption
| when several file descriptors were used. As a consequence, ksh
| terminated unexpectedly. This updated version resizes the file
| descriptor list every time it is needed, and ksh no longer
| crashes in the described scenario. (BZ#924440)

No reproducer means no regression test can be added now.

src/cmd/ksh93/sh/io.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Change several fcntl(2) calls to sh_fcntl(). This function calls
  fcntl(2) and then updates the shell's file descriptor state.
  • Loading branch information
McDutchie committed Jul 10, 2020
1 parent c4236cc commit 778fd6c
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 5 deletions.
3 changes: 3 additions & 0 deletions NEWS
Expand Up @@ -8,6 +8,9 @@ Any uppercase BUG_* names are modernish shell bug IDs.
- Fixed a bug that caused types created with 'typeset -T' to throw an error
when used if the type name started with a lowercase 'a'.

- A potential crash due to memory corruption when using many file
descriptors has been fixed.

2020-07-09:

- Fixed a crash on syntax error when sourcing/dotting multiple files.
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/io.c
Expand Up @@ -1503,7 +1503,7 @@ int sh_redirect(Shell_t *shp,struct ionod *iop, int flag)
fn = fd;
if(fd<10)
{
if((fn=fcntl(fd,F_DUPFD,10)) < 0)
if((fn=sh_fcntl(fd,F_DUPFD,10)) < 0)
goto fail;
if(fn>=shp->gd->lim.open_max && !sh_iovalidfd(shp,fn))
goto fail;
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/ksh93/sh/subshell.c
Expand Up @@ -121,7 +121,7 @@ void sh_subtmpfile(Shell_t *shp)
register struct checkpt *pp = (struct checkpt*)shp->jmplist;
register struct subshell *sp = subshell_data->pipe;
/* save file descriptor 1 if open */
if((sp->tmpfd = fd = fcntl(1,F_DUPFD,10)) >= 0)
if((sp->tmpfd = fd = sh_fcntl(1,F_DUPFD,10)) >= 0)
{
fcntl(fd,F_SETFD,FD_CLOEXEC);
shp->fdstatus[fd] = shp->fdstatus[1]|IOCLEX;
Expand Down Expand Up @@ -541,7 +541,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
sp->pwdfd = n;
if(n<10)
{
sp->pwdfd = fcntl(n,F_DUPFD,10);
sp->pwdfd = sh_fcntl(n,F_DUPFD,10);
close(n);
}
if(sp->pwdfd>0)
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/ksh93/sh/xec.c
Expand Up @@ -115,7 +115,7 @@ static int iousepipe(Shell_t *shp)
return(0);
usepipe++;
fcntl(subpipe[0],F_SETFD,FD_CLOEXEC);
subpipe[2] = fcntl(1,F_DUPFD,10);
subpipe[2] = sh_fcntl(1,F_DUPFD,10);
fcntl(subpipe[2],F_SETFD,FD_CLOEXEC);
shp->fdstatus[subpipe[2]] = shp->fdstatus[1];
close(1);
Expand Down Expand Up @@ -3613,7 +3613,7 @@ static void coproc_init(Shell_t *shp, int pipes[])
sh_pipe(shp->cpipe);
if((outfd=shp->cpipe[1]) < 10)
{
int fd=fcntl(shp->cpipe[1],F_DUPFD,10);
int fd=sh_fcntl(shp->cpipe[1],F_DUPFD,10);
if(fd>=10)
{
shp->fdstatus[fd] = (shp->fdstatus[outfd]&~IOCLEX);
Expand Down

0 comments on commit 778fd6c

Please sign in to comment.