linux-kernel - [PATCH 1/1] devpts: use dynamic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 16 Aug 2017 19:12:11 +0200
From:   Christian Brauner <christian.brauner@...ntu.com>
To:     linux-kernel@...r.kernel.org, serge@...lyn.com,
        torvalds@...ux-foundation.org, viro@...iv.linux.org.uk
Cc:     Christian Brauner <christian.brauner@...ntu.com>
Subject: [PATCH 1/1] devpts: use dynamic_dname() to generate proc name

Recently the kernel has implemented the TIOCGPTPEER ioctl() call which allows
users to retrieve an fd for the slave side of a pty solely based on the
corresponding fd for the master side. The ioctl()-based fd retrieval however
causes the "/proc/<pid>/fd/<pty-slave-fd>" symlink to point to the wrong dentry.
Currently, it will always point to "/".

With the symlink pointing to the wrong path any interesting path-based operation
on the slave side will fail. Also this will cause ttyname{_r}() to falsely
report that this fd does not return to a tty. So I really think this needs to be
fixed. The fix however doesn't seem super obvious to me. It seems the most
straightforward way for now is to behave like the implementation of pipes and
sockets that implement a dynamic_dname() method to correctly set the content of
the "/proc/<pid>/fd/<n>" symlink without requiring special-casing in the proc
implementation itself. I prefer this approach. The downside of this however is
that in case the devpts is not mounted at its standard "/dev/pts" location but
e.g. at "/mnt" the content of the corresponding "/proc/<pid>/fd/<n>" symlink
will still be "/dev/pts/<idx>" although it should likely be "/mnt/<idx". I've
gone over this back and forth in my head but I think that this is not a deal
breaker. All libcs currently hard-code "/dev/pts/<idx>" into their
implementation of ptsname{_r}() and so wouldn't be affected by this change at
all. Furthermore, mounting devpts somewhere other than "/dev/pts" (e.g. "/mnt")
doesn't seem to work and from what I gather from LKML is not really expected nor
supported to work.

Signed-off-by: Christian Brauner <christian.brauner@...ntu.com>
---
 fs/devpts/inode.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index 108df2e3602c..1b1b9b0813e8 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -369,6 +369,18 @@ static const struct super_operations devpts_sops = {
 	.show_options	= devpts_show_options,
 };

+/*
+ * pty_peer_dname() is called from d_path().
+ */
+static char *pty_peer_dname(struct dentry *dentry, char *buffer, int buflen)
+{
+	return dynamic_dname(dentry, buffer, buflen, "/dev/pts/%pd", dentry);
+}
+
+static const struct dentry_operations devpts_dops = {
+	.d_dname = pty_peer_dname,
+};
+
 static void *new_pts_fs_info(struct super_block *sb)
 {
 	struct pts_fs_info *fsi;
@@ -397,6 +409,7 @@ devpts_fill_super(struct super_block *s, void *data, int silent)
 	s->s_magic = DEVPTS_SUPER_MAGIC;
 	s->s_op = &devpts_sops;
 	s->s_time_gran = 1;
+	s->s_d_op = &devpts_dops;

 	error = -ENOMEM;
 	s->s_fs_info = new_pts_fs_info(s);
-- 
2.13.3