Skip to content

Lock tty in sudo before the password prompt shows - #1651

Open
squell wants to merge 1 commit into
mainfrom
fix-tty-contention
Open

Lock tty in sudo before the password prompt shows#1651
squell wants to merge 1 commit into
mainfrom
fix-tty-contention

Conversation

@squell

@squell squell commented Jul 8, 2026

Copy link
Copy Markdown
Member

Replaces #1649, closes #1650

Comment thread src/system/term/mod.rs Outdated
@squell
squell force-pushed the fix-tty-contention branch from a54b0b6 to 51d0d5f Compare July 8, 2026 09:59
Comment thread src/system/file/lock.rs Outdated
Comment thread src/system/term/mod.rs Outdated
@squell
squell force-pushed the fix-tty-contention branch from 51d0d5f to 54a0bf8 Compare July 9, 2026 09:04
Comment thread src/sudo/pipeline.rs
None
} else {
lock_tty()
};

@bjorn3 bjorn3 Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe put this above attempt_authenticate? No need to lock the tty if we aren't going to ask for a password.
Edit: Ah, this is to avoid two sudo instances asking for a password. Maybe add a comment to that effect?

@squell squell Jul 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other open question that I have: why would we be we locking /dev/tty in particular? We could lock anything arbitrary (e.g. /etc/sudoers), right. The only important thing here would be that both sudo-rs instances agree on that the lock is (and if possble: that ogsudo and sudo-rs also agree in the oddball case that they are both in a pipeline -- but that is very much a "nice to have but not important" territory).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like ogsudo locks /dev/tty:

$ target/debug/sudo dash -c 'echo yes'  | /bin/sudo  dash -c 'sed s/yes/no/'
[sudo] password for squell: [sudo: authenticate] Password: 

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

Successfully merging this pull request may close these issues.

A pipeline of two sudo invocations causes TTY contention

2 participants