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

Trying to use exec to replace a not-interactive process with an interactive one doesn't work inside su #461

Open
ale5000-git opened this issue Oct 5, 2024 · 18 comments

Comments

@ale5000-git
Copy link

ale5000-git commented Oct 5, 2024

Trying to use exec to replace a not-interactive process with an interactive one doesn't work inside su.

Example:
busybox su -c 'exec sh -s -c "echo 123"'

It should remain open but it didn't (even after adding -N).

@rmyorston
Copy link
Owner

It's a regression caused by 074ebfc.

If you need this to work the previous prerelease (PRE-5462) is still available.

rmyorston added a commit that referenced this issue Oct 6, 2024
It was noted this command didn't work properly:

   su -t -c 'exec sh -s -c "echo 123"'

The child shell performed the 'echo' but then exited, despite the
'-s' flag.  This is a regression introduced by commit 074ebfc
(ash: code shrink).

This simpler command also failed the same way:

   sh -c "exec sh -s"

This regression dates back even further, to commit da7c8cd
(ash: run ash_main() directly from a FS_SHELLEXEC shell).

The issue can be avoided if shells invoked by the 'exec' builtin
aren't run by calling ash_main() directly.

Adds 80-96 bytes.

GitHub issue #461.
@rmyorston
Copy link
Owner

The similar command sh -c "exec sh -s" didn't work either and has been broken since FRP-3532.

Try the latest prerelease (PRE-5487 or above).

@ale5000-git
Copy link
Author

ale5000-git commented Oct 6, 2024

Thanks but unfortunately it doesn't fix all cases.
I first thought the problem was only with exec but instead the problem is also with su.

This is the behavior of the old PRE-5462 version:

CASE 1
busybox su -c 'exec sh -s -c "echo 123"'
It remain interactive and when typing 'exit' it exit

CASE 2
busybox su -N -c 'exec sh -s -c "echo 123"'
It remain interactive and when typing 'exit' it exit (not sure if it is correct to skip 'Press any key to exit...')

CASE 3
busybox su -c 'sh -s -c "echo 123"'
It remain interactive and when typing 'exit' it exit

CASE 4
busybox su -N -c 'sh -s -c "echo 123"'
It remain interactive and when typing 'exit' it display 'Press any key to exit...' and after pressing any key it exit


On the latest version the case 3 broke but strangely the case 4 is working.

@ale5000-git
Copy link
Author

ale5000-git commented Oct 6, 2024

@rmyorston

I noticed another issue, this:
sh -c 'sh'
should enter in an interactive subshell but instead it just remain not-interactive.

rmyorston added a commit that referenced this issue Oct 7, 2024
- Change the 'flags' argument to shellexec()/tryexec() so it only
  indicates that it's permissible to run ash_main() directly.  The
  flag is only set TRUE in forkshell_shellexec().

- The check for a script with an interpreter which is an applet is
  now performed for all calls to tryexec(), not just those that
  originate from evalcommand().

Saves 64-80 bytes.

GitHub issue #461.
@rmyorston
Copy link
Owner

In case 2, sh -N passes the -N argument to the shell so it can issue the prompt on exit, but in your case the shell is being replaced due to the exec. Use the (undocumented) -N on the inner shell instead of su -N.

Case 4 works because the shell being forced to run in a separate process when the -N flag is used.

The other issues might be fixed in the latest prerelease (PRE-5488 or above). I'm sure you'll let me know if they aren't.

@ale5000-git
Copy link
Author

ale5000-git commented Oct 7, 2024

Thanks, now the interactive/non-interactive mode works fine in all cases.

Now I tried to check exec.
I usually use exec to avoid keeping the previous process open, but it seems to make no difference.

First I open busybox ash (total busybox processes: 1)

then:

  • I execute su (total busybox processes: 2)
  • or I execute su -c 'sh' (total busybox processes: 3)
  • or I execute su -c 'exec sh' (total busybox processes: 3)

@rmyorston
Copy link
Owner

Windows doesn't have an exec() system call so the implementation of the exec shell builtin can't work as it does on Unix. The shell tries to reuse processes where it can but there are still going to be cases where we end up having more processes than on Unix.

@ale5000-git
Copy link
Author

ale5000-git commented Oct 7, 2024

Windows doesn't have an exec() system call so the implementation of the exec shell builtin can't work as it does on Unix. The shell tries to reuse processes where it can but there are still going to be cases where we end up having more processes than on Unix.

It is good to reuse processes but what I was asking is different.
Isn't it possible that when using exec it create a new process and then the old process close itself without waiting for the new process to end?

@rmyorston
Copy link
Owner

That wouldn't work.

The parent of the old process is waiting for it to exit (assuming it's running in foreground):

~ $ sh -c 'exit 42'; echo $?
42

On Unix if the old process execs a different binary the process id remains the same and the parent is still waiting for that PID to exit:

~ $ sh -c 'echo $$; exec sh -c "echo \$$; exit 42"'; echo $?
10110
10110
42

On Windows the exec creates a new process. The old process waits for the new process to die and passes its exit status on to its parent. So the exit status of the command above is just the same, though the details are different:

~ $ sh -c 'echo $$; exec sh -c "echo \$$; exit 42"'; echo $?
2744
2776
42

If the old process were to close itself the parent would get its exit status, not that of the execed process. There's no way to tell the parent process it needs to wait for a different PID.

@ale5000-git
Copy link
Author

If the su process (process 2) isn't called with -W then the caller (process 1) won't wait for the su process (process 2) to exit, so when the su process (process 2) get replaced with exec (process 3) then why the process 2 can't close itself?

@rmyorston
Copy link
Owner

Process 2 doesn't know whether or not its parent is waiting for it.

Yes, we could pass that information to it. And yes, we could have process 2 close itself if it then execs something else and knows its parent doesn't care. (Probably. I haven't worked out all the details.)

But I doubt it's worth the effort of implementing this - just to allow one process to be closed early in what seems to be a very, very, very rare situation.

@ale5000-git
Copy link
Author

ale5000-git commented Oct 9, 2024

If I'm not wrong we already know it, when $PPID is 1 the process can close itself without problems (I think).
This would also apply to other cases where the parent is gone.


su
and then:
echo $PPID
result in
1


su -W
and then:
echo $PPID
result in
10716

@rmyorston
Copy link
Owner

Yes, that seems to work, and at a very modest cost.

I'll need to think about it some more, though...

@rmyorston
Copy link
Owner

This would also apply to other cases where the parent is gone.

What other cases did you have in mind?

@ale5000-git
Copy link
Author

What other cases did you have in mind?

I haven't thought too much about other cases but I can think of one example.

For example if a script (near its the end) execute code in background (with &), then the script end and since the command in background no longer have a parent it can be safely replaced when needed.

rmyorston added a commit that referenced this issue Oct 13, 2024
If a process performing an exec is an orphan there's no reason for
it to wait for its child's exit code.  Let it exit immediately.

Adds 16 bytes.

(GitHub issue #461)
@rmyorston
Copy link
Owner

I'm still a bit uneasy about this. But, for what it's worth, the latest prerelease (PRE-5511 or above) allows certain intermediate processes to exit early.

Be careful out there!

@ale5000-git
Copy link
Author

ale5000-git commented Oct 13, 2024

I tried in various ways in a new window created with su but also with a separate window created using cmd.exe /c start busybox ash and it seems to work perfectly.
Now, at least in these cases, even I use 20 exec the number of processes does not increase.
It seems a good improvement.

@ale5000-git
Copy link
Author

ale5000-git commented Oct 14, 2024

I also tried this:

my_func() { sleep 8; exec sh -c 'echo "test" > my_file.txt; sleep 25'; }
my_func &
kill $$

An it seems to behave properly even when the main process is terminated.

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