Skip to content

Commit fabb626

Browse files
millertmillert
committed
Better handling of multiple sudo processes modifying terminal settings.
1. Lock the terminal before tcgetattr/tcsetattr 2. Don't restore terminal settings if changed by another process 3. Don't set terminal to raw mode if it is already raw GitHub issue #312
1 parent 2f80865 commit fabb626

File tree

1 file changed

+209
-75
lines changed

1 file changed

+209
-75
lines changed

‎lib/util/term.c

+209-75
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*
22
* SPDX-License-Identifier: ISC
33
*
4-
* Copyright (c) 2011-2015, 2017-2020 Todd C. Miller <Todd.Miller@sudo.ws>
4+
* Copyright (c) 2011-2015, 2017-2023 Todd C. Miller <Todd.Miller@sudo.ws>
55
*
66
* Permission to use, copy, modify, and distribute this software for any
77
* purpose with or without fee is hereby granted, provided that the above
@@ -82,12 +82,16 @@
8282
#ifndef ECHOKE
8383
# define ECHOKE 0
8484
#endif
85-
#ifndef PENDIN
86-
# define PENDIN 0
87-
#endif
8885

89-
static struct termios oterm;
90-
static int changed;
86+
/* Termios flags to copy between terminals. */
87+
#define INPUT_FLAGS (IGNPAR|PARMRK|INPCK|ISTRIP|INLCR|IGNCR|ICRNL|IUCLC|IXON|IXANY|IXOFF|IMAXBEL|IUTF8)
88+
#define OUTPUT_FLAGS (OPOST|OLCUC|ONLCR|OCRNL|ONOCR|ONLRET)
89+
#define CONTROL_FLAGS (CS7|CS8|PARENB|PARODD)
90+
#define LOCAL_FLAGS (ISIG|ICANON|XCASE|ECHO|ECHOE|ECHOK|ECHONL|NOFLSH|TOSTOP|IEXTEN|ECHOCTL|ECHOKE)
91+
92+
static struct termios orig_term;
93+
static struct termios cur_term;
94+
static bool changed;
9195

9296
/* tgetpass() needs to know the erase and kill chars for cbreak mode. */
9397
sudo_dso_public int sudo_term_eof;
@@ -129,7 +133,7 @@ tcsetattr_nobg(int fd, int flags, struct termios *tp)
129133
sigaction(SIGTTOU, &sa, &osa);
130134
do {
131135
rc = tcsetattr(fd, flags, tp);
132-
} while (rc != 0 && errno == EINTR && !got_sigttou);
136+
} while (rc == -1 && errno == EINTR && !got_sigttou);
133137
sigaction(SIGTTOU, &osa, NULL);
134138

135139
debug_return_int(rc);
@@ -142,15 +146,75 @@ tcsetattr_nobg(int fd, int flags, struct termios *tp)
142146
bool
143147
sudo_term_restore_v1(int fd, bool flush)
144148
{
149+
const int flags = flush ? (TCSASOFT|TCSAFLUSH) : (TCSASOFT|TCSADRAIN);
150+
struct termios term = { 0 };
151+
bool ret = false;
145152
debug_decl(sudo_term_restore, SUDO_DEBUG_UTIL);
146153

147-
if (changed) {
148-
const int flags = flush ? (TCSASOFT|TCSAFLUSH) : (TCSASOFT|TCSADRAIN);
149-
if (tcsetattr_nobg(fd, flags, &oterm) != 0)
150-
debug_return_bool(false);
151-
changed = 0;
154+
if (!changed)
155+
debug_return_bool(true);
156+
157+
sudo_lock_file(fd, SUDO_LOCK);
158+
159+
/* Avoid changing term settings if changed out from under us. */
160+
if (tcgetattr(fd, &term) == -1) {
161+
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
162+
"%s: tcgetattr(%d)", __func__, fd);
163+
goto unlock;
152164
}
153-
debug_return_bool(true);
165+
if ((term.c_iflag & INPUT_FLAGS) != (cur_term.c_iflag & INPUT_FLAGS)) {
166+
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: not restoring terminal, "
167+
"c_iflag changed; 0x%x, expected 0x%x", __func__,
168+
(unsigned int)term.c_iflag, (unsigned int)cur_term.c_iflag);
169+
/* Not an error. */
170+
ret = true;
171+
goto unlock;
172+
}
173+
if ((term.c_oflag & OUTPUT_FLAGS) != (cur_term.c_oflag & OUTPUT_FLAGS)) {
174+
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: not restoring terminal, "
175+
"c_oflag changed; 0x%x, expected 0x%x", __func__,
176+
(unsigned int)term.c_oflag, (unsigned int)cur_term.c_oflag);
177+
/* Not an error. */
178+
ret = true;
179+
goto unlock;
180+
}
181+
if ((term.c_cflag & CONTROL_FLAGS) != (cur_term.c_cflag & CONTROL_FLAGS)) {
182+
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: not restoring terminal, "
183+
"c_cflag changed; 0x%x, expected 0x%x", __func__,
184+
(unsigned int)term.c_cflag, (unsigned int)cur_term.c_cflag);
185+
/* Not an error. */
186+
ret = true;
187+
goto unlock;
188+
}
189+
if ((term.c_lflag & LOCAL_FLAGS) != (cur_term.c_lflag & LOCAL_FLAGS)) {
190+
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: not restoring terminal, "
191+
"c_lflag changed; 0x%x, expected 0x%x", __func__,
192+
(unsigned int)term.c_lflag, (unsigned int)cur_term.c_lflag);
193+
/* Not an error. */
194+
ret = true;
195+
goto unlock;
196+
}
197+
if (memcmp(term.c_cc, cur_term.c_cc, sizeof(term.c_cc)) != 0) {
198+
sudo_debug_printf(SUDO_DEBUG_INFO,
199+
"%s: not restoring terminal, c_cc[] changed", __func__);
200+
/* Not an error. */
201+
ret = true;
202+
goto unlock;
203+
}
204+
205+
if (tcsetattr_nobg(fd, flags, &orig_term) == -1) {
206+
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
207+
"%s: tcsetattr(%d)", __func__, fd);
208+
goto unlock;
209+
}
210+
cur_term = orig_term;
211+
changed = false;
212+
ret = true;
213+
214+
unlock:
215+
sudo_lock_file(fd, SUDO_UNLOCK);
216+
217+
debug_return_bool(ret);
154218
}
155219

156220
/*
@@ -160,21 +224,75 @@ sudo_term_restore_v1(int fd, bool flush)
160224
bool
161225
sudo_term_noecho_v1(int fd)
162226
{
163-
struct termios term;
227+
struct termios term = { 0 };
228+
bool ret = false;
164229
debug_decl(sudo_term_noecho, SUDO_DEBUG_UTIL);
165230

166-
if (!changed && tcgetattr(fd, &oterm) != 0)
167-
debug_return_bool(false);
168-
(void) memcpy(&term, &oterm, sizeof(term));
231+
sudo_lock_file(fd, SUDO_LOCK);
232+
if (!changed && tcgetattr(fd, &orig_term) == -1) {
233+
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
234+
"%s: tcgetattr(%d)", __func__, fd);
235+
goto unlock;
236+
}
237+
238+
term = orig_term;
169239
CLR(term.c_lflag, ECHO|ECHONL);
170240
#ifdef VSTATUS
171241
term.c_cc[VSTATUS] = _POSIX_VDISABLE;
172242
#endif
173-
if (tcsetattr_nobg(fd, TCSASOFT|TCSADRAIN, &term) == 0) {
174-
changed = 1;
175-
debug_return_bool(true);
243+
if (tcsetattr_nobg(fd, TCSASOFT|TCSADRAIN, &term) == -1) {
244+
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
245+
"%s: tcsetattr(%d)", __func__, fd);
246+
goto unlock;
176247
}
177-
debug_return_bool(false);
248+
cur_term = term;
249+
changed = true;
250+
ret = true;
251+
252+
unlock:
253+
sudo_lock_file(fd, SUDO_UNLOCK);
254+
debug_return_bool(ret);
255+
}
256+
257+
/*
258+
* Returns true if term is in raw mode, else false.
259+
*/
260+
static bool
261+
sudo_term_is_raw_int(struct termios *term)
262+
{
263+
debug_decl(sudo_term_is_raw_int, SUDO_DEBUG_UTIL);
264+
265+
if (term->c_cc[VMIN] != 1 || term->c_cc[VTIME] != 0)
266+
debug_return_bool(false);
267+
268+
if (ISSET(term->c_oflag, OPOST))
269+
debug_return_bool(false);
270+
271+
if (ISSET(term->c_oflag, ECHO|ECHONL|ICANON))
272+
debug_return_bool(false);
273+
274+
debug_return_bool(true);
275+
}
276+
277+
/*
278+
* Returns true if fd refers to a tty in raw mode, else false.
279+
*/
280+
bool
281+
sudo_term_is_raw_v1(int fd)
282+
{
283+
struct termios term = { 0 };
284+
debug_decl(sudo_term_is_raw, SUDO_DEBUG_UTIL);
285+
286+
sudo_lock_file(fd, SUDO_LOCK);
287+
if (tcgetattr(fd, &term) == -1) {
288+
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
289+
"%s: tcgetattr(%d)", __func__, fd);
290+
sudo_lock_file(fd, SUDO_UNLOCK);
291+
debug_return_bool(false);
292+
}
293+
sudo_lock_file(fd, SUDO_UNLOCK);
294+
295+
debug_return_bool(sudo_term_is_raw_int(&term));
178296
}
179297

180298
/*
@@ -184,28 +302,48 @@ sudo_term_noecho_v1(int fd)
184302
bool
185303
sudo_term_raw_v1(int fd, unsigned int flags)
186304
{
187-
struct termios term;
305+
struct termios term = { 0 };
306+
bool ret = false;
188307
tcflag_t oflag;
189308
debug_decl(sudo_term_raw, SUDO_DEBUG_UTIL);
190309

191-
if (!changed && tcgetattr(fd, &oterm) != 0)
192-
debug_return_bool(false);
193-
(void) memcpy(&term, &oterm, sizeof(term));
310+
sudo_lock_file(fd, SUDO_LOCK);
311+
if (!changed && tcgetattr(fd, &orig_term) == -1) {
312+
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
313+
"%s: tcgetattr(%d)", __func__, fd);
314+
goto unlock;
315+
}
316+
317+
if (sudo_term_is_raw_int(&term)) {
318+
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: fd %d already in raw mode",
319+
__func__, fd);
320+
ret = true;
321+
goto unlock;
322+
}
323+
194324
/*
195325
* Set terminal to raw mode but optionally enable terminal signals
196326
* and/or preserve output flags.
197327
*/
328+
term = orig_term;
198329
oflag = term.c_oflag;
199330
cfmakeraw(&term);
200331
if (ISSET(flags, SUDO_TERM_ISIG))
201332
SET(term.c_lflag, ISIG);
202333
if (ISSET(flags, SUDO_TERM_OFLAG))
203334
term.c_oflag = oflag;
204-
if (tcsetattr_nobg(fd, TCSASOFT|TCSADRAIN, &term) == 0) {
205-
changed = 1;
206-
debug_return_bool(true);
335+
if (tcsetattr_nobg(fd, TCSASOFT|TCSADRAIN, &term) == -1) {
336+
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
337+
"%s: tcsetattr(%d)", __func__, fd);
338+
goto unlock;
207339
}
208-
debug_return_bool(false);
340+
cur_term = term;
341+
changed = true;
342+
ret = true;
343+
344+
unlock:
345+
sudo_lock_file(fd, SUDO_UNLOCK);
346+
debug_return_bool(ret);
209347
}
210348

211349
/*
@@ -215,13 +353,19 @@ sudo_term_raw_v1(int fd, unsigned int flags)
215353
bool
216354
sudo_term_cbreak_v1(int fd)
217355
{
218-
struct termios term;
356+
struct termios term = { 0 };
357+
bool ret = false;
219358
debug_decl(sudo_term_cbreak, SUDO_DEBUG_UTIL);
220359

221-
if (!changed && tcgetattr(fd, &oterm) != 0)
222-
debug_return_bool(false);
223-
(void) memcpy(&term, &oterm, sizeof(term));
360+
sudo_lock_file(fd, SUDO_LOCK);
361+
if (!changed && tcgetattr(fd, &orig_term) == -1) {
362+
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
363+
"%s: tcgetattr(%d)", __func__, fd);
364+
goto unlock;
365+
}
366+
224367
/* Set terminal to half-cooked mode */
368+
term = orig_term;
225369
term.c_cc[VMIN] = 1;
226370
term.c_cc[VTIME] = 0;
227371
/* cppcheck-suppress redundantAssignment */
@@ -231,22 +375,23 @@ sudo_term_cbreak_v1(int fd)
231375
#ifdef VSTATUS
232376
term.c_cc[VSTATUS] = _POSIX_VDISABLE;
233377
#endif
234-
if (tcsetattr_nobg(fd, TCSASOFT|TCSADRAIN, &term) == 0) {
235-
sudo_term_eof = term.c_cc[VEOF];
236-
sudo_term_erase = term.c_cc[VERASE];
237-
sudo_term_kill = term.c_cc[VKILL];
238-
changed = 1;
239-
debug_return_bool(true);
378+
if (tcsetattr_nobg(fd, TCSASOFT|TCSADRAIN, &term) == -1) {
379+
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
380+
"%s: tcsetattr(%d)", __func__, fd);
381+
goto unlock;
240382
}
241-
debug_return_bool(false);
383+
sudo_term_eof = term.c_cc[VEOF];
384+
sudo_term_erase = term.c_cc[VERASE];
385+
sudo_term_kill = term.c_cc[VKILL];
386+
cur_term = term;
387+
changed = true;
388+
ret = true;
389+
390+
unlock:
391+
sudo_lock_file(fd, SUDO_UNLOCK);
392+
debug_return_bool(ret);
242393
}
243394

244-
/* Termios flags to copy between terminals. */
245-
#define INPUT_FLAGS (IGNPAR|PARMRK|INPCK|ISTRIP|INLCR|IGNCR|ICRNL|IUCLC|IXON|IXANY|IXOFF|IMAXBEL|IUTF8)
246-
#define OUTPUT_FLAGS (OPOST|OLCUC|ONLCR|OCRNL|ONOCR|ONLRET)
247-
#define CONTROL_FLAGS (CS7|CS8|PARENB|PARODD)
248-
#define LOCAL_FLAGS (ISIG|ICANON|XCASE|ECHO|ECHOE|ECHOK|ECHONL|NOFLSH|TOSTOP|IEXTEN|ECHOCTL|ECHOKE|PENDIN)
249-
250395
/*
251396
* Copy terminal settings from one descriptor to another.
252397
* We cannot simply copy the struct termios as src and dst may be
@@ -260,10 +405,16 @@ sudo_term_copy_v1(int src, int dst)
260405
struct winsize wsize;
261406
speed_t speed;
262407
unsigned int i;
408+
bool ret = false;
263409
debug_decl(sudo_term_copy, SUDO_DEBUG_UTIL);
264410

265-
if (tcgetattr(src, &tt_src) != 0 || tcgetattr(dst, &tt_dst) != 0)
266-
debug_return_bool(false);
411+
sudo_lock_file(src, SUDO_LOCK);
412+
sudo_lock_file(dst, SUDO_LOCK);
413+
if (tcgetattr(src, &tt_src) == -1 || tcgetattr(dst, &tt_dst) == -1) {
414+
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
415+
"%s: tcgetattr", __func__);
416+
goto unlock;
417+
}
267418

268419
/* Clear select input, output, control and local flags. */
269420
CLR(tt_dst.c_iflag, INPUT_FLAGS);
@@ -288,35 +439,18 @@ sudo_term_copy_v1(int src, int dst)
288439
speed = cfgetispeed(&tt_src);
289440
cfsetispeed(&tt_dst, speed);
290441

291-
if (tcsetattr_nobg(dst, TCSASOFT|TCSAFLUSH, &tt_dst) == -1)
292-
debug_return_bool(false);
442+
if (tcsetattr_nobg(dst, TCSASOFT|TCSAFLUSH, &tt_dst) == -1) {
443+
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
444+
"%s: tcsetattr(%d)", __func__, dst);
445+
goto unlock;
446+
}
447+
ret = true;
293448

294449
if (ioctl(src, TIOCGWINSZ, &wsize) == 0)
295450
(void)ioctl(dst, IOCTL_REQ_CAST TIOCSWINSZ, &wsize);
296451

297-
debug_return_bool(true);
298-
}
299-
300-
/*
301-
* Returns true if fd refers to a tty in raw mode, else false.
302-
*/
303-
bool
304-
sudo_term_is_raw_v1(int fd)
305-
{
306-
struct termios term;
307-
debug_decl(sudo_term_is_raw, SUDO_DEBUG_UTIL);
308-
309-
if (tcgetattr(fd, &term) != 0)
310-
debug_return_bool(false);
311-
312-
if (term.c_cc[VMIN] != 1 || term.c_cc[VTIME] != 0)
313-
debug_return_bool(false);
314-
315-
if (ISSET(term.c_oflag, OPOST))
316-
debug_return_bool(false);
317-
318-
if (ISSET(term.c_oflag, ECHO|ECHONL|ICANON))
319-
debug_return_bool(false);
320-
321-
debug_return_bool(true);
452+
unlock:
453+
sudo_lock_file(dst, SUDO_UNLOCK);
454+
sudo_lock_file(src, SUDO_UNLOCK);
455+
debug_return_bool(ret);
322456
}

0 commit comments

Comments
 (0)