History

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

Christian Brauner reported that if you use the TIOCGPTPEER ioctl() to get a slave pty file descriptor, the resulting file descriptor doesn't look right in /proc/<pid>/fd/<fd>. In particular, he wanted to use readlink() on /proc/self/fd/<fd> to get the pathname of the slave pty (basically implementing "ptsname{_r}()"). The reason for that was that we had generated the wrong 'struct path' when we create the pty in ptmx_open(). In particular, the dentry was correct, but the vfsmount pointed to the mount of the ptmx node. That _can_ be correct - in case you use "/dev/pts/ptmx" to open the master - but usually is not. The normal case is to use /dev/ptmx, which then looks up the pts/ directory, and then the vfsmount of the ptmx node is obviously the /dev directory, not the /dev/pts/ directory. We actually did have the right vfsmount available, but in the wrong place (it gets looked up in 'devpts_acquire()' when we get a reference to the pts filesystem), and so ptmx_open() used the wrong mnt pointer. The end result of this confusion was that the pty worked fine, but when if you did TIOCGPTPEER to get the slave side of the pty, end end result would also work, but have that dodgy 'struct path'. And then when doing "d_path()" on to get the pathname, the vfsmount would not match the root of the pts directory, and d_path() would return an empty pathname thinking that the entry had escaped a bind mount into another mount. This fixes the problem by making devpts_acquire() return the vfsmount for the pts filesystem, allowing ptmx_open() to trivially just use the right mount for the pts dentry, and create the proper 'struct path'.

Linus Torvalds committed 9 years ago

Commit c8c03f11 parent ac9a4093 files · +9 −4Browse files

Unified · Split

3 files
  1. drivers/tty/pty.c +5 −2
  2. fs/devpts/inode.c +3 −1
  3. include/linux/devpts_fs.h +1 −1
drivers/tty/pty.c+5−2
@@ -793,6 +793,7 @@ static int ptmx_open(struct inode *inode, struct file *filp) · 792 unmodified lines
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;
796 int retval;797 int retval;
797 int index;798 int index;
798 799
@@ -805,7 +806,7 @@ static int ptmx_open(struct inode *inode, struct file *filp) · 6 unmodified lines
805 if (retval)806 if (retval)
806 return retval;807 return retval;
807 808
808 fsi = devpts_acquire(filp);809 fsi = devpts_acquire(filp, &mnt);
809 if (IS_ERR(fsi)) {810 if (IS_ERR(fsi)) {
810 retval = PTR_ERR(fsi);811 retval = PTR_ERR(fsi);
811 goto out_free_file;812 goto out_free_file;
@@ -849,7 +850,7 @@ static int ptmx_open(struct inode *inode, struct file *filp) · 37 unmodified lines
849 pts_path = kmalloc(sizeof(struct path), GFP_KERNEL);850 pts_path = kmalloc(sizeof(struct path), GFP_KERNEL);
850 if (!pts_path)851 if (!pts_path)
851 goto err_release;852 goto err_release;
852 pts_path->mnt = filp->f_path.mnt;853 pts_path->mnt = mnt;
853 pts_path->dentry = dentry;854 pts_path->dentry = dentry;
854 path_get(pts_path);855 path_get(pts_path);
855 tty->link->driver_data = pts_path;856 tty->link->driver_data = pts_path;
@@ -866,6 +867,7 @@ err_path_put: · 10 unmodified lines
866 path_put(pts_path);867 path_put(pts_path);
867 kfree(pts_path);868 kfree(pts_path);
868err_release:869err_release:
870 mntput(mnt);
869 tty_unlock(tty);871 tty_unlock(tty);
870 // This will also put-ref the fsi872 // This will also put-ref the fsi
871 tty_release(inode, filp);873 tty_release(inode, filp);
@@ -874,6 +876,7 @@ out: · 2 unmodified lines
874 devpts_kill_index(fsi, index);876 devpts_kill_index(fsi, index);
875out_put_fsi:877out_put_fsi:
876 devpts_release(fsi);878 devpts_release(fsi);
879 mntput(mnt);
877out_free_file:880out_free_file:
878 tty_free_file(filp);881 tty_free_file(filp);
879 return retval;882 return retval;
fs/devpts/inode.c+3−1
@@ -133,7 +133,7 @@ static inline struct pts_fs_info *DEVPTS_SB(struct super_block *sb) · 132 unmodified lines
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)136struct pts_fs_info *devpts_acquire(struct file *filp, struct vfsmount **ptsmnt)
137{137{
138 struct pts_fs_info *result;138 struct pts_fs_info *result;
139 struct path path;139 struct path path;
@@ -142,6 +142,7 @@ struct pts_fs_info *devpts_acquire(struct file *filp) · 2 unmodified lines
142 142
143 path = filp->f_path;143 path = filp->f_path;
144 path_get(&path);144 path_get(&path);
145 *ptsmnt = NULL;
145 146
146 /* Has the devpts filesystem already been found? */147 /* Has the devpts filesystem already been found? */
147 sb = path.mnt->mnt_sb;148 sb = path.mnt->mnt_sb;
@@ -165,6 +166,7 @@ struct pts_fs_info *devpts_acquire(struct file *filp) · 17 unmodified lines
165 * pty code needs to hold extra references in case of last /dev/tty close166 * pty code needs to hold extra references in case of last /dev/tty close
166 */167 */
167 atomic_inc(&sb->s_active);168 atomic_inc(&sb->s_active);
169 *ptsmnt = mntget(path.mnt);
168 result = DEVPTS_SB(sb);170 result = DEVPTS_SB(sb);
169 171
170out:172out:
include/linux/devpts_fs.h+1−1
@@ -19,7 +19,7 @@ · 18 unmodified lines
19 19
20struct pts_fs_info;20struct pts_fs_info;
21 21
22struct pts_fs_info *devpts_acquire(struct file *);22struct pts_fs_info *devpts_acquire(struct file *, struct vfsmount **ptsmnt);
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 *);