aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLinus Torvalds <torvalds@linux-foundation.org>2017-08-23 21:16:11 -0400
committerLinus Torvalds <torvalds@linux-foundation.org>2017-08-23 21:16:11 -0400
commit143c97cc652949893c8056c679012f0aeccb80e5 (patch)
tree987e3a7fd939ae8169f7ddfc4d0b145a666536fa
parent2acf097f16abba684012cca670a61d94178bd1ab (diff)
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>
-rw-r--r--drivers/tty/pty.c7
-rw-r--r--fs/devpts/inode.c4
-rw-r--r--include/linux/devpts_fs.h2
3 files changed, 4 insertions, 9 deletions
diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 1fc80ea87c13..284749fb0f6b 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -793,7 +793,6 @@ static int ptmx_open(struct inode *inode, struct file *filp)
793 struct tty_struct *tty; 793 struct tty_struct *tty;
794 struct path *pts_path; 794 struct path *pts_path;
795 struct dentry *dentry; 795 struct dentry *dentry;
796 struct vfsmount *mnt;
797 int retval; 796 int retval;
798 int index; 797 int index;
799 798
@@ -806,7 +805,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
806 if (retval) 805 if (retval)
807 return retval; 806 return retval;
808 807
809 fsi = devpts_acquire(filp, &mnt); 808 fsi = devpts_acquire(filp);
810 if (IS_ERR(fsi)) { 809 if (IS_ERR(fsi)) {
811 retval = PTR_ERR(fsi); 810 retval = PTR_ERR(fsi);
812 goto out_free_file; 811 goto out_free_file;
@@ -850,7 +849,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
850 pts_path = kmalloc(sizeof(struct path), GFP_KERNEL); 849 pts_path = kmalloc(sizeof(struct path), GFP_KERNEL);
851 if (!pts_path) 850 if (!pts_path)
852 goto err_release; 851 goto err_release;
853 pts_path->mnt = mnt; 852 pts_path->mnt = filp->f_path.mnt;
854 pts_path->dentry = dentry; 853 pts_path->dentry = dentry;
855 path_get(pts_path); 854 path_get(pts_path);
856 tty->link->driver_data = pts_path; 855 tty->link->driver_data = pts_path;
@@ -867,7 +866,6 @@ err_path_put:
867 path_put(pts_path); 866 path_put(pts_path);
868 kfree(pts_path); 867 kfree(pts_path);
869err_release: 868err_release:
870 mntput(mnt);
871 tty_unlock(tty); 869 tty_unlock(tty);
872 // This will also put-ref the fsi 870 // This will also put-ref the fsi
873 tty_release(inode, filp); 871 tty_release(inode, filp);
@@ -876,7 +874,6 @@ out:
876 devpts_kill_index(fsi, index); 874 devpts_kill_index(fsi, index);
877out_put_fsi: 875out_put_fsi:
878 devpts_release(fsi); 876 devpts_release(fsi);
879 mntput(mnt);
880out_free_file: 877out_free_file:
881 tty_free_file(filp); 878 tty_free_file(filp);
882 return retval; 879 return retval;
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index 44dfbca9306f..108df2e3602c 100644
--- 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)
133 return sb->s_fs_info; 133 return sb->s_fs_info;
134} 134}
135 135
136struct pts_fs_info *devpts_acquire(struct file *filp, struct vfsmount **ptsmnt) 136struct pts_fs_info *devpts_acquire(struct file *filp)
137{ 137{
138 struct pts_fs_info *result; 138 struct pts_fs_info *result;
139 struct path path; 139 struct path path;
@@ -142,7 +142,6 @@ struct pts_fs_info *devpts_acquire(struct file *filp, struct vfsmount **ptsmnt)
142 142
143 path = filp->f_path; 143 path = filp->f_path;
144 path_get(&path); 144 path_get(&path);
145 *ptsmnt = NULL;
146 145
147 /* Has the devpts filesystem already been found? */ 146 /* Has the devpts filesystem already been found? */
148 sb = path.mnt->mnt_sb; 147 sb = path.mnt->mnt_sb;
@@ -166,7 +165,6 @@ struct pts_fs_info *devpts_acquire(struct file *filp, struct vfsmount **ptsmnt)
166 * pty code needs to hold extra references in case of last /dev/tty close 165 * pty code needs to hold extra references in case of last /dev/tty close
167 */ 166 */
168 atomic_inc(&sb->s_active); 167 atomic_inc(&sb->s_active);
169 *ptsmnt = mntget(path.mnt);
170 result = DEVPTS_SB(sb); 168 result = DEVPTS_SB(sb);
171 169
172out: 170out:
diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h
index 7883e901f65c..277ab9af9ac2 100644
--- a/include/linux/devpts_fs.h
+++ b/include/linux/devpts_fs.h
@@ -19,7 +19,7 @@
19 19
20struct pts_fs_info; 20struct pts_fs_info;
21 21
22struct pts_fs_info *devpts_acquire(struct file *, struct vfsmount **ptsmnt); 22struct pts_fs_info *devpts_acquire(struct file *);
23void devpts_release(struct pts_fs_info *); 23void devpts_release(struct pts_fs_info *);
24 24
25int devpts_new_index(struct pts_fs_info *); 25int devpts_new_index(struct pts_fs_info *);