Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: ksh segfaults when unsetting SHLVL then exec'ing #788

Open
vmihalko opened this issue Sep 23, 2024 · 1 comment
Open

Regression: ksh segfaults when unsetting SHLVL then exec'ing #788

vmihalko opened this issue Sep 23, 2024 · 1 comment

Comments

@vmihalko
Copy link

What happens?

If the user unsets the SHLVL variable and later replaces the shell by executing a command, ksh will segfault.

Reproducer

# unset -v SHLVL
# exec bash
Segmentation fault (core dumped)

Reason

The reason for this is that the SHLVL variable is getting decremented without any guard making sure it's still in the environment, see:

/* if the main shell is about to be replaced, decrease SHLVL to cancel out a subsequent increase */
if(!sh.realsubshell)
(*SHLVL->nvalue.ip)--;

Fix

I tested the following simple fix, and the segmentation fault no longer occurs.

diff --git a/src/cmd/ksh93/bltins/misc.c b/src/cmd/ksh93/bltins/misc.c
index cb7e883b..0e09b9ea 100644
--- a/src/cmd/ksh93/bltins/misc.c
+++ b/src/cmd/ksh93/bltins/misc.c
@@ -143,7 +143,7 @@ int    b_exec(int argc,char *argv[], Shbltin_t *context)
                if(job_close() < 0)
                        return 1;
                /* if the main shell is about to be replaced, decrease SHLVL to cancel out a subsequent increase */
-               if(!sh.realsubshell)
+               if(!sh.realsubshell && SHLVL->nvalue.ip)
                        (*SHLVL->nvalue.ip)--;
                sh_onstate(SH_EXEC);
                if(sh.subshell && !sh.subshare)

Should I use some other way to test if SHLVL is set, or is this OK, and I can create a PR?

@posguy99
Copy link

posguy99 commented Sep 25, 2024

Why is SHLVL unsettable? Shouldn't that generate some sort of error?

Edit... NVM, the man page actually refers to a case where it's not in the environment, so it's on purpose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants