mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Christian Brauner <christian.brauner@ubuntu.com>
To: linux-kernel@vger.kernel.org, serge@hallyn.com,
	torvalds@linux-foundation.org, viro@zeniv.linux.org.uk
Cc: Christian Brauner <christian.brauner@ubuntu.com>
Subject: [PATCH 1/1] devpts: use dynamic_dname() to generate proc name
Date: Wed, 16 Aug 2017 19:12:11 +0200	[thread overview]
Message-ID: <20170816171211.4021-2-christian.brauner@ubuntu.com> (raw)
In-Reply-To: <20170816171211.4021-1-christian.brauner@ubuntu.com>

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@ubuntu.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

  reply	other threads:[~2017-08-16 17:13 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-16 17:12 [PATCH 0/1] " Christian Brauner
2017-08-16 17:12 ` Christian Brauner [this message]
2017-08-16 18:26 ` Linus Torvalds
2017-08-16 18:48   ` Linus Torvalds
2017-08-16 19:48     ` Christian Brauner
2017-08-16 19:56       ` Linus Torvalds
2017-08-16 20:19         ` Linus Torvalds
2017-08-16 20:30           ` Linus Torvalds
2017-08-16 21:03             ` Linus Torvalds
2017-08-16 21:37               ` Christian Brauner
2017-08-16 21:45                 ` Linus Torvalds
2017-08-16 21:55                   ` Linus Torvalds
2017-08-16 22:05                     ` Christian Brauner
2017-08-16 22:28                   ` Christian Brauner
2017-08-23 15:31                     ` Eric W. Biederman
2017-08-23 21:15                       ` Christian Brauner
2017-08-16 22:46               ` Eric W. Biederman
2017-08-16 22:58                 ` Linus Torvalds
2017-08-16 23:51                   ` Eric W. Biederman
2017-08-17  0:08                     ` Linus Torvalds
2017-08-17  1:24                       ` Eric W. Biederman
2017-08-24  0:24                       ` Stefan Lippers-Hollmann
2017-08-24  0:42                         ` Linus Torvalds
2017-08-24  1:16                           ` Linus Torvalds
2017-08-24  1:25                           ` Eric W. Biederman
2017-08-24  1:32                             ` Linus Torvalds
2017-08-24  1:49                               ` Linus Torvalds
2017-08-24  2:01                                 ` Linus Torvalds
2017-08-24  3:11                                   ` Eric W. Biederman
2017-08-24  3:24                                     ` Linus Torvalds
2017-08-24 15:51                                       ` Eric W. Biederman
2017-08-24  4:24                                     ` Stefan Lippers-Hollmann
2017-08-24 15:54                                       ` Eric W. Biederman
2017-08-24 17:52                                         ` Linus Torvalds
2017-08-24 18:06                                           ` Linus Torvalds
2017-08-24 18:13                                             ` Linus Torvalds
2017-08-24 18:31                                               ` Linus Torvalds
2017-08-24 18:36                                                 ` Linus Torvalds
2017-08-24 20:24                                                   ` Stefan Lippers-Hollmann
2017-08-24 20:27                                                     ` Linus Torvalds
2017-08-24 18:40                                               ` Eric W. Biederman
2017-08-24 18:51                                                 ` Linus Torvalds
2017-08-24 19:23                                                   ` Eric W. Biederman
2017-08-24 20:13                                                   ` [PATCH v3] pty: Repair TIOCGPTPEER Eric W. Biederman
2017-08-24 21:01                                                     ` Stefan Lippers-Hollmann
     [not found]                                                 ` <CAPP7u0WHqDfxTW6hmc=DsmHuoALZcrWdU-Odu=FfoTX26SGHQg@mail.gmail.com>
2017-08-24 19:22                                                   ` [PATCH 0/1] devpts: use dynamic_dname() to generate proc name Linus Torvalds
2017-08-24 19:25                                                     ` Linus Torvalds
2017-08-24 20:43                                                     ` Eric W. Biederman
2017-08-24 21:07                                                       ` Linus Torvalds
2017-08-24 23:01                                                         ` Eric W. Biederman
2017-08-24 23:27                                                           ` Linus Torvalds
2017-08-24 23:37                                                           ` Christian Brauner
2017-08-26  1:00                                                             ` Linus Torvalds
2017-08-24 19:48                                                   ` Eric W. Biederman
2017-08-17  1:37           ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170816171211.4021-2-christian.brauner@ubuntu.com \
    --to=christian.brauner@ubuntu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=serge@hallyn.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®