Revert "pty: fix the cached path of the pty slave file descriptor in the master"

This reverts commit c8c03f1858331e85d397bacccd34ef409aae993c.

It turns out that while fixing the ptmx file descriptor to have the
correct 'struct path' to the associated slave pty is a really good
thing, it breaks some user space tools for a very annoying reason.

The problem is that /dev/ptmx and its associated slave pty (/dev/pts/X)
are on different mounts.  That was what caused us to have the wrong path
in the first place (we would mix up the vfsmount of the 'ptmx' node,
with the dentry of the pty slave node), but it also means that now while
we use the right vfsmount, having the pty master open also keeps the pts
mount busy.

And it turn sout that that makes 'pbuilder' very unhappy, as noted by
Stefan Lippers-Hollmann:

 "This patch introduces a regression for me when using pbuilder
  0.228.7[2] (a helper to build Debian packages in a chroot and to
  create and update its chroots) when trying to umount /dev/ptmx (inside
  the chroot) on Debian/ unstable (full log and pbuilder configuration
  file[3] attached).

  [...]
  Setting up build-essential (12.3) ...
  Processing triggers for libc-bin (2.24-15) ...
  I: unmounting dev/ptmx filesystem
  W: Could not unmount dev/ptmx: umount: /var/cache/pbuilder/build/1340/dev/ptmx: target is busy
          (In some cases useful info about processes that
           use the device is found by lsof(8) or fuser(1).)"

apparently pbuilder tries to unmount the /dev/pts filesystem while still
holding at least one master node open, which is arguably not very nice,
but we don't break user space even when fixing other bugs.

So this commit has to be reverted.

I'll try to figure out a way to avoid caching the path to the slave pty
in the master pty.  The only thing that actually wants that slave pty
path is the "TIOCGPTPEER" ioctl, and I think we could just recreate the
path at that time.

Reported-by: Stefan Lippers-Hollmann <s.l-h@gmx.de>
Cc: Eric W Biederman <ebiederm@xmission.com>
Cc: Christian Brauner <christian.brauner@canonical.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 1fc80ea87c13c05bcda2a1cb5daf1c3a8d611024..284749fb0f6b96d3d6d667332e569ba0d745e048 100644 (file)
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -793,7 +793,6 @@ static int ptmx_open(struct inode *inode, struct file *filp)
        struct tty_struct *tty;
        struct path *pts_path;
        struct dentry *dentry;
-       struct vfsmount *mnt;
        int retval;
        int index;
 
@@ -806,7 +805,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
        if (retval)
                return retval;
 
-       fsi = devpts_acquire(filp, &mnt);
+       fsi = devpts_acquire(filp);
        if (IS_ERR(fsi)) {
                retval = PTR_ERR(fsi);
                goto out_free_file;
@@ -850,7 +849,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
        pts_path = kmalloc(sizeof(struct path), GFP_KERNEL);
        if (!pts_path)
                goto err_release;
-       pts_path->mnt = mnt;
+       pts_path->mnt = filp->f_path.mnt;
        pts_path->dentry = dentry;
        path_get(pts_path);
        tty->link->driver_data = pts_path;
@@ -867,7 +866,6 @@ err_path_put:
        path_put(pts_path);
        kfree(pts_path);
 err_release:
-       mntput(mnt);
        tty_unlock(tty);
        // This will also put-ref the fsi
        tty_release(inode, filp);
@@ -876,7 +874,6 @@ out:
        devpts_kill_index(fsi, index);
 out_put_fsi:
        devpts_release(fsi);
-       mntput(mnt);
 out_free_file:
        tty_free_file(filp);
        return retval;

diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index 44dfbca9306f04c29081354cc232b8e9b590bb21..108df2e3602c2c63186b61b2fede52794cb46482 100644 (file)
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -133,7 +133,7 @@ static inline struct pts_fs_info *DEVPTS_SB(struct super_block *sb)
        return sb->s_fs_info;
 }
 
-struct pts_fs_info *devpts_acquire(struct file *filp, struct vfsmount **ptsmnt)
+struct pts_fs_info *devpts_acquire(struct file *filp)
 {
        struct pts_fs_info *result;
        struct path path;
@@ -142,7 +142,6 @@ struct pts_fs_info *devpts_acquire(struct file *filp, struct vfsmount **ptsmnt)
 
        path = filp->f_path;
        path_get(&path);
-       *ptsmnt = NULL;
 
        /* Has the devpts filesystem already been found? */
        sb = path.mnt->mnt_sb;
@@ -166,7 +165,6 @@ struct pts_fs_info *devpts_acquire(struct file *filp, struct vfsmount **ptsmnt)
         * pty code needs to hold extra references in case of last /dev/tty close
         */
        atomic_inc(&sb->s_active);
-       *ptsmnt = mntget(path.mnt);
        result = DEVPTS_SB(sb);
 
 out:

diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h
index 7883e901f65c826d8cd98bde911352aa72dafcce..277ab9af9ac29a95773ce2201809f7c7613391a1 100644 (file)
--- a/include/linux/devpts_fs.h
+++ b/include/linux/devpts_fs.h
@@ -19,7 +19,7 @@
 
 struct pts_fs_info;
 
-struct pts_fs_info *devpts_acquire(struct file *, struct vfsmount **ptsmnt);
+struct pts_fs_info *devpts_acquire(struct file *);
 void devpts_release(struct pts_fs_info *);
 
 int devpts_new_index(struct pts_fs_info *);