tty: cleanup prohibition of direct opening for unix98 pty master

cleanup hack added in v2.6.27-3203-g15582d3

comment from that patch:

: pty: If the administrator creates a device for a ptmx slave we should not error :
: The open path for ptmx slaves is via the ptmx device. Opening them any : other way is not allowed. Vegard Nossum found that previously this was not : the case and mknod foo c 128 42; cat foo would produce nasty diagnostics :
: Signed-off-by: Alan Cox <[email protected]>
: Signed-off-by: Linus Torvalds <[email protected]>

Quoted 4 lines · Konstantin Khlebnikov

cleanup hack added in v2.6.27-3203-g15582d3

comment from that patch:

Quoted 8 lines

: pty: If the administrator creates a device for a ptmx slave we should not error :
: The open path for ptmx slaves is via the ptmx device. Opening them any : other way is not allowed. Vegard Nossum found that previously this was not : the case and mknod foo c 128 42; cat foo would produce nasty diagnostics :
: Signed-off-by: Alan Cox<[email protected]>
: Signed-off-by: Linus Torvalds<[email protected]>

Any reason why you didn't include these two people on this patch? Perhaps they might wish to review it...

greg k-h

Quoted 14 lines · Greg KH

Quoted 12 lines · Konstantin Khlebnikov

cleanup hack added in v2.6.27-3203-g15582d3

comment from that patch:

: pty: If the administrator creates a device for a ptmx slave we should not error :
: The open path for ptmx slaves is via the ptmx device. Opening them any : other way is not allowed. Vegard Nossum found that previously this was not : the case and mknod foo c 128 42; cat foo would produce nasty diagnostics :
: Signed-off-by: Alan Cox<[email protected]>
: Signed-off-by: Linus Torvalds<[email protected]>

Quoted 2 lines

Any reason why you didn't include these two people on this patch? Perhaps they might wish to review it...

This is just simple cleanup patch.

cleanup hack added in v2.6.27-3203-g15582d3

comment from that patch:

: pty: If the administrator creates a device for a ptmx slave we should not error :
: The open path for ptmx slaves is via the ptmx device. Opening them any : other way is not allowed. Vegard Nossum found that previously this was not : the case and mknod foo c 128 42; cat foo would produce nasty diagnostics :
: Signed-off-by: Alan Cox <[email protected]>
: Signed-off-by: Linus Torvalds <[email protected]>

devpts_get_tty() returns non-null only for inodes on devpts, but there is no inodes for master-devices, /dev/ptmx (/dev/pts/ptmx) is the only way to open them. Thus we can completely forbid lookup for master-devices and eliminate that hack in tty_init_dev() because tty_open() will get EIO from tty_driver_lookup_tty().

Signed-off-by Konstantin Khlebnikov <[email protected]>

---
 drivers/tty/pty.c    |    8 +++-----
 drivers/tty/tty_io.c |   12 ++----------
 include/linux/tty.h  |    3 +--

 3 files changed, 6 insertions(+), 17 deletions(-)

drivers/tty/pty.c +3 -5

index e18604b..d2bf3c1 100644
@@ -526,10 +526,8 @@ static int pty_unix98_ioctl(struct tty_struct *tty,
  static struct tty_struct *ptm_unix98_lookup(struct tty_driver *driver,
                struct inode *ptm_inode, int idx)
  {
-       struct tty_struct *tty = devpts_get_tty(ptm_inode, idx);
-       if (tty)
-               tty = tty->link;
-       return tty;
+       /* Master must be open via /dev/ptmx */
+       return ERR_PTR(-EIO);
  }
 
  /**

@@ -685,7 +683,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
 
        mutex_lock(&tty_mutex);
        tty_lock();
-       tty = tty_init_dev(ptm_driver, index, 1);
+       tty = tty_init_dev(ptm_driver, index);
        mutex_unlock(&tty_mutex);
 
        if (IS_ERR(tty)) {

drivers/tty/tty_io.c +2 -10

index 05085be..7cddf91 100644
@@ -1366,7 +1366,6 @@ static int tty_reopen(struct tty_struct *tty)
  *     @driver: tty driver we are opening a device on
  *     @idx: device index
  *     @ret_tty: returned tty structure
- *     @first_ok: ok to open a new device (used by ptmx)
  *
  *     Prepare a tty device. This may not be a "new" clean device but
  *     could also be an active device. The pty drivers require special

@@ -1386,18 +1385,11 @@ static int tty_reopen(struct tty_struct *tty)
  * relaxed for the (most common) case of reopening a tty.
  */
 
-struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx,
-                                                               int first_ok)
+struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx)
  {
        struct tty_struct *tty;
        int retval;
 
-       /* Check if pty master is being opened multiple times */
-       if (driver->subtype == PTY_TYPE_MASTER &&
-               (driver->flags & TTY_DRIVER_DEVPTS_MEM) && !first_ok) {
-               return ERR_PTR(-EIO);
-       }
-
        /*
         * First time open is complex, especially for PTY devices.
         * This code guarantees that either everything succeeds and the

@@ -1909,7 +1901,7 @@ got_driver:
                if (retval)
                        tty = ERR_PTR(retval);
        } else
-               tty = tty_init_dev(driver, index, 0);
+               tty = tty_init_dev(driver, index);
 
        mutex_unlock(&tty_mutex);
        tty_driver_kref_put(driver);

include/linux/tty.h +1 -2

index 5dbb3cb..d3ebd76 100644
@@ -480,8 +480,7 @@ extern void free_tty_struct(struct tty_struct *tty);
  extern void initialize_tty_struct(struct tty_struct *tty,
                struct tty_driver *driver, int idx);
  extern void deinitialize_tty_struct(struct tty_struct *tty);
-extern struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx,
-                                                               int first_ok);
+extern struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx);
  extern int tty_release(struct inode *inode, struct file *filp);
  extern int tty_init_termios(struct tty_struct *tty);
 

Looks ok as far as I can tell, and does indeed clean things up. I assume somebody tested the case that we used to have issues with?

Linus

Quoted 19 lines · Konstantin Khlebnikov

cleanup hack added in v2.6.27-3203-g15582d3

comment from that patch:

: pty: If the administrator creates a device for a ptmx slave we should not error :
: The open path for ptmx slaves is via the ptmx device. Opening them any : other way is not allowed. Vegard Nossum found that previously this was not : the case and mknod foo c 128 42; cat foo would produce nasty diagnostics :
: Signed-off-by: Alan Cox<[email protected]>
: Signed-off-by: Linus Torvalds<[email protected]>

devpts_get_tty() returns non-null only for inodes on devpts, but there is no inodes for master-devices, /dev/ptmx (/dev/pts/ptmx) is the only way to open them. Thus we can completely forbid lookup for master-devices and eliminate that hack in tty_init_dev() because tty_open() will get EIO from tty_driver_lookup_tty().

Signed-off-by Konstantin Khlebnikov<[email protected]>

cleanup hack added in v2.6.27-3203-g15582d3

comment from that patch:

: pty: If the administrator creates a device for a ptmx slave we should not error :
: The open path for ptmx slaves is via the ptmx device. Opening them any : other way is not allowed. Vegard Nossum found that previously this was not : the case and mknod foo c 128 42; cat foo would produce nasty diagnostics :
: Signed-off-by: Alan Cox <[email protected]>
: Signed-off-by: Linus Torvalds <[email protected]>

devpts_get_tty() returns non-null only for inodes on devpts, but there is no inodes for master-devices, /dev/ptmx (/dev/pts/ptmx) is the only way to open them. Thus we can completely forbid lookup for master-devices and eliminate that hack in tty_init_dev() because tty_open() will get EIO from tty_driver_lookup_tty().

Signed-off-by Konstantin Khlebnikov <[email protected]>

Quoted 18 lines · Konstantin Khlebnikov

cleanup hack added in v2.6.27-3203-g15582d3

comment from that patch:

: pty: If the administrator creates a device for a ptmx slave we should not error :
: The open path for ptmx slaves is via the ptmx device. Opening them any : other way is not allowed. Vegard Nossum found that previously this was not : the case and mknod foo c 128 42; cat foo would produce nasty diagnostics :
: Signed-off-by: Alan Cox<[email protected]>
: Signed-off-by: Linus Torvalds<[email protected]>

devpts_get_tty() returns non-null only for inodes on devpts, but there is no inodes for master-devices, /dev/ptmx (/dev/pts/ptmx) is the only way to open them. Thus we can completely forbid lookup for master-devices and eliminate that hack in tty_init_dev() because tty_open() will get EIO from tty_driver_lookup_tty().

Quoted 1 line

Signed-off-by Konstantin Khlebnikov<[email protected]>

What tree did you make this patch against? I get fuzz warnings when applying it to the tty-next branch (it's in the linux-next releases).

Please rediff it and resend it.

thanks,

greg k-h