[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: [PATCH] kpty weirdness (was: Re: kdelibs/kdecore)
From: Oswald Buddenhagen <ossi () kde ! org>
Date: 2004-01-13 18:37:55
Message-ID: 20040113183755.GA21611 () ugly ! local
[Download RAW message or body]
On Mon, Jan 12, 2004 at 07:38:01PM +0200, Andy Fawcett wrote:
> On Monday 12 January 2004 18:23, Oswald Buddenhagen wrote:
> > On Mon, Jan 12, 2004 at 06:17:02PM +0200, Andy Fawcett wrote:
> > > On Saturday 10 January 2004 20:35, Oswald Buddenhagen wrote:
> > > > CVS commit by ossi:
> > > >
> > > > trying to fix #67464 by not trying to chownpty if everything is
> > > > ok already. some cleanup as well. ok'd by waldo.
> > > >
> > > >
> > > > M +69 -75 kpty.cpp 1.15
> > >
> > > This change prevents konsole from starting on FreeBSD.
> >
> > this stuff is weird ...
>
> Yes.
>
and here is yet another attempt at fixing this stuff. the patch changes
quite a lot of details, so i decided to adhere to the commit policy and
post it. :) please review not only the patch, but also the explainations
below.
there are two types of pseudo ttys: unix98 and bsd.
unix98: there is one pty master multiplexer device. every time you open
it, you get a new pty pair; a device node for the pty slave appears in
the /dev/pts/ virtual file system. when you close the master handle, the
slave pty node disappears again.
bsd: /dev/ is statically populated by a helluva lot of ptyp* and ttyp*
device nodes, every such pair representing a pty. a pty is available, if
one can open the master (every master can be opened only once at a time)
and the permissions of the slave being rw-rw-rw-, i.e. available to
everybody. the latter means, that it is mandatory to reset the
permissions when shutting down the pty.
now to the patch. #) denotes reverts of (much) earlier patches of mine,
so those parts can't be that wrong. :)
- #) now i think grantpt is guaranteed to work only with unix98 ptys, so
use it only in that branch and use the hand-crafted code for bsd ptys.
- #) in the bsd pty branch, remove the geteuid() == 0 before the
access() call. i had mistaken this as a permission check - in fact, it
is part of the availability check.
- entirely remove the openpty() part. it is redundand: it is only a neat
encapsulation of all the code below, so one of the two can go away. as
for some (to me) absolutely incomprehensible reasons the openpty()
based variant causes total failure on tap's freebsd setup, the choice
is simple. :)
- the bsd pty branch is guaranteed to be unable to chown the pty if
running as non-root (hence the check); the unix98 pty branch could
fail if the grantpt() call offered by the system does not have an own
variant of kgrantpty behind the scenes. therefore after both branches
a check is done if the pty slave node ownership and permissions are
ok, and if not, we call kgrantpty.
- pty slave node group ownership: the preferred gid is "tty". if that
fails, try "wheel". if that fails, too, use the user's gid.
when shutting down, we check if the pty's gid is the current user's
gid - if so, we assume that no appropriate group was found at setup
time and therefore reset it to root. otherwise we leave it as-is.
this is now consistent for both the cases with and without kgrantpty,
and is not dependant on the os.
- don't bother to reset pty slave ownership of unix98 ptys, as they
vanish after closing the master anyway.
- BSD only: revoke() moved to a place where we are sure to own the pty
slave, and therefore have permission to revoke() it. therefore it
needn't to be in kgrantpty.
- in kgrantpty, first try ptsname instead of the weird pty -> tty
transformation that works only with bsd ptys. iow, make kgrantpty work
with unix98 ptys in the first place (just in case grantpt() fails as
non-root on some systems).
ok, that's it except for some minor cleanups.
maybe somebody can verify whether this fixes #22479.
greetings
--
Hi! I'm a .signature virus! Copy me into your ~/.signature, please!
--
Chaos, panic, and disorder - my work here is done.
["kpty.diff" (text/plain)]
Index: kgrantpty.c
===================================================================
RCS file: /home/kde/kdelibs/kdecore/kgrantpty.c,v
retrieving revision 1.13
diff -U2 -r1.13 kgrantpty.c
--- kgrantpty.c 20 Nov 2003 15:53:10 -0000 1.13
+++ kgrantpty.c 13 Jan 2004 17:36:44 -0000
@@ -25,6 +25,7 @@
*/
+#include <config.h>
+
#include <sys/types.h>
-#include <sys/param.h> /* for BSD */
#include <errno.h>
#include <grp.h>
@@ -46,5 +47,4 @@
int main (int argc, char *argv[])
{
- char* pty;
struct stat st;
struct group* p;
@@ -72,84 +72,68 @@
}
- /* setup parameters for the operation ***********************************/
-
- if (!strcmp(argv[1],"--grant"))
- {
- uid = getuid(); /* current user id */
- mod = S_IRUSR | S_IWUSR | S_IWGRP;
- }
- else
- {
- uid = 0; /* root */
- mod = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
- }
- /* Get the group ID of the special `tty' group. */
- p = getgrnam(TTY_GROUP); /* posix */
- gid = p ? p->gr_gid : getgid (); /* posix */
fd = atoi(argv[2]);
- /* get slave pty name from master pty file handle in PTY_FILENO *********/
-
- /* Check that fd is a valid master pseudo terminal. */
- pty = ttyname(fd); /* posix */
-
-#if defined(BSD_PTY_HACK)
- if (pty == NULL)
+ /* get slave pty name from master pty file handle *********/
+#ifdef HAVE_PTSNAME
+ tty = ptsname(fd);
+ if (!tty)
+#endif
{
- /*
- Hack to make kgrantpty work on some versions of FreeBSD (and possibly
- other systems): ttyname(3) does not work with a file descriptor opened
- on a /dev/pty?? device.
-
- Instead, this code looks through all the devices in /dev for a device
- which has the same inode as our PTY_FILENO descriptor... if found, we
- have the name for our pty.
- */
-
- struct dirent *dirp;
- DIR *dp;
- struct stat dsb;
-
- if (uid == 0) {
- p = getgrnam("wheel");
- gid = p ? p->gr_gid : getgid();
- }
+ /* Check that fd is a valid master pseudo terminal. */
+ char *pty = ttyname(fd);
- if (fstat(fd, &dsb) != -1) {
- if ((dp = opendir(_PATH_DEV)) != NULL) {
- while ((dirp = readdir(dp))) {
- if (dirp->d_fileno != dsb.st_ino)
- continue;
- pty = malloc(sizeof(_PATH_DEV) + strlen(dirp->d_name));
- if (pty) {
- strcpy(pty, _PATH_DEV);
- strcat(pty, dirp->d_name);
+#ifdef BSD_PTY_HACK
+ if (pty == NULL)
+ {
+ /*
+ Hack to make kgrantpty work on some versions of FreeBSD (and possibly
+ other systems): ttyname(3) does not work with a file descriptor opened
+ on a /dev/pty?? device.
+
+ Instead, this code looks through all the devices in /dev for a device
+ which has the same inode as our PTY_FILENO descriptor... if found, we
+ have the name for our pty.
+ */
+
+ struct dirent *dirp;
+ DIR *dp;
+ struct stat dsb;
+
+ if (fstat(fd, &dsb) != -1) {
+ if ((dp = opendir(_PATH_DEV)) != NULL) {
+ while ((dirp = readdir(dp))) {
+ if (dirp->d_fileno != dsb.st_ino)
+ continue;
+ pty = malloc(sizeof(_PATH_DEV) + strlen(dirp->d_name));
+ if (pty) {
+ strcpy(pty, _PATH_DEV);
+ strcat(pty, dirp->d_name);
+ }
+ break;
}
- break;
- }
- (void) closedir(dp);
+ (void) closedir(dp);
+ }
}
}
- }
#endif
- if (pty == NULL)
- {
- fprintf(stderr,"%s: cannot determine pty name.\n",argv[0]);
- return 1; /* FAIL */
- }
- close(fd);
+ if (pty == NULL)
+ {
+ fprintf(stderr,"%s: cannot determine pty name.\n",argv[0]);
+ return 1; /* FAIL */
+ }
- /* matches /dev/pty?? */
- if (strlen(pty) < 8 || strncmp(pty,"/dev/pty",8))
- {
- fprintf(stderr,"%s: determined a strange pty name `%s'.\n",argv[0],pty);
- return 1; /* FAIL */
- }
+ /* matches /dev/pty?? */
+ if (memcmp(pty,"/dev/pty",8))
+ {
+ fprintf(stderr,"%s: determined a strange pty name `%s'.\n",argv[0],pty);
+ return 1; /* FAIL */
+ }
- tty = malloc(strlen(pty) + 1);
- strcpy(tty,"/dev/tty");
- strcat(tty,pty+8);
+ tty = malloc(strlen(pty) + 1);
+ strcpy(tty,"/dev/tty");
+ strcat(tty,pty+8);
+ }
/* Check that the returned slave pseudo terminal is a character device. */
@@ -160,4 +144,22 @@
}
+ /* setup parameters for the operation ***********************************/
+
+ if (!strcmp(argv[1],"--grant"))
+ {
+ uid = getuid();
+ p = getgrnam(TTY_GROUP);
+ if (!p)
+ p = getgrnam("wheel");
+ gid = p ? p->gr_gid : getgid ();
+ mod = S_IRUSR | S_IWUSR | S_IWGRP;
+ }
+ else
+ {
+ uid = 0;
+ gid = st.st_gid == getgid () ? 0 : -1;
+ mod = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
+ }
+
/* Perform the actual chown/chmod ************************************/
@@ -174,12 +176,4 @@
}
-#ifdef BSD
- if (revoke(tty) < 0)
- {
- fprintf(stderr,"%s: cannot revoke %s: %s\n",argv[0],tty,strerror(errno));
- return 1; /* FAIL */
- }
-#endif
-
return 0; /* OK */
}
Index: kpty.cpp
===================================================================
RCS file: /home/kde/kdelibs/kdecore/kpty.cpp,v
retrieving revision 1.18
diff -U2 -r1.18 kpty.cpp
--- kpty.cpp 13 Jan 2004 01:02:29 -0000 1.18
+++ kpty.cpp 13 Jan 2004 17:36:44 -0000
@@ -159,5 +159,5 @@
struct KPtyPrivate {
KPtyPrivate() :
- xonXoff(false), needGrantPty(false),
+ xonXoff(false),
masterFd(-1), slaveFd(-1)
{
@@ -168,5 +168,4 @@
bool xonXoff : 1;
- bool needGrantPty : 1;
int masterFd;
int slaveFd;
@@ -204,13 +203,6 @@
// We try, as we know them, one by one.
-#if defined(HAVE_OPENPTY)
- if (openpty(&d->masterFd, &d->slaveFd, NULL, NULL, NULL) == 0)
- {
- d->ttyName = ttyname(d->slaveFd);
- goto gotpty2;
- }
-#endif
-#if defined(HAVE_PTSNAME)
+#if defined(HAVE_PTSNAME) && defined(HAVE_GRANTPT)
#ifdef _AIX
d->masterFd = ::open("/dev/ptc",O_RDWR);
@@ -222,6 +214,7 @@
char *ptsn = ptsname(d->masterFd);
if (ptsn) {
+ grantpt(d->masterFd);
d->ttyName = ptsn;
- goto gotpty1;
+ goto gotpty;
} else {
::close(d->masterFd);
@@ -254,6 +247,18 @@
}
#endif /* sun */
- if (geteuid() == 0 || access(d->ttyName.data(),R_OK|W_OK) == 0)
- goto gotpty1; // Success !!
+ if (!access(d->ttyName.data(),R_OK|W_OK)) // checks availability based on permission bits
+ {
+ if (!geteuid())
+ {
+ struct group* p = getgrnam(TTY_GROUP);
+ if (!p)
+ p = getgrnam("wheel");
+ gid_t gid = p ? p->gr_gid : getgid ();
+
+ chown(d->ttyName.data(), getuid(), gid);
+ chmod(d->ttyName.data(), S_IRUSR|S_IWUSR|S_IWGRP);
+ }
+ goto gotpty;
+ }
::close(d->masterFd);
d->masterFd = -1;
@@ -265,28 +270,11 @@
return false;
- gotpty1:
-#if defined(HAVE_GRANTPT)
- grantpt(d->masterFd);
-#else
- {
- /* Get the group ID of the special `tty' group. */
- struct group* p = getgrnam(TTY_GROUP); /* posix */
- gid_t gid = p ? p->gr_gid : getgid (); /* posix */
-
- chown(d->ttyName.data(), getuid(), gid);
- chmod(d->ttyName.data(), S_IRUSR|S_IWUSR|S_IWGRP);
- }
-#endif
- gotpty2:
+ gotpty:
struct stat st;
if (stat(d->ttyName.data(), &st))
return false; // this just cannot happen ... *cough*
- d->needGrantPty = st.st_uid != getuid() || (st.st_mode & (S_IRGRP|S_IXGRP|S_IROTH|S_IWOTH|S_IXOTH));
-
-#ifdef BSD
- revoke(d->ttyName.data());
-#endif
-
- if (!chownpty(true))
+ if (((st.st_uid != getuid()) ||
+ (st.st_mode & (S_IRGRP|S_IXGRP|S_IROTH|S_IWOTH|S_IXOTH))) &&
+ !chownpty(true))
{
kdWarning(175)
@@ -295,17 +283,19 @@
}
+#ifdef BSD
+ revoke(d->ttyName.data());
+#endif
+
#ifdef HAVE_UNLOCKPT
unlockpt(d->masterFd);
#endif
- if (d->slaveFd < 0) { // slave pty not yet opened?
- d->slaveFd = ::open(d->ttyName, O_RDWR);
- if (d->slaveFd < 0)
- {
- kdWarning(175) << "Can't open slave pseudo teletype" << endl;
- ::close(d->masterFd);
- d->masterFd = -1;
- return false;
- }
+ d->slaveFd = ::open(d->ttyName.data(), O_RDWR);
+ if (d->slaveFd < 0)
+ {
+ kdWarning(175) << "Can't open slave pseudo teletype" << endl;
+ ::close(d->masterFd);
+ d->masterFd = -1;
+ return false;
}
@@ -348,7 +338,17 @@
if (d->masterFd < 0)
return;
- chown(d->ttyName.data(), 0, 0);
- chmod(d->ttyName.data(), S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
- chownpty(false);
+ // don't bother resetting unix98 pty, it will go away after closing master anyway.
+ if (memcmp(d->ttyName.data(), "/dev/pts/", 9)) {
+ if (!geteuid()) {
+ struct stat st;
+ if (!stat(d->ttyName.data(), &st)) {
+ chown(d->ttyName.data(), 0, st.st_gid == getgid() ? 0 : -1);
+ chmod(d->ttyName.data(), S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
+ }
+ } else {
+ fcntl(d->masterFd, F_SETFD, 0);
+ chownpty(false);
+ }
+ }
::close(d->slaveFd);
::close(d->masterFd);
@@ -492,8 +492,4 @@
bool KPty::chownpty(bool grant)
{
- if (!d->needGrantPty)
- return true;
-
- fcntl(d->masterFd, F_SETFD, 0);
KProcess proc;
proc << locate("exe", BASE_CHOWN) << (grant?"--grant":"--revoke") << QString::number(d->masterFd);
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic