* ext/pty/pty.c (getDevice): add nomesg argument.

  (get_device_once): add nomesg argument.  chmod slave tty to 0600
  if nomesg.  more error tests.
  (no_mesg): new function.
  (pty_open): make slave tty's mode 0600.



git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@20795 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

3 files changed, 90 insertions, 80 deletions

diff --git a/ChangeLog b/ChangeLog
index b58239430c..fe2c0688bd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+Tue Dec 16 20:34:44 2008  Tanaka Akira  <akr@fsij.org>
+
+	* ext/pty/pty.c (getDevice): add nomesg argument.
+	  (get_device_once): add nomesg argument.  chmod slave tty to 0600
+	  if nomesg.  more error tests.
+	  (no_mesg): new function.
+	  (pty_open): make slave tty's mode 0600.
+
 Tue Dec 16 20:24:20 2008  Tadayoshi Funaba  <tadf@dotrb.org>
 
 	* lib/date/format.rb (_parse): m17n compliant.
diff --git a/ext/pty/pty.c b/ext/pty/pty.c
index a8a02d8769..2798e88cc0 100644
--- a/ext/pty/pty.c
+++ b/ext/pty/pty.c
@@ -130,7 +130,7 @@ struct pty_info {
     rb_pid_t child_pid;
 };
 
-static void getDevice(int*, int*, char [DEVICELEN]);
+static void getDevice(int*, int*, char [DEVICELEN], int);
 
 struct exec_info {
     int argc;
@@ -248,7 +248,7 @@ establishShell(int argc, VALUE *argv, struct pty_info *info,
 	argv = &v;
     }
 
-    getDevice(&master, &slave, SlaveName);
+    getDevice(&master, &slave, SlaveName, 0);
 
     carg.master = master;
     carg.slave = slave;
@@ -271,10 +271,19 @@ establishShell(int argc, VALUE *argv, struct pty_info *info,
 }
 
 static int
-get_device_once(int *master, int *slave, char SlaveName[DEVICELEN], int fail)
+no_mesg(char *slavedevice, int nomesg)
+{
+    if (nomesg)
+        return chmod(slavedevice, 0600);
+    else
+        return 0;
+}
+
+static int
+get_device_once(int *master, int *slave, char SlaveName[DEVICELEN], int nomesg, int fail)
 {
 #if defined(HAVE_POSIX_OPENPT)
-    int masterfd,slavefd;
+    int masterfd = -1, slavefd = -1;
     char *slavedevice;
     struct sigaction dfl, old;
 
@@ -282,33 +291,34 @@ get_device_once(int *master, int *slave, char SlaveName[DEVICELEN], int fail)
     dfl.sa_flags = 0;
     sigemptyset(&dfl.sa_mask);
 
-    if((masterfd = posix_openpt(O_RDWR|O_NOCTTY)) != -1) {
-	sigaction(SIGCHLD, &dfl, &old);
-	if(grantpt(masterfd) != -1) {
-	    sigaction(SIGCHLD, &old, NULL);
-	    if(unlockpt(masterfd) != -1) {
-		if((slavedevice = ptsname(masterfd)) != NULL) {
-		    if((slavefd = open(slavedevice, O_RDWR|O_NOCTTY, 0)) != -1) {
-#if defined I_PUSH && !defined linux
-			if(ioctl(slavefd, I_PUSH, "ptem") != -1) {
-			    if(ioctl(slavefd, I_PUSH, "ldterm") != -1) {
-				ioctl(slavefd, I_PUSH, "ttcompat");
-#endif
-				*master = masterfd;
-				*slave = slavefd;
-				strlcpy(SlaveName, slavedevice, DEVICELEN);
-				return 0;
+    if ((masterfd = posix_openpt(O_RDWR|O_NOCTTY)) == -1) goto error;
+    if (sigaction(SIGCHLD, &dfl, &old) == -1) goto error;
+    if (grantpt(masterfd) == -1) goto grantpt_error;
+    if (sigaction(SIGCHLD, &old, NULL) == -1) goto error;
+    if (unlockpt(masterfd) == -1) goto error;
+    if ((slavedevice = ptsname(masterfd)) == NULL) goto error;
+    if (no_mesg(slavedevice, nomesg) == -1) goto error;
+    if ((slavefd = open(slavedevice, O_RDWR|O_NOCTTY, 0)) == -1) goto error;
+
 #if defined I_PUSH && !defined linux
-			    }
-			}
+    if (ioctl(slavefd, I_PUSH, "ptem") == -1) goto error;
+    if (ioctl(slavefd, I_PUSH, "ldterm") == -1) goto error;
+    if (ioctl(slavefd, I_PUSH, "ttcompat") == -1) goto error;
 #endif
-		    }
-		}
-	    }
-	}
-	close(masterfd);
+
+    *master = masterfd;
+    *slave = slavefd;
+    strlcpy(SlaveName, slavedevice, DEVICELEN);
+    return 0;
+
+  grantpt_error:
+    sigaction(SIGCHLD, &old, NULL);
+  error:
+    if (slavefd != -1) close(slavefd);
+    if (masterfd != -1) close(masterfd);
+    if (!fail) {
+        rb_raise(rb_eRuntimeError, "can't get Master/Slave device");
     }
-    if (!fail) rb_raise(rb_eRuntimeError, "can't get Master/Slave device");
     return -1;
 #elif defined HAVE_OPENPTY
 /*
@@ -320,12 +330,18 @@ get_device_once(int *master, int *slave, char SlaveName[DEVICELEN], int fail)
 	if (!fail) return -1;
 	rb_raise(rb_eRuntimeError, "openpty() failed");
     }
+    if (no_mesg(slavedevice, nomesg) == -1) {
+	if (!fail) return -1;
+	rb_raise(rb_eRuntimeError, "can't chmod slave pty");
+    }
 
     return 0;
+
 #elif defined HAVE__GETPTY
     char *name;
+    mode_t mode = nomesg ? 0600 : 0622;
 
-    if (!(name = _getpty(master, O_RDWR, 0622, 0))) {
+    if (!(name = _getpty(master, O_RDWR, mode, 0))) {
 	if (!fail) return -1;
 	rb_raise(rb_eRuntimeError, "_getpty() failed");
     }
@@ -335,72 +351,70 @@ get_device_once(int *master, int *slave, char SlaveName[DEVICELEN], int fail)
 
     return 0;
 #elif defined(HAVE_PTSNAME)
-    int	 i,j;
-    char *pn;
+    int	 masterfd = -1, slavefd = -1;
+    char *slavedevice;
     void (*s)();
 
     extern char *ptsname(int);
     extern int unlockpt(int);
     extern int grantpt(int);
 
-    if((i = open("/dev/ptmx", O_RDWR, 0)) != -1) {
-	s = signal(SIGCHLD, SIG_DFL);
-	if(grantpt(i) != -1) {
-	    signal(SIGCHLD, s);
-	    if(unlockpt(i) != -1) {
-		if((pn = ptsname(i)) != NULL) {
-		    if((j = open(pn, O_RDWR, 0)) != -1) {
-#if defined I_PUSH && !defined linux
-			if(ioctl(j, I_PUSH, "ptem") != -1) {
-			    if(ioctl(j, I_PUSH, "ldterm") != -1) {
-				ioctl(j, I_PUSH, "ttcompat");
-#endif
-				*master = i;
-				*slave = j;
-				strlcpy(SlaveName, pn, DEVICELEN);
-				return 0;
+    if((masterfd = open("/dev/ptmx", O_RDWR, 0)) == -1) goto error;
+    s = signal(SIGCHLD, SIG_DFL);
+    if(grantpt(masterfd) == -1) goto error;
+    signal(SIGCHLD, s);
+    if(unlockpt(masterfd) == -1) goto error;
+    if((slavedevice = ptsname(masterfd)) == NULL) goto error;
+    if (no_mesg(slavedevice, nomesg) == -1) goto error;
+    if((slavefd = open(slavedevice, O_RDWR, 0)) == -1) goto error;
 #if defined I_PUSH && !defined linux
-			    }
-			}
+    if(ioctl(slavefd, I_PUSH, "ptem") == -1) goto error;
+    if(ioctl(slavefd, I_PUSH, "ldterm") == -1) goto error;
+    ioctl(slavefd, I_PUSH, "ttcompat");
 #endif
-		    }
-		}
-	    }
-	}
-	close(i);
-    }
+    *master = masterfd;
+    *slave = slavefd;
+    strlcpy(SlaveName, slavedevice, DEVICELEN);
+    return 0;
+
+  error:
+    if (slavefd != -1) close(slavefd);
+    if (masterfd != -1) close(masterfd);
     if (!fail) rb_raise(rb_eRuntimeError, "can't get Master/Slave device");
     return -1;
 #else
-    int	 i,j;
+    int	 masterfd = -1, slavefd = -1;
     const char *const *p;
     char MasterName[DEVICELEN];
 
     for (p = deviceNo; *p != NULL; p++) {
 	snprintf(MasterName, sizeof MasterName, MasterDevice, *p);
-	if ((i = open(MasterName,O_RDWR,0)) >= 0) {
-	    *master = i;
+	if ((masterfd = open(MasterName,O_RDWR,0)) >= 0) {
+	    *master = masterfd;
 	    snprintf(SlaveName, DEVICELEN, SlaveDevice, *p);
-	    if ((j = open(SlaveName,O_RDWR,0)) >= 0) {
-		*slave = j;
-		chown(SlaveName, getuid(), getgid());
-		chmod(SlaveName, 0622);
+	    if ((slavefd = open(SlaveName,O_RDWR,0)) >= 0) {
+		*slave = slavefd;
+		if (chown(SlaveName, getuid(), getgid()) != 0) goto error;
+		if (chmod(SlaveName, nomesg ? 0600 : 0622) != 0) goto error;
 		return 0;
 	    }
-	    close(i);
+	    close(masterfd);
 	}
     }
+  error:
+    if (slavefd != -1) close(slavefd);
+    if (masterfd != -1) close(masterfd);
     if (fail) rb_raise(rb_eRuntimeError, "can't get %s", SlaveName);
     return -1;
 #endif
 }
 
 static void
-getDevice(int *master, int *slave, char SlaveName[DEVICELEN])
+getDevice(int *master, int *slave, char SlaveName[DEVICELEN], int nomesg)
 {
-    if (get_device_once(master, slave, SlaveName, 0)) {
+    if (get_device_once(master, slave, SlaveName, nomesg, 0)) {
 	rb_gc();
-	get_device_once(master, slave, SlaveName, 1);
+	get_device_once(master, slave, SlaveName, nomesg, 1);
     }
 }
 
@@ -444,7 +458,7 @@ pty_open(VALUE klass)
     rb_io_t *master_fptr, *slave_fptr;
     VALUE assoc;
 
-    getDevice(&master_fd, &slave_fd, slavename);
+    getDevice(&master_fd, &slave_fd, slavename, 1);
 
     master_io = rb_obj_alloc(rb_cIO);
     MakeOpenFile(master_io, master_fptr);
diff --git a/test/test_pty.rb b/test/test_pty.rb
index 704861dd3c..76986764cd 100644
--- a/test/test_pty.rb
+++ b/test/test_pty.rb
@@ -99,23 +99,11 @@ class TestPTY < Test::Unit::TestCase
   end
 
   def test_stat_slave
-    # If grantpt is used, the slave device is changed as follows.
-    #   owner: real UID
-    #   group: an unspecified value (e.g. tty)
-    #   mode: 0620 (rw--w----)
-    #
-    # The group is not testable because unspecified.
-    #
-    # The mode is testable but the condition is relaxed because other
-    # pty functions (openpty, _getpty, etc.) may not use 0620.
-    # But no one can read from the tty, I hope (for security reason).
-    #
     PTY.open {|master, slave|
       s =  File.stat(slave.path)
       assert_equal(Process.uid, s.uid)
-      assert_equal(0600, s.mode & 0755)
+      assert_equal(0600, s.mode & 0777)
     }
   end
-
 end if defined? PTY