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.

Sign up for GitHub

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

Jump to bottom

vipw/vigr affect bash because it changes to PGID. #1194

Open
yonecle opened this issue Jan 21, 2025 · 2 comments · May be fixed by #1196
Open

vipw/vigr affect bash because it changes to PGID. #1194

yonecle opened this issue Jan 21, 2025 · 2 comments · May be fixed by #1196

Comments

@yonecle
Copy link

yonecle commented Jan 21, 2025 •
edited
Loading

To fix vipw/vigr not resuming when suspended, the following commit had been added to change pgid(ie. pgrp)

Fix vipw not resuming correctly when suspended

However, vipw/vigr changes to bash's PGID, which affect bash.

[root@rhel9 ~]# ps axo stat,euid,ruid,tty,tpgid,sess,pgrp,ppid,pid,pcpu,comm
STAT  EUID  RUID TT         TPGID    SESS    PGRP    PPID     PID %CPU COMMAND 
__snip__
Ss       0     0 pts/2      10607    4070    4070    3571    4070  0.0 bash
S        0     0 pts/2      10607    4070    4070    4070   10606  0.0 vipw
S+       0     0 pts/2      10607    4070   10607   10606   10607  0.0 vipw
S+       0     0 pts/2      10607    4070   10607   10607   10608  0.1 vim

Steps to reproduce

Problem#1

  1. Disable monitor-mode in bash.

# set +m

  1. Run vipw or vigr

# vipw

After exiting the editor, bash exists on its own.

[root@rhel9 ~]# strace -ffvv -p $bash_pid
__snip__
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 10606
__snip__
rt_sigprocmask(SIG_BLOCK, [CHLD], [], 8) = 0
rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
setpgid(0, 10658)                       = -1 EPERM (Operation not permitted)
exit_group(0)                           = ?
+++ exited with 0 +++

Problem#2

  1. Create the following script file:
#!/bin/bash
/usr/sbin/vipw
echo Enter any key
read -s
  1. Run the script
[root@rhel9 ~]# chmod +x test.sh
[root@rhel9 ~]# ./test.sh

The script stops by SIGTTOU.

[root@rhel9 ~]# ./test.sh 
vipw: /etc/passwd is unchanged
Enter any key

[1]+  Stopped                 ./test.sh
[root@rhel9 ~]# jobs -l
[1]+ 61799 Stopped (tty output)    ./test.sh
@alejandro-colomar
Copy link
Collaborator

alejandro-colomar commented Jan 23, 2025 •
edited
Loading

Steps to reproduce

Problem#1

1. Disable monitor-mode in bash.

# set +m 

2. Run vipw or vigr

# vipw

After exiting the editor, bash exists on its own.

I can reproduce:

alx@devuan:~$ sudo bash
[sudo] password for alx: 
root@devuan:/home/alx# set +m
root@devuan:/home/alx# vipw
vipw: /etc/passwd is unchanged
root@devuan:/home/alx# 
exit
alx@devuan:~$

Cc: @millert

Should we just revert the commit? Or does anyone have a better idea?

millert added a commit to millert/shadow that referenced this issue Jan 24, 2025
@millert
Restore the original terminal pgrp after editing
d61b626 
This fixes a problem when the shell is not in monitor mode (job control
enabled) which resulted in the terminal pgrp being set to an invalid
value once vipw exited.  Fixes shadow-maint#1194.
@millert millert linked a pull request Jan 24, 2025 that will close this issue
Open
@millert
Copy link
Contributor

millert commented Jan 24, 2025

There is a missing tcsetpgrp() to restore the original terminal process group. It is fixed by #1196.

alejandro-colomar pushed a commit to millert/shadow that referenced this issue Jan 24, 2025
@millert @alejandro-colomar
src/vipw.c: Restore the original terminal pgrp after editing
7dcb9a0 
This fixes a problem when the shell is not in monitor mode (job control
enabled) which resulted in the terminal pgrp being set to an invalid
value once vipw exited.

Fixes: 7eca111 (2019-11-11; "Fix vipw not resuming correctly when suspended")
Closes: <shadow-maint#1194>
Reported-by: <https://github.com/yonecle>
Tested-by: Alejandro Colomar <[email protected]>
Reviewed-by: Alejandro Colomar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Restore the original terminal pgrp after editing millert/shadow
3 participants
@millert @yonecle @alejandro-colomar