Skip to content

Commit a319b01

Browse files
Christian Braunergregkh
authored andcommitted
devpts: resolve devpts bind-mounts
Most libcs will still look at /dev/ptmx when opening the master fd of a pty device. When /dev/ptmx is a bind-mount of /dev/pts/ptmx and the TIOCGPTPEER ioctl() is used to safely retrieve a file descriptor for the slave side of the pty based on the master fd, the /proc/self/fd/{0,1,2} symlinks will point to /. A very simply reproducer for this issue presupposing a libc that uses TIOCGPTPEER in its openpty() implementation is: unshare --mount mount --bind /dev/pts/ptmx /dev/ptmx chmod 666 /dev/ptmx script ls -al /proc/self/fd/0 Having bind-mounts of /dev/pts/ptmx to /dev/ptmx not working correctly is a regression. In addition, it is also a fairly common scenario in containers employing user namespaces. The reason for the current failure is that the kernel tries to verify the useability of the devpts filesystem without resolving the /dev/ptmx bind-mount first. This will lead it to detect that the dentry is escaping its bind-mount. The reason is that while the devpts filesystem mounted at /dev/pts has the devtmpfs mounted at /dev as its parent mount: 21 -- -- / /dev -- 21 -- / /dev/pts devtmpfs and devpts are on different devices -- -- 0:6 / /dev -- -- 0:20 / /dev/pts This has the consequence that the pathname of the parent directory of the devpts filesystem mount at /dev/pts is /. So if /dev/ptmx is a bind-mount of /dev/pts/ptmx then the /dev/ptmx bind-mount and the devpts mount at /dev/pts will end up being located on the same device which is recorded in the superblock of their vfsmount. This means the parent directory of the /dev/ptmx bind-mount will be /ptmx: -- -- ---- /ptmx /dev/ptmx Without the bind-mount resolution patch the kernel will now perform the bind-mount escape check directly on /dev/ptmx. The function responsible for this is devpts_ptmx_path() which calls pts_path() which in turn calls path_parent_directory(). Based on the above explanation, path_parent_directory() will yield / as the parent directory for the /dev/ptmx bind-mount and not the expected /dev. Thus, the kernel detects that /dev/ptmx is escaping its bind-mount and will set /proc/<pid>/fd/<nr> to /. This patch changes the logic to first resolve any bind-mounts. After the bind-mounts have been resolved (i.e. we have traced it back to the associated devpts mount) devpts_ptmx_path() can be called. In order to guarantee correct path generation for the slave file descriptor the kernel now requires that a pts directory is found in the parent directory of the ptmx bind-mount. This implies that when doing bind-mounts the ptmx bind-mount and the devpts mount should have a common parent directory. A valid example is: mount -t devpts devpts /dev/pts mount --bind /dev/pts/ptmx /dev/ptmx an invalid example is: mount -t devpts devpts /dev/pts mount --bind /dev/pts/ptmx /ptmx This allows us to support: - calling open on ptmx devices located inside non-standard devpts mounts: mount -t devpts devpts /mnt master = open("/mnt/ptmx", ...); slave = ioctl(master, TIOCGPTPEER, ...); - calling open on ptmx devices located outside the devpts mount with a common ancestor directory: mount -t devpts devpts /dev/pts mount --bind /dev/pts/ptmx /dev/ptmx master = open("/dev/ptmx", ...); slave = ioctl(master, TIOCGPTPEER, ...); while failing on ptmx devices located outside the devpts mount without a common ancestor directory: mount -t devpts devpts /dev/pts mount --bind /dev/pts/ptmx /ptmx master = open("/ptmx", ...); slave = ioctl(master, TIOCGPTPEER, ...); in which case save path generation cannot be guaranteed. Signed-off-by: Christian Brauner <[email protected]> Suggested-by: Eric Biederman <[email protected]> Suggested-by: Linus Torvalds <[email protected]> Reviewed-by: "Eric W. Biederman" <[email protected]> Acked-by: Linus Torvalds <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 7d71109 commit a319b01

File tree

1 file changed

+16
-10
lines changed

1 file changed

+16
-10
lines changed

fs/devpts/inode.c

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -160,21 +160,27 @@ struct vfsmount *devpts_mntget(struct file *filp, struct pts_fs_info *fsi)
160160
path = filp->f_path;
161161
path_get(&path);
162162

163-
/* Has the devpts filesystem already been found? */
164-
if (path.mnt->mnt_sb->s_magic != DEVPTS_SUPER_MAGIC)
163+
/* Walk upward while the start point is a bind mount of
164+
* a single file.
165+
*/
166+
while (path.mnt->mnt_root == path.dentry)
167+
if (follow_up(&path) == 0)
168+
break;
169+
170+
/* devpts_ptmx_path() finds a devpts fs or returns an error. */
171+
if ((path.mnt->mnt_sb->s_magic != DEVPTS_SUPER_MAGIC) ||
172+
(DEVPTS_SB(path.mnt->mnt_sb) != fsi))
165173
err = devpts_ptmx_path(&path);
166174
dput(path.dentry);
167-
if (err) {
168-
mntput(path.mnt);
169-
return ERR_PTR(err);
170-
}
175+
if (!err) {
176+
if (DEVPTS_SB(path.mnt->mnt_sb) == fsi)
177+
return path.mnt;
171178

172-
if (DEVPTS_SB(path.mnt->mnt_sb) != fsi) {
173-
mntput(path.mnt);
174-
return ERR_PTR(-ENODEV);
179+
err = -ENODEV;
175180
}
176181

177-
return path.mnt;
182+
mntput(path.mnt);
183+
return ERR_PTR(err);
178184
}
179185

180186
struct pts_fs_info *devpts_acquire(struct file *filp)

0 commit comments

Comments
 (0)