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

Fish invoked from sudo stops itself #7388

Closed
Nikratio opened this issue Oct 9, 2020 · 18 comments
Closed

Fish invoked from sudo stops itself #7388

Nikratio opened this issue Oct 9, 2020 · 18 comments
Labels
bug Something that's not working as intended
Milestone

Comments

@Nikratio
Copy link

Nikratio commented Oct 9, 2020

When I'm running sudo, and root is using the fish shell, the new shell immediately stops:

nikratio@vostro ~> fish --version
fish, version 3.0.2

nikratio@vostro ~> sudo cat /root/.bash_profile
echo exec /usr/bin/fish -i
exec /usr/bin/fish -i

nikratio@vostro ~> sudo -i
exec /usr/bin/fish -i
Job 1, 'sudo -i' has stopped

Removing the exec from .bash_profile and executing it manually works just fine:

nikratio@vostro ~> sudo cat /root/.bash_profile
#echo  exec /usr/bin/fish -i
#exec /usr/bin/fish -i

nikratio@vostro ~> sudo -i

[0] root@vostro:~
# exec /usr/bin/fish -i
Welcome to fish, the friendly interactive shell
root@vostro ~# 

Am I doing something wrong?

@faho
Copy link
Member

faho commented Oct 9, 2020

Please upgrade to 3.1.2 there were a few fixes in that area. We offer numerous PPAs at fishshell.com.

@Nikratio
Copy link
Author

Nikratio commented Oct 9, 2020

Same problem, I'm afraid:

nikratio@vostro ~> sudo -i
Job 1, 'sudo -i' has stopped
nikratio@vostro ~> fish --version
fish, version 3.1.2

@faho
Copy link
Member

faho commented Oct 9, 2020

Okay, do you have anything in config.fish? Can you try moving that out of the way?

I can't reproduce this on 3.1.2 or 3.0.2.

@Nikratio
Copy link
Author

Nikratio commented Oct 9, 2020

Nope, config.fish is empty.

@Nikratio
Copy link
Author

Nikratio commented Oct 9, 2020

(for both root and the user calling sudo)

@faho
Copy link
Member

faho commented Oct 9, 2020

What operating system is this on?

Do you have anything else in bashrc or profile?

@faho
Copy link
Member

faho commented Oct 9, 2020

Compare #5147, which was supposedly fixed in 3.0.0 (and never in a release, AFAICT)

I don't suppose it's possible the fish in your $PATH isn't updated, so sudo -i ends up calling an older fish?

Or do you have any sudo configuration?

@Nikratio
Copy link
Author

Nikratio commented Oct 9, 2020

This is on Debian stable.

There are the standard bash init files in /etc that come with Debian, but /root/.bashrc contains nothing else.

Here's the sudo configuration:

# cat /etc/sudoers
#
# This file MUST be edited with the 'visudo' command as root.
#
# Please consider adding local content in /etc/sudoers.d/ instead of
# directly modifying this file.
#
# See the man page for details on how to write a sudoers file.
#
Defaults	env_reset
Defaults	secure_path="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"

# Don't modify user's umask
Defaults        umask=0777

# Host alias specification

# User alias specification

# Cmnd alias specification

# User privilege specification
root	ALL=(ALL:ALL) ALL

# Allow members of group sudo to execute any command
%sudo	ALL=(ALL:ALL) ALL

#includedir /etc/sudoers.d

The fish version is the right one:

nikratio@vostro ~/i/pyfuse3 (master)> cat /root/.bashrc
#
# ~/.bashrc (executed by bash(1))
# Settings for interactive subshells
#

# Use fish shell instead
if [[ $- = *i* && -z $WHICH_FISH ]] && WHICH_FISH=$(type -p fish); then
  # Safeguard to only activate fish for interactive shells and only if fish
  # shell is present and executable. Verify that this is a new session by
  # checking if $WHICH_FISH is set. If it is not set, we export WHICH_FISH
  # and start fish.
  #
  # If this is not a new session, the user probably typed 'bash' into their
  # console and wants bash, so we skip this.
  #
  # Note that we do not set $SHELL, and neither does fish, because doing so
  # tends to interfere with programs that expect $SHELL to point to a
  # POSIX-compatible shell, which fish is not.
  export WHICH_FISH
  echo "Running exec \"$WHICH_FISH\" -i"
  exec "$WHICH_FISH" -i
fi

nikratio@vostro ~/i/pyfuse3 (master)> sudo -i
[sudo] password for nikratio: 
Running exec "/usr/bin/fish" -i
Job 1, 'sudo -i' has stopped
nikratio@vostro ~/i/pyfuse3 (master)> /usr/bin/fish --version
fish, version 3.1.2

@Nikratio
Copy link
Author

Nikratio commented Oct 9, 2020

It looks like it actually stops twice before it becomes usable:

nikratio@vostro ~/i/pyfuse3 (master)> sudo -i
Running exec "/usr/bin/fish" -i
Job 1, 'sudo -i' has stopped
nikratio@vostro ~/i/pyfuse3 (master)> fg
Send job 1, “sudo -i” to foreground
Job 1, 'sudo -i' has stopped
nikratio@vostro ~/i/pyfuse3 (master)> fg
Send job 1, “sudo -i” to foreground
Welcome to fish, the friendly interactive shell

@zanchey
Copy link
Member

zanchey commented Oct 11, 2020

I can reproduce this on 3.1.2 and on f73ee30 with the .bashrc given above. I've managed to get a (massive) strace of the whole thing, but I don't really have time to dig into it. I suspect funkiness with the PGID handling: when stopped, the output of ps -Ao user,pid,pgid,command includes:

root     30123 30123 sudo -i
root     30124 30123 /usr/bin/fish -i

but then after running fg twice, looks more sensible with:

root     30123 30123 sudo -i
root     30124 30124 /usr/bin/fish -i

@zanchey zanchey added bug Something that's not working as intended and removed needs more info labels Oct 11, 2020
@zanchey zanchey added this to the fish-future milestone Oct 11, 2020
@ridiculousfish
Copy link
Member

I have not been able to reproduce on Arch; I will try on a Debian system next.

@faho
Copy link
Member

faho commented Oct 12, 2020

Okay, I can reproduce on Debian (also not on Arch) with fish 3.1.2 , I can't with current-ish master (ca730cf).

Lemme bisect.

Edit: No, master also exhibits the issue. Must not have restarted correctly after applying some changes to my config.

@faho
Copy link
Member

faho commented Oct 12, 2020

Welp, this is fish's doing.

In

fish-shell/src/reader.cpp

Lines 2053 to 2067 in 13459d4

if (check_for_orphaned_process(loop_count, shell_pgid)) {
// We're orphaned, so we just die. Another sad statistic.
const wchar_t *fmt =
_(L"I appear to be an orphaned process, so I am quitting politely. "
L"My pid is %d.");
FLOGF(warning, fmt, static_cast<int>(getpid()));
exit_without_destructors(1);
}
// Try stopping us.
int ret = killpg(shell_pgid, SIGTTIN);
if (ret < 0) {
wperror(L"killpg(shell_pgid, SIGTTIN)");
exit_without_destructors(1);
}
, fish sends a SIGTTIN to itself, in the hopes of somehow (I don't understand the particular mechanism, tbh) becoming pgroup leader.

Only it actually ends up stopping itself.

I don't understand what this is supposed to achieve given that we reset the signal handlers to default, and IIRC for SIGTTIN that's stopping?

@krobelus
Copy link
Member

I believe fish doesn't stop on SIGTTIN because in set_interactive_handlers we set a handler for that signal.
No idea why it stops in this case, it doesn't reproduce on Ubuntu 18.

@faho
Copy link
Member

faho commented Oct 12, 2020

@krobelus Doesn't signal_reset_handlers() reset the signal handlers to default, or am I reading that wrong?

@ridiculousfish
Copy link
Member

ridiculousfish commented Oct 18, 2020

I was able to repro on Debian too. Some background:

Something that may happen is a shell is started interactively, but does not have control of the terminal. For example bash -i &. As soon as it tries to do anything it will get SIGTTIN, but interactive shells ignore SIGTTIN. So the operation will fail and it will try again, leading to a 100% CPU loop.

Shells try to detect this case by comparing their pgroup (getpgrp) to the tty owner (tcgetpgrp). If different, they temporarily remove their SIGTTIN handler, send SIGTTIN to themselves, and wait to be resumed. Here's where bash does just that.

What ends up happening here is very strange: fish is launched in sudo's pgroup, but the tty is assigned to fish's pid (NOT its pgroup). It's extremely weird to assign a tty to a non-pgroup leader. This seems like a sudo bug. It might also be a funny edge case in bash, where it assigns the tty to itself but does not become pgroup leader, and then execs fish in that strange state.

I'll have to see what bash does differently here, but both bash and fish have other code to fix up its pgroup, so bash is probably just working around this.

@ridiculousfish
Copy link
Member

ridiculousfish commented Oct 18, 2020

Ha, bash doesn't do anything different here. If you launch bash instead of fish then bash gets stopped too.

If fish is launched such that its pid (but not the pgrp) owns the tty, then it should probably just put itself into its own pgroup. I can't imagine what that would break. But yeah, this seems like a sudo or bash bug.

@ridiculousfish
Copy link
Member

"bbbbe00" sweet

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something that's not working as intended
Projects
None yet
Development

No branches or pull requests

5 participants