* [BUG] Interactive (d)ash exits on assigning to readonly from 'command eval' @ 2016-10-27 21:40 Martijn Dekker 2016-10-28 13:55 ` Denys Vlasenko 0 siblings, 1 reply; 4+ messages in thread From: Martijn Dekker @ 2016-10-27 21:40 UTC (permalink / raw) To: dash, busybox This bug is on both dash and busybox ash. The "command" builtin is supposed to stop special builtins (such as "eval") from exiting the shell on error. So doing something like isreadonly() { ! command eval "$1=" 2>/dev/null } ought to be a way to test if a variable is read-only without the performance hit of forking a subshell. Using this function works fine in scripts, but it immediately makes an interactive dash or ash exit. The same happens if you try a similar command manually. bash$ dash $ readonly bla=123 $ command eval bla=457 dash: 1: eval: bla: is read only $ bash$ After the "is read only" error, dash prints a prompt, but does not wait for input and exits instead. Busybox ash does not even print the prompt before exiting. The funny part is that the interactive shell does not exit if you leave out the 'command'. The very thing that is supposed to stop a non-interactive shell from exiting makes an interactive shell exit. HTH, - M. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [BUG] Interactive (d)ash exits on assigning to readonly from 'command eval' 2016-10-27 21:40 [BUG] Interactive (d)ash exits on assigning to readonly from 'command eval' Martijn Dekker @ 2016-10-28 13:55 ` Denys Vlasenko 2016-10-29 18:22 ` Harald van Dijk 0 siblings, 1 reply; 4+ messages in thread From: Denys Vlasenko @ 2016-10-28 13:55 UTC (permalink / raw) To: Martijn Dekker, Herbert Xu; +Cc: dash, busybox This will probably be mangled by gmail, but here is the proposed fix: Date: Fri, 28 Oct 2016 15:43:50 +0200 Subject: [PATCH] ash: fix interactive "command eval STRING" exiting on errors. This bug is also present in current dash Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com> --- shell/ash.c | 25 ++++++++++++++++++++++++- shell/ash_test/ash-vars/readonly1.right | 2 ++ shell/ash_test/ash-vars/readonly1.tests | 7 +++++++ 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 shell/ash_test/ash-vars/readonly1.right create mode 100755 shell/ash_test/ash-vars/readonly1.tests diff --git a/shell/ash.c b/shell/ash.c index 1ef02b8..fe11245 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -2180,6 +2180,7 @@ setvareq(char *s, int flags) if (flags & VNOSAVE) free(s); n = vp->var_text; + exitstatus = 1; ash_msg_and_raise_error("%.*s: is read only", strchrnul(n, '=') - n, n); } @@ -9599,7 +9600,7 @@ evalcommand(union node *cmd, int flags) if (evalbltin(cmdentry.u.cmd, argc, argv, flags)) { if (exception_type == EXERROR && spclbltin <= 0) { FORCE_INT_ON; - break; + goto readstatus; } raise: longjmp(exception_handler->loc, 1); @@ -12280,6 +12281,10 @@ expandstr(const char *ps) static int evalstring(char *s, int flags) { + struct jmploc *volatile savehandler = exception_handler; + struct jmploc jmploc; + int ex; + union node *n; struct stackmark smark; int status; @@ -12289,6 +12294,19 @@ evalstring(char *s, int flags) setstackmark(&smark); status = 0; + /* On exception inside execution loop, we must popfile(). + * Try interactively: + * readonly a=a + * command eval "a=b" # throws "is read only" error + * "command BLTIN" is not supposed to abort (even in non-interactive use). + * But if we skip popfile(), we hit EOF in eval's string, and exit. + */ + savehandler = exception_handler; + exception_handler = &jmploc; + ex = setjmp(jmploc.loc); + if (ex) + goto out; + while ((n = parsecmd(0)) != NODE_EOF) { int i; @@ -12299,10 +12317,15 @@ evalstring(char *s, int flags) if (evalskip) break; } + out: popstackmark(&smark); popfile(); stunalloc(s); + exception_handler = savehandler; + if (ex) + longjmp(exception_handler->loc, ex); + return status; } diff --git a/shell/ash_test/ash-vars/readonly1.right b/shell/ash_test/ash-vars/readonly1.right new file mode 100644 index 0000000..2b363e3 --- /dev/null +++ b/shell/ash_test/ash-vars/readonly1.right @@ -0,0 +1,2 @@ +One:1 +One:1 diff --git a/shell/ash_test/ash-vars/readonly1.tests b/shell/ash_test/ash-vars/readonly1.tests new file mode 100755 index 0000000..81b461f --- /dev/null +++ b/shell/ash_test/ash-vars/readonly1.tests @@ -0,0 +1,7 @@ +readonly bla=123 +# Bare "eval bla=123" should abort ("eval" is a special builtin): +(eval bla=123 2>/dev/null; echo BUG) +echo One:$? +# "command BLTIN" disables "special-ness", should not abort: +command eval bla=123 2>/dev/null +echo One:$? -- 2.9.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [BUG] Interactive (d)ash exits on assigning to readonly from 'command eval' 2016-10-28 13:55 ` Denys Vlasenko @ 2016-10-29 18:22 ` Harald van Dijk 2016-10-30 17:32 ` Denys Vlasenko 0 siblings, 1 reply; 4+ messages in thread From: Harald van Dijk @ 2016-10-29 18:22 UTC (permalink / raw) To: Denys Vlasenko, Martijn Dekker, Herbert Xu; +Cc: dash, busybox On 28/10/16 15:55, Denys Vlasenko wrote: > This will probably be mangled by gmail, but here is the proposed fix: This looks about the right approach, but it causes problems in subshells, a double free: $ ./busybox ash -c 'readonly x; echo $(command eval x=2)' ash: eval: line 1: x: is read only *** Error in `./busybox': free(): invalid pointer: 0x000055a784c1c300 *** [...] That's with busybox checked out from git (commit 9db74e49e5b462089c6eec0182d819c0d4708e57), where your patch is applied, completely unpatched and completely default config. I omitted the backtrace output, but it's popfile() getting called, after popallfiles() has already been called. Cheers, Harald van Dijk ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [BUG] Interactive (d)ash exits on assigning to readonly from 'command eval' 2016-10-29 18:22 ` Harald van Dijk @ 2016-10-30 17:32 ` Denys Vlasenko 0 siblings, 0 replies; 4+ messages in thread From: Denys Vlasenko @ 2016-10-30 17:32 UTC (permalink / raw) To: Harald van Dijk; +Cc: Martijn Dekker, Herbert Xu, dash, busybox On Sat, Oct 29, 2016 at 8:22 PM, Harald van Dijk <harald@gigawatt.nl> wrote: > On 28/10/16 15:55, Denys Vlasenko wrote: >> >> This will probably be mangled by gmail, but here is the proposed fix: > > > This looks about the right approach, but it causes problems in subshells, a > double free: > > $ ./busybox ash -c 'readonly x; echo $(command eval x=2)' > ash: eval: line 1: x: is read only > *** Error in `./busybox': free(): invalid pointer: 0x000055a784c1c300 *** > [...] > > That's with busybox checked out from git (commit > 9db74e49e5b462089c6eec0182d819c0d4708e57), where your patch is applied, > completely unpatched and completely default config. > > I omitted the backtrace output, but it's popfile() getting called, after > popallfiles() has already been called. Thanks! Hopefully fixed in git, please try it. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-10-30 17:32 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-27 21:40 [BUG] Interactive (d)ash exits on assigning to readonly from 'command eval' Martijn Dekker 2016-10-28 13:55 ` Denys Vlasenko 2016-10-29 18:22 ` Harald van Dijk 2016-10-30 17:32 ` Denys Vlasenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).