On Mon, Jan 25, 2010 at 11:12:10PM +0100, Yann Droneaud wrote:
> When launched, check fd 0 (stdin) using isatty(), bail out if not.
> When the tty device is re-opened, check it using isatty(), leave if not.
> This will help to prevent some possible races.
I have applied the patch below. Thanks for the suggestion.
Karel
>From 8bee984a93cdf62b3cc84bb9fbfe023b6f554818 Mon Sep 17 00:00:00 2001
From: Karel Zak <kzak@xxxxxxxxxx>
Date: Thu, 28 Jan 2010 16:25:52 +0100
Subject: [PATCH] login: check that after tty reopen we still work with a terminal
* the login code assumes that stdin is a terminal, it's better to
check (by isatty()) that after tty reopen we still have a terminal
* this patch also removes very old obscure fallback for situations where
ttyname() returns nothing (then ttyn = "/dev/tty??"). I guess that the
fake string was originally for utmp records or so. Currently (in last 10
years...) code requires that the tty name is a real open-able file.
It means the fake tty name is completely useless.
Reported-by: Yann Droneaud <yann@xxxxxxxxxxx>
Signed-off-by: Karel Zak <kzak@xxxxxxxxxx>
---
login-utils/login.c | 22 +++++++++++++---------
1 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/login-utils/login.c b/login-utils/login.c
index fdc8078..1550388 100644
--- a/login-utils/login.c
+++ b/login-utils/login.c
@@ -200,6 +200,13 @@ opentty(const char * tty) {
exit(1);
}
+ if (!isatty(fd)) {
+ close(fd);
+ syslog(LOG_ERR, _("FATAL: %s is not a terminal"), tty);
+ sleep(1);
+ exit(1);
+ }
+
flags = fcntl(fd, F_GETFL);
flags &= ~O_NONBLOCK;
fcntl(fd, F_SETFL, flags);
@@ -222,7 +229,9 @@ static void
check_ttyname(char *ttyn) {
struct stat statbuf;
- if (lstat(ttyn, &statbuf)
+ if (ttyn == NULL
+ || *ttyn == '\0'
+ || lstat(ttyn, &statbuf)
|| !S_ISCHR(statbuf.st_mode)
|| (statbuf.st_nlink > 1 && strncmp(ttyn, "/dev/", 5))
|| (access(ttyn, R_OK | W_OK) != 0)) {
@@ -378,7 +387,7 @@ main(int argc, char **argv)
int ask, fflag, hflag, pflag, cnt, errsv;
int quietlog, passwd_req;
char *domain, *ttyn;
- char tbuf[MAXPATHLEN + 2], tname[sizeof(_PATH_DEV_TTY) + 10];
+ char tbuf[MAXPATHLEN + 2];
char *termenv;
char *childArgv[10];
char *buff;
@@ -495,14 +504,9 @@ main(int argc, char **argv)
for (cnt = getdtablesize(); cnt > 2; cnt--)
close(cnt);
+ /* note that libc checks that the file descriptor is a terminal, so we don't
+ * have to call isatty() here */
ttyn = ttyname(0);
-
- if (ttyn == NULL || *ttyn == '\0') {
- /* no snprintf required - see definition of tname */
- snprintf(tname, sizeof(tname), "%s??", _PATH_DEV_TTY);
- ttyn = tname;
- }
-
check_ttyname(ttyn);
if (strncmp(ttyn, "/dev/", 5) == 0)
--
1.6.5.2
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
