linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/17] clean up readlinks
@ 2016-09-12 19:29 Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 01/17] bad_inode: add missing i_op initializers Miklos Szeredi
                   ` (17 more replies)
  0 siblings, 18 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro, David Howells, Tyler Hicks

The first patch is actually a bug fix, but I put it into this bunch for
simplicity...

The rest are really cleanups as well as minor bugfixes that are byproducts
of the cleanups.

This series builds on the fact that i_op.readlink is already set to
generic_readlink() in 43/50 of the cases.  And of those 7 only 4 are doing
something special.  So more than 90% of readlinks are/could actually just
call back into get_link.

The interesting cases are:

 - AFS, which has readlink but not get_link
 - proc, that allow jumping while following symlinks

The first is handled by setting IOP_NOFOLLOW on the inode by the fs.

The second one is handled by introducing is_following_link() which returns
a bool depending on whether current->nameidata is NULL or not.  If it
returns false ->get_link() should behave as ->readlink() did.  Otherwise it
should behave as id did previously.

Builds and boots.  Can even read symlinks.

Git tree is here:

  git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git readlink

Thanks,
Miklos
---

Miklos Szeredi (17):
  bad_inode: add missing i_op initializers
  ovl: use generic_readlink
  proc/self: use generic_readlink
  afs: use generic_readlink
  bad_inode: use generic_readlink
  vfs: remove page_readlink()
  vfs: add is_following_link() helper
  proc: merge proc_pid_readlink() into proc_pid_get_link()
  proc: merge proc_ns_readlink() into proc_ns_get_link()
  nsfs: clean up ns_get_name() interface
  vfs: replace calling i_op->readlink with vfs_readlink()
  vfs: remove ".readlink = generic_readlink" assignments
  vfs: remove unused i_op->readlink
  vfs: remove unused generic_readlink()
  vfs: add vfs_get_link() helper
  ovl: use vfs_get_link()
  ecryptfs: use vfs_get_link()

 Documentation/filesystems/Locking             |  2 -
 Documentation/filesystems/porting             | 15 +++++
 Documentation/filesystems/vfs.txt             | 25 ++++----
 drivers/staging/lustre/lustre/llite/symlink.c |  1 -
 fs/9p/vfs_inode.c                             |  1 -
 fs/9p/vfs_inode_dotl.c                        |  1 -
 fs/affs/symlink.c                             |  1 -
 fs/afs/inode.c                                |  2 +
 fs/afs/mntpt.c                                |  2 +-
 fs/autofs4/symlink.c                          |  1 -
 fs/bad_inode.c                                | 62 ++++++++++++++++----
 fs/btrfs/inode.c                              |  1 -
 fs/ceph/inode.c                               |  1 -
 fs/cifs/cifsfs.c                              |  1 -
 fs/coda/cnode.c                               |  1 -
 fs/configfs/symlink.c                         |  1 -
 fs/ecryptfs/inode.c                           | 30 ++++------
 fs/ext2/symlink.c                             |  2 -
 fs/ext4/symlink.c                             |  3 -
 fs/f2fs/namei.c                               |  2 -
 fs/fuse/dir.c                                 |  1 -
 fs/gfs2/inode.c                               |  1 -
 fs/hostfs/hostfs_kern.c                       |  1 -
 fs/jffs2/symlink.c                            |  1 -
 fs/jfs/symlink.c                              |  2 -
 fs/kernfs/symlink.c                           |  1 -
 fs/libfs.c                                    |  1 -
 fs/minix/inode.c                              |  1 -
 fs/namei.c                                    | 82 ++++++++++++++++++++-------
 fs/ncpfs/inode.c                              |  1 -
 fs/nfs/symlink.c                              |  1 -
 fs/nfsd/nfs4xdr.c                             |  8 +--
 fs/nfsd/vfs.c                                 |  4 +-
 fs/nilfs2/namei.c                             |  1 -
 fs/nsfs.c                                     | 17 ++++--
 fs/ocfs2/symlink.c                            |  1 -
 fs/orangefs/symlink.c                         |  1 -
 fs/overlayfs/copy_up.c                        | 46 ++-------------
 fs/overlayfs/inode.c                          | 25 +-------
 fs/proc/base.c                                | 66 +++++++--------------
 fs/proc/inode.c                               |  1 -
 fs/proc/namespaces.c                          | 41 +++++---------
 fs/proc/self.c                                | 13 -----
 fs/proc/thread_self.c                         | 14 -----
 fs/reiserfs/namei.c                           |  1 -
 fs/squashfs/symlink.c                         |  1 -
 fs/stat.c                                     |  5 +-
 fs/sysv/inode.c                               |  1 -
 fs/ubifs/file.c                               |  1 -
 fs/xfs/xfs_ioctl.c                            |  4 +-
 fs/xfs/xfs_iops.c                             |  2 -
 include/linux/fs.h                            |  9 ++-
 include/linux/namei.h                         |  1 +
 include/linux/proc_ns.h                       |  4 +-
 mm/shmem.c                                    |  2 -
 55 files changed, 226 insertions(+), 291 deletions(-)

-- 
2.5.5

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 01/17] bad_inode: add missing i_op initializers
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 02/17] ovl: use generic_readlink Miklos Szeredi
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

New inode operations were forgotten to be added to bad_inode.  Most of the
time the op is checked for NULL before being called but marking the inode
bad and the check can race (very unlikely).

However in case of ->get_link() only DCACHE_SYMLINK_TYPE is checked before
calling the op, so there's no race and will definitely oops when trying to
follow links on such a beast.

Also remove comments about extinct ops.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: <stable@vger.kernel.org>
---
 fs/bad_inode.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 4 deletions(-)

diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index 3ba385eaa26e..b8db8531e837 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -123,6 +123,50 @@ static int bad_inode_removexattr(struct dentry *dentry, const char *name)
 	return -EIO;
 }
 
+static const char *bad_inode_get_link(struct dentry *dentry,
+				      struct inode *inode,
+				      struct delayed_call *done)
+{
+	return ERR_PTR(-EIO);
+}
+
+static struct posix_acl *bad_inode_get_acl(struct inode *inode, int type)
+{
+	return ERR_PTR(-EIO);
+}
+
+static int bad_inode_fiemap(struct inode *inode,
+			    struct fiemap_extent_info *fieinfo, u64 start,
+			    u64 len)
+{
+	return -EIO;
+}
+
+static int bad_inode_update_time(struct inode *inode, struct timespec *time,
+				 int flags)
+{
+	return -EIO;
+}
+
+static int bad_inode_atomic_open(struct inode *inode, struct dentry *dentry,
+				 struct file *file, unsigned int open_flag,
+				 umode_t create_mode, int *opened)
+{
+	return -EIO;
+}
+
+static int bad_inode_tmpfile(struct inode *inode, struct dentry *dentry,
+			     umode_t mode)
+{
+	return -EIO;
+}
+
+static int bad_inode_set_acl(struct inode *inode, struct posix_acl *acl,
+			     int type)
+{
+	return -EIO;
+}
+
 static const struct inode_operations bad_inode_ops =
 {
 	.create		= bad_inode_create,
@@ -135,10 +179,6 @@ static const struct inode_operations bad_inode_ops =
 	.mknod		= bad_inode_mknod,
 	.rename2	= bad_inode_rename2,
 	.readlink	= bad_inode_readlink,
-	/* follow_link must be no-op, otherwise unmounting this inode
-	   won't work */
-	/* put_link returns void */
-	/* truncate returns void */
 	.permission	= bad_inode_permission,
 	.getattr	= bad_inode_getattr,
 	.setattr	= bad_inode_setattr,
@@ -146,6 +186,13 @@ static const struct inode_operations bad_inode_ops =
 	.getxattr	= bad_inode_getxattr,
 	.listxattr	= bad_inode_listxattr,
 	.removexattr	= bad_inode_removexattr,
+	.get_link	= bad_inode_get_link,
+	.get_acl	= bad_inode_get_acl,
+	.fiemap		= bad_inode_fiemap,
+	.update_time	= bad_inode_update_time,
+	.atomic_open	= bad_inode_atomic_open,
+	.tmpfile	= bad_inode_tmpfile,
+	.set_acl	= bad_inode_set_acl,
 };
 
 
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 02/17] ovl: use generic_readlink
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 01/17] bad_inode: add missing i_op initializers Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 03/17] proc/self: " Miklos Szeredi
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

This will allow us to move the generic readlink logic into the VFS and get
rid of the readlink method.

All filesystems that are backers for overlayfs would also use
generic_readlink().  Move this logic to the overlay itself, which is a nice
cleanup in itself.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/overlayfs/inode.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index c75625c1efa3..3b636e3acda4 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -173,25 +173,6 @@ static const char *ovl_get_link(struct dentry *dentry,
 	return p;
 }
 
-static int ovl_readlink(struct dentry *dentry, char __user *buf, int bufsiz)
-{
-	struct path realpath;
-	struct inode *realinode;
-	const struct cred *old_cred;
-	int err;
-
-	ovl_path_real(dentry, &realpath);
-	realinode = realpath.dentry->d_inode;
-
-	if (!realinode->i_op->readlink)
-		return -EINVAL;
-
-	old_cred = ovl_override_creds(dentry->d_sb);
-	err = realinode->i_op->readlink(realpath.dentry, buf, bufsiz);
-	revert_creds(old_cred);
-	return err;
-}
-
 bool ovl_is_private_xattr(const char *name)
 {
 	return strncmp(name, OVL_XATTR_PREFIX,
@@ -378,7 +359,7 @@ static const struct inode_operations ovl_file_inode_operations = {
 static const struct inode_operations ovl_symlink_inode_operations = {
 	.setattr	= ovl_setattr,
 	.get_link	= ovl_get_link,
-	.readlink	= ovl_readlink,
+	.readlink	= generic_readlink,
 	.getattr	= ovl_getattr,
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 03/17] proc/self: use generic_readlink
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 01/17] bad_inode: add missing i_op initializers Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 02/17] ovl: use generic_readlink Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 04/17] afs: " Miklos Szeredi
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

This will also allow us to move the generic readlink logic into the VFS and
get rid of the readlink method.

The /proc/self and /proc/self-thread symlinks have separate but identical
functionality for reading and following.  This cleanup utilizes
generic_readlink to remove the duplication.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/proc/self.c        | 14 +-------------
 fs/proc/thread_self.c | 15 +--------------
 2 files changed, 2 insertions(+), 27 deletions(-)

diff --git a/fs/proc/self.c b/fs/proc/self.c
index b6a8d3529fea..80bc52fd7846 100644
--- a/fs/proc/self.c
+++ b/fs/proc/self.c
@@ -6,18 +6,6 @@
 /*
  * /proc/self:
  */
-static int proc_self_readlink(struct dentry *dentry, char __user *buffer,
-			      int buflen)
-{
-	struct pid_namespace *ns = dentry->d_sb->s_fs_info;
-	pid_t tgid = task_tgid_nr_ns(current, ns);
-	char tmp[PROC_NUMBUF];
-	if (!tgid)
-		return -ENOENT;
-	sprintf(tmp, "%d", tgid);
-	return readlink_copy(buffer, buflen, tmp);
-}
-
 static const char *proc_self_get_link(struct dentry *dentry,
 				      struct inode *inode,
 				      struct delayed_call *done)
@@ -38,7 +26,7 @@ static const char *proc_self_get_link(struct dentry *dentry,
 }
 
 static const struct inode_operations proc_self_inode_operations = {
-	.readlink	= proc_self_readlink,
+	.readlink	= generic_readlink,
 	.get_link	= proc_self_get_link,
 };
 
diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
index e58a31e8fb2a..55053d5280ff 100644
--- a/fs/proc/thread_self.c
+++ b/fs/proc/thread_self.c
@@ -6,19 +6,6 @@
 /*
  * /proc/thread_self:
  */
-static int proc_thread_self_readlink(struct dentry *dentry, char __user *buffer,
-			      int buflen)
-{
-	struct pid_namespace *ns = dentry->d_sb->s_fs_info;
-	pid_t tgid = task_tgid_nr_ns(current, ns);
-	pid_t pid = task_pid_nr_ns(current, ns);
-	char tmp[PROC_NUMBUF + 6 + PROC_NUMBUF];
-	if (!pid)
-		return -ENOENT;
-	sprintf(tmp, "%d/task/%d", tgid, pid);
-	return readlink_copy(buffer, buflen, tmp);
-}
-
 static const char *proc_thread_self_get_link(struct dentry *dentry,
 					     struct inode *inode,
 					     struct delayed_call *done)
@@ -40,7 +27,7 @@ static const char *proc_thread_self_get_link(struct dentry *dentry,
 }
 
 static const struct inode_operations proc_thread_self_inode_operations = {
-	.readlink	= proc_thread_self_readlink,
+	.readlink	= generic_readlink,
 	.get_link	= proc_thread_self_get_link,
 };
 
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 04/17] afs: use generic_readlink
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (2 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 03/17] proc/self: " Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 05/17] bad_inode: " Miklos Szeredi
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro, David Howells

This will allow us to move the generic readlink logic into the VFS and get
rid of the readlink method.

AFS mountpoints seem to be special symlinks that can be read but not
followed.  So add IOP_NOFOLLOW to i_opflags and use page_get_link +
generic_readlink instead of page_readlink.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: David Howells <dhowells@redhat.com>
---
 fs/afs/inode.c | 2 ++
 fs/afs/mntpt.c | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 86cc7264c21c..56709b2c46e3 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -88,6 +88,8 @@ static int afs_inode_map_status(struct afs_vnode *vnode, struct key *key)
 			inode->i_mode	= S_IFDIR | vnode->status.mode;
 			inode->i_op	= &afs_mntpt_inode_operations;
 			inode->i_fop	= &afs_mntpt_file_operations;
+			/* Not a real symlink, don't follow */
+			inode->i_opflags |= IOP_NOFOLLOW;
 		}
 	}
 
diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c
index 81dd075356b9..7d170de2ce1b 100644
--- a/fs/afs/mntpt.c
+++ b/fs/afs/mntpt.c
@@ -33,7 +33,8 @@ const struct file_operations afs_mntpt_file_operations = {
 
 const struct inode_operations afs_mntpt_inode_operations = {
 	.lookup		= afs_mntpt_lookup,
-	.readlink	= page_readlink,
+	.get_link	= page_get_link,
+	.readlink	= generic_readlink,
 	.getattr	= afs_getattr,
 };
 
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 05/17] bad_inode: use generic_readlink
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (3 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 04/17] afs: " Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 06/17] vfs: remove page_readlink() Miklos Szeredi
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

This will allow us to move the generic readlink logic into the VFS and get
rid of the readlink method.

If inode has i_link set then this patch changes behavior, because
generic_readlink will return the cached contents instead of calling
i_op->get_link.  It would be hard to imagine that this change could break
userspace in any sane setup, though.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/bad_inode.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index b8db8531e837..7cf1f445fe04 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -78,12 +78,6 @@ static int bad_inode_rename2(struct inode *old_dir, struct dentry *old_dentry,
 	return -EIO;
 }
 
-static int bad_inode_readlink(struct dentry *dentry, char __user *buffer,
-		int buflen)
-{
-	return -EIO;
-}
-
 static int bad_inode_permission(struct inode *inode, int mask)
 {
 	return -EIO;
@@ -178,7 +172,7 @@ static const struct inode_operations bad_inode_ops =
 	.rmdir		= bad_inode_rmdir,
 	.mknod		= bad_inode_mknod,
 	.rename2	= bad_inode_rename2,
-	.readlink	= bad_inode_readlink,
+	.readlink	= generic_readlink,
 	.permission	= bad_inode_permission,
 	.getattr	= bad_inode_getattr,
 	.setattr	= bad_inode_setattr,
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 06/17] vfs: remove page_readlink()
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (4 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 05/17] bad_inode: " Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 07/17] vfs: add is_following_link() helper Miklos Szeredi
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

Remove this unused helper (sole previous user was AFS).

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/namei.c         | 11 -----------
 include/linux/fs.h |  1 -
 2 files changed, 12 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index adb04146df09..c06a68b82088 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4713,17 +4713,6 @@ void page_put_link(void *arg)
 }
 EXPORT_SYMBOL(page_put_link);
 
-int page_readlink(struct dentry *dentry, char __user *buffer, int buflen)
-{
-	DEFINE_DELAYED_CALL(done);
-	int res = readlink_copy(buffer, buflen,
-				page_get_link(dentry, d_inode(dentry),
-					      &done));
-	do_delayed_call(&done);
-	return res;
-}
-EXPORT_SYMBOL(page_readlink);
-
 /*
  * The nofs argument instructs pagecache_write_begin to pass AOP_FLAG_NOFS
  */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 901e25d495cc..5448a9b88c41 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2890,7 +2890,6 @@ extern const struct file_operations generic_ro_fops;
 #define special_file(m) (S_ISCHR(m)||S_ISBLK(m)||S_ISFIFO(m)||S_ISSOCK(m))
 
 extern int readlink_copy(char __user *, int, const char *);
-extern int page_readlink(struct dentry *, char __user *, int);
 extern const char *page_get_link(struct dentry *, struct inode *,
 				 struct delayed_call *);
 extern void page_put_link(void *);
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 07/17] vfs: add is_following_link() helper
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (5 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 06/17] vfs: remove page_readlink() Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 08/17] proc: merge proc_pid_readlink() into proc_pid_get_link() Miklos Szeredi
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

The remainging instances of ->readlink that don't use generic_readlink are
/proc/$$/fd/N, /proc/$$/map_files/A and /proc/$$/ns/X.  The reason is that
these have special get_link() implementations is that they "jump" to
locations not indicated by the symlink contents.

Since there are so few of these (the fd and map_files ones are essentially
the same), it doesn't make sense to create a separate i_op method for them.
This patch adds a helper: is_following_link() by which the two different
modes of operation can be differentiated by the special get_link()
implementations.

Also add a WARN_ON_ONCE() in generic_readlink() to prevent misuse.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/namei.c            | 9 +++++++++
 include/linux/namei.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/fs/namei.c b/fs/namei.c
index c06a68b82088..f72c405d1a27 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -880,6 +880,11 @@ void nd_jump_link(struct path *path)
 	nd->flags |= LOOKUP_JUMPED;
 }
 
+bool is_following_link(void)
+{
+	return current->nameidata;
+}
+
 static inline void put_link(struct nameidata *nd)
 {
 	struct saved *last = nd->stack + --nd->depth;
@@ -4670,6 +4675,10 @@ int generic_readlink(struct dentry *dentry, char __user *buffer, int buflen)
 		link = inode->i_op->get_link(dentry, inode, &done);
 		if (IS_ERR(link))
 			return PTR_ERR(link);
+
+		/* "jumping" is unacceptable, warn and return error */
+		if (WARN_ON_ONCE(!link))
+			return -EIO;
 	}
 	res = readlink_copy(buffer, buflen, link);
 	do_delayed_call(&done);
diff --git a/include/linux/namei.h b/include/linux/namei.h
index f29abda31e6d..ec7b8ccfe064 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -90,6 +90,7 @@ extern struct dentry *lock_rename(struct dentry *, struct dentry *);
 extern void unlock_rename(struct dentry *, struct dentry *);
 
 extern void nd_jump_link(struct path *path);
+extern bool is_following_link(void);
 
 static inline void nd_terminate_link(void *name, size_t len, size_t maxlen)
 {
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 08/17] proc: merge proc_pid_readlink() into proc_pid_get_link()
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (6 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 07/17] vfs: add is_following_link() helper Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 09/17] proc: merge proc_ns_readlink() into proc_ns_get_link() Miklos Szeredi
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

This will allow us to move the generic readlink logic into the VFS and get
rid of the readlink method.

And it's a cleanup, removing more lines than it adds, since the two
functions have a lot in common.

/proc/$$/map_files/A allowed reading the symlink with the normal proc
permission checks, but following only allowed for CAP_SYS_ADMIN capable
tasks.  So in proc_map_files_get_link() check for is_following_link()
before bailing out if not CAP_SYS_ADMIN capable.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/proc/base.c | 68 ++++++++++++++++++----------------------------------------
 1 file changed, 21 insertions(+), 47 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index ac0df4dde823..84769c763afe 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1578,6 +1578,7 @@ static const char *proc_pid_get_link(struct dentry *dentry,
 {
 	struct path path;
 	int error = -EACCES;
+	char *res;
 
 	if (!dentry)
 		return ERR_PTR(-ECHILD);
@@ -1590,58 +1591,31 @@ static const char *proc_pid_get_link(struct dentry *dentry,
 	if (error)
 		goto out;
 
-	nd_jump_link(&path);
-	return NULL;
-out:
-	return ERR_PTR(error);
-}
-
-static int do_proc_readlink(struct path *path, char __user *buffer, int buflen)
-{
-	char *tmp = (char*)__get_free_page(GFP_TEMPORARY);
-	char *pathname;
-	int len;
-
-	if (!tmp)
-		return -ENOMEM;
-
-	pathname = d_path(path, tmp, PAGE_SIZE);
-	len = PTR_ERR(pathname);
-	if (IS_ERR(pathname))
-		goto out;
-	len = tmp + PAGE_SIZE - 1 - pathname;
-
-	if (len > buflen)
-		len = buflen;
-	if (copy_to_user(buffer, pathname, len))
-		len = -EFAULT;
- out:
-	free_page((unsigned long)tmp);
-	return len;
-}
-
-static int proc_pid_readlink(struct dentry * dentry, char __user * buffer, int buflen)
-{
-	int error = -EACCES;
-	struct inode *inode = d_inode(dentry);
-	struct path path;
+	if (is_following_link()) {
+		nd_jump_link(&path);
+		res = NULL;
+	} else {
+		char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
 
-	/* Are we allowed to snoop on the tasks file descriptors? */
-	if (!proc_fd_access_allowed(inode))
-		goto out;
+		error = -ENOMEM;
+		if (!buf)
+			goto out;
 
-	error = PROC_I(inode)->op.proc_get_link(dentry, &path);
-	if (error)
-		goto out;
+		res = d_path(&path, buf, PAGE_SIZE);
+		if (IS_ERR(res))
+			kfree(buf);
+		else
+			set_delayed_call(done, kfree_link, buf);
+	}
+	return res;
 
-	error = do_proc_readlink(&path, buffer, buflen);
-	path_put(&path);
 out:
-	return error;
+	return ERR_PTR(error);
 }
 
+
 const struct inode_operations proc_pid_link_inode_operations = {
-	.readlink	= proc_pid_readlink,
+	.readlink	= generic_readlink,
 	.get_link	= proc_pid_get_link,
 	.setattr	= proc_setattr,
 };
@@ -1966,7 +1940,7 @@ proc_map_files_get_link(struct dentry *dentry,
 			struct inode *inode,
 		        struct delayed_call *done)
 {
-	if (!capable(CAP_SYS_ADMIN))
+	if (is_following_link() && !capable(CAP_SYS_ADMIN))
 		return ERR_PTR(-EPERM);
 
 	return proc_pid_get_link(dentry, inode, done);
@@ -1976,7 +1950,7 @@ proc_map_files_get_link(struct dentry *dentry,
  * Identical to proc_pid_link_inode_operations except for get_link()
  */
 static const struct inode_operations proc_map_files_link_inode_operations = {
-	.readlink	= proc_pid_readlink,
+	.readlink	= generic_readlink,
 	.get_link	= proc_map_files_get_link,
 	.setattr	= proc_setattr,
 };
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 09/17] proc: merge proc_ns_readlink() into proc_ns_get_link()
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (7 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 08/17] proc: merge proc_pid_readlink() into proc_pid_get_link() Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 10/17] nsfs: clean up ns_get_name() interface Miklos Szeredi
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

This will allow us to move the generic readlink logic into the VFS and get
rid of the readlink method.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/proc/namespaces.c | 56 +++++++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c
index 51b8b0a8ad91..9c9a1683791a 100644
--- a/fs/proc/namespaces.c
+++ b/fs/proc/namespaces.c
@@ -33,6 +33,8 @@ static const struct proc_ns_operations *ns_entries[] = {
 #endif
 };
 
+#define PROC_NS_LINK_MAX 50
+
 static const char *proc_ns_get_link(struct dentry *dentry,
 				    struct inode *inode,
 				    struct delayed_call *done)
@@ -40,47 +42,47 @@ static const char *proc_ns_get_link(struct dentry *dentry,
 	const struct proc_ns_operations *ns_ops = PROC_I(inode)->ns_ops;
 	struct task_struct *task;
 	struct path ns_path;
-	void *error = ERR_PTR(-EACCES);
+	char *res = ERR_PTR(-EACCES);
 
 	if (!dentry)
 		return ERR_PTR(-ECHILD);
 
 	task = get_proc_task(inode);
 	if (!task)
-		return error;
-
-	if (ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) {
-		error = ns_get_path(&ns_path, task, ns_ops);
-		if (!error)
-			nd_jump_link(&ns_path);
-	}
-	put_task_struct(task);
-	return error;
-}
-
-static int proc_ns_readlink(struct dentry *dentry, char __user *buffer, int buflen)
-{
-	struct inode *inode = d_inode(dentry);
-	const struct proc_ns_operations *ns_ops = PROC_I(inode)->ns_ops;
-	struct task_struct *task;
-	char name[50];
-	int res = -EACCES;
+		goto out;
 
-	task = get_proc_task(inode);
-	if (!task)
-		return res;
+	if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS))
+		goto out_put;
 
-	if (ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) {
-		res = ns_get_name(name, sizeof(name), task, ns_ops);
-		if (res >= 0)
-			res = readlink_copy(buffer, buflen, name);
+	if (is_following_link()) {
+		res = ns_get_path(&ns_path, task, ns_ops);
+		if (!res)
+			nd_jump_link(&ns_path);
+	} else {
+		char *name = kmalloc(PROC_NS_LINK_MAX, GFP_KERNEL);
+		int err;
+
+		res = ERR_PTR(-ENOMEM);
+		if (!name)
+			goto out_put;
+
+		err = ns_get_name(name, PROC_NS_LINK_MAX, task, ns_ops);
+		if (err < 0) {
+			kfree(name);
+			res = ERR_PTR(err);
+			goto out_put;
+		}
+		set_delayed_call(done, kfree_link, name);
+		res = name;
 	}
+out_put:
 	put_task_struct(task);
+out:
 	return res;
 }
 
 static const struct inode_operations proc_ns_link_inode_operations = {
-	.readlink	= proc_ns_readlink,
+	.readlink	= generic_readlink,
 	.get_link	= proc_ns_get_link,
 	.setattr	= proc_setattr,
 };
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 10/17] nsfs: clean up ns_get_name() interface
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (8 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 09/17] proc: merge proc_ns_readlink() into proc_ns_get_link() Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 11/17] vfs: replace calling i_op->readlink with vfs_readlink() Miklos Szeredi
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

The only user of this is proc_ns_get_link(), which needs an allocated
buffer.  And ns_get_name() is much better positioned to know the size of
the buffer to allocate, since it's the one that is going to fill it.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/nsfs.c               | 17 +++++++++++++----
 fs/proc/namespaces.c    | 20 +++-----------------
 include/linux/proc_ns.h |  4 ++--
 3 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/fs/nsfs.c b/fs/nsfs.c
index 8f20d6016e20..33ec85b91aba 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -5,6 +5,7 @@
 #include <linux/magic.h>
 #include <linux/ktime.h>
 #include <linux/seq_file.h>
+#include <linux/slab.h>
 
 static struct vfsmount *nsfs_mnt;
 
@@ -106,14 +107,22 @@ slow:
 	goto got_it;
 }
 
-int ns_get_name(char *buf, size_t size, struct task_struct *task,
-			const struct proc_ns_operations *ns_ops)
+char *ns_get_name(struct task_struct *task,
+		  const struct proc_ns_operations *ns_ops)
 {
 	struct ns_common *ns;
-	int res = -ENOENT;
+	char *res = ERR_PTR(-ENOENT);
+
 	ns = ns_ops->get(task);
 	if (ns) {
-		res = snprintf(buf, size, "%s:[%u]", ns_ops->name, ns->inum);
+		/* 10 for unsigned int in decimal + 3 extra chars + term null */
+		size_t size = strlen(ns_ops->name) + 14;
+
+		res = kmalloc(size, GFP_KERNEL);
+		if (!res)
+			res = ERR_PTR(-ENOMEM);
+		else
+			snprintf(res, size, "%s:[%u]", ns_ops->name, ns->inum);
 		ns_ops->put(ns);
 	}
 	return res;
diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c
index 9c9a1683791a..9c7209734b05 100644
--- a/fs/proc/namespaces.c
+++ b/fs/proc/namespaces.c
@@ -33,8 +33,6 @@ static const struct proc_ns_operations *ns_entries[] = {
 #endif
 };
 
-#define PROC_NS_LINK_MAX 50
-
 static const char *proc_ns_get_link(struct dentry *dentry,
 				    struct inode *inode,
 				    struct delayed_call *done)
@@ -59,21 +57,9 @@ static const char *proc_ns_get_link(struct dentry *dentry,
 		if (!res)
 			nd_jump_link(&ns_path);
 	} else {
-		char *name = kmalloc(PROC_NS_LINK_MAX, GFP_KERNEL);
-		int err;
-
-		res = ERR_PTR(-ENOMEM);
-		if (!name)
-			goto out_put;
-
-		err = ns_get_name(name, PROC_NS_LINK_MAX, task, ns_ops);
-		if (err < 0) {
-			kfree(name);
-			res = ERR_PTR(err);
-			goto out_put;
-		}
-		set_delayed_call(done, kfree_link, name);
-		res = name;
+		res = ns_get_name(task, ns_ops);
+		if (!IS_ERR(res))
+			set_delayed_call(done, kfree_link, res);
 	}
 out_put:
 	put_task_struct(task);
diff --git a/include/linux/proc_ns.h b/include/linux/proc_ns.h
index de0e7719d4c5..9ab3fb9b9101 100644
--- a/include/linux/proc_ns.h
+++ b/include/linux/proc_ns.h
@@ -74,8 +74,8 @@ extern struct file *proc_ns_fget(int fd);
 extern void *ns_get_path(struct path *path, struct task_struct *task,
 			const struct proc_ns_operations *ns_ops);
 
-extern int ns_get_name(char *buf, size_t size, struct task_struct *task,
-			const struct proc_ns_operations *ns_ops);
+char *ns_get_name(struct task_struct *task,
+		  const struct proc_ns_operations *ns_ops);
 extern void nsfs_init(void);
 
 #endif /* _LINUX_PROC_NS_H */
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 11/17] vfs: replace calling i_op->readlink with vfs_readlink()
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (9 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 10/17] nsfs: clean up ns_get_name() interface Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 12/17] vfs: remove ".readlink = generic_readlink" assignments Miklos Szeredi
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

At this point all ->readlink instances are set to generic_readlink.  So
instead of calling i_op->readlink, we can call generic_readlink directly.

Add an alias for generic_readlink(): vfs_readlink().  They are exactly the
same but result in consistent naming.  We can get rid of generic_readlink()
after the next patch.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/ecryptfs/inode.c    | 4 +---
 fs/nfsd/nfs4xdr.c      | 8 ++++----
 fs/nfsd/vfs.c          | 4 ++--
 fs/overlayfs/copy_up.c | 5 ++---
 fs/stat.c              | 5 ++---
 fs/xfs/xfs_ioctl.c     | 4 ++--
 include/linux/fs.h     | 6 ++++++
 7 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 9d153b6a1d72..9885a8f88260 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -638,9 +638,7 @@ static char *ecryptfs_readlink_lower(struct dentry *dentry, size_t *bufsiz)
 		return ERR_PTR(-ENOMEM);
 	old_fs = get_fs();
 	set_fs(get_ds());
-	rc = d_inode(lower_dentry)->i_op->readlink(lower_dentry,
-						   (char __user *)lower_buf,
-						   PATH_MAX);
+	rc = vfs_readlink(lower_dentry, (char __user *)lower_buf, PATH_MAX);
 	set_fs(old_fs);
 	if (rc < 0)
 		goto out;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 0aa0236a1429..623268f32ee2 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3552,10 +3552,10 @@ nfsd4_encode_readlink(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd
 	if (!p)
 		return nfserr_resource;
 	/*
-	 * XXX: By default, the ->readlink() VFS op will truncate symlinks
-	 * if they would overflow the buffer.  Is this kosher in NFSv4?  If
-	 * not, one easy fix is: if ->readlink() precisely fills the buffer,
-	 * assume that truncation occurred, and return NFS4ERR_RESOURCE.
+	 * XXX: By default, vfs_readlink() will truncate symlinks if they
+	 * would overflow the buffer.  Is this kosher in NFSv4?  If not, one
+	 * easy fix is: if vfs_readlink() precisely fills the buffer, assume
+	 * that truncation occurred, and return NFS4ERR_RESOURCE.
 	 */
 	nfserr = nfsd_readlink(readlink->rl_rqstp, readlink->rl_fhp,
 						(char *)p, &maxcount);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index ff476e654b8f..196620e1dc71 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1450,7 +1450,7 @@ nfsd_readlink(struct svc_rqst *rqstp, struct svc_fh *fhp, char *buf, int *lenp)
 	inode = d_inode(path.dentry);
 
 	err = nfserr_inval;
-	if (!inode->i_op->readlink)
+	if (!inode->i_op->get_link)
 		goto out;
 
 	touch_atime(&path);
@@ -1459,7 +1459,7 @@ nfsd_readlink(struct svc_rqst *rqstp, struct svc_fh *fhp, char *buf, int *lenp)
 	 */
 
 	oldfs = get_fs(); set_fs(KERNEL_DS);
-	host_err = inode->i_op->readlink(path.dentry, (char __user *)buf, *lenp);
+	host_err = vfs_readlink(path.dentry, (char __user *)buf, *lenp);
 	set_fs(oldfs);
 
 	if (host_err < 0)
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 43fdc2765aea..a2d4c10dd74d 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -175,7 +175,7 @@ static char *ovl_read_symlink(struct dentry *realdentry)
 	mm_segment_t old_fs;
 
 	res = -EINVAL;
-	if (!inode->i_op->readlink)
+	if (!inode->i_op->get_link)
 		goto err;
 
 	res = -ENOMEM;
@@ -186,8 +186,7 @@ static char *ovl_read_symlink(struct dentry *realdentry)
 	old_fs = get_fs();
 	set_fs(get_ds());
 	/* The cast to a user pointer is valid due to the set_fs() */
-	res = inode->i_op->readlink(realdentry,
-				    (char __user *)buf, PAGE_SIZE - 1);
+	res = vfs_readlink(realdentry, (char __user *)buf, PAGE_SIZE - 1);
 	set_fs(old_fs);
 	if (res < 0) {
 		free_page((unsigned long) buf);
diff --git a/fs/stat.c b/fs/stat.c
index bc045c7994e1..a9bf2e8db854 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -329,12 +329,11 @@ retry:
 		struct inode *inode = d_backing_inode(path.dentry);
 
 		error = empty ? -ENOENT : -EINVAL;
-		if (inode->i_op->readlink) {
+		if (inode->i_op->get_link) {
 			error = security_inode_readlink(path.dentry);
 			if (!error) {
 				touch_atime(&path);
-				error = inode->i_op->readlink(path.dentry,
-							      buf, bufsiz);
+				error = vfs_readlink(path.dentry, buf, bufsiz);
 			}
 		}
 		path_put(&path);
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 96a70fd1f5d6..e8fdac738dc4 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -287,7 +287,7 @@ xfs_readlink_by_handle(
 		return PTR_ERR(dentry);
 
 	/* Restrict this handle operation to symlinks only. */
-	if (!d_inode(dentry)->i_op->readlink) {
+	if (!d_inode(dentry)->i_op->get_link) {
 		error = -EINVAL;
 		goto out_dput;
 	}
@@ -297,7 +297,7 @@ xfs_readlink_by_handle(
 		goto out_dput;
 	}
 
-	error = d_inode(dentry)->i_op->readlink(dentry, hreq->ohandle, olen);
+	error = vfs_readlink(dentry, hreq->ohandle, olen);
 
  out_dput:
 	dput(dentry);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5448a9b88c41..bcb0bc774cb6 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2919,6 +2919,12 @@ extern int vfs_lstat(const char __user *, struct kstat *);
 extern int vfs_fstat(unsigned int, struct kstat *);
 extern int vfs_fstatat(int , const char __user *, struct kstat *, int);
 
+static inline int vfs_readlink(struct dentry *dentry, char __user *buffer,
+			       int buflen)
+{
+	return generic_readlink(dentry, buffer, buflen);
+}
+
 extern int __generic_block_fiemap(struct inode *inode,
 				  struct fiemap_extent_info *fieinfo,
 				  loff_t start, loff_t len,
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 12/17] vfs: remove ".readlink = generic_readlink" assignments
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (10 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 11/17] vfs: replace calling i_op->readlink with vfs_readlink() Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 13/17] vfs: remove unused i_op->readlink Miklos Szeredi
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

i_op->readlink is never called now, we can get rid of it.  This is the
first part, getting rid of the assignments.

Generated by:

to_del="\.readlink.*=.*generic_readlink"
for i in `git grep -l $to_del`; do sed -i "/$to_del"/d $i; done

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 drivers/staging/lustre/lustre/llite/symlink.c | 1 -
 fs/9p/vfs_inode.c                             | 1 -
 fs/9p/vfs_inode_dotl.c                        | 1 -
 fs/affs/symlink.c                             | 1 -
 fs/afs/mntpt.c                                | 1 -
 fs/autofs4/symlink.c                          | 1 -
 fs/bad_inode.c                                | 1 -
 fs/btrfs/inode.c                              | 1 -
 fs/ceph/inode.c                               | 1 -
 fs/cifs/cifsfs.c                              | 1 -
 fs/coda/cnode.c                               | 1 -
 fs/configfs/symlink.c                         | 1 -
 fs/ecryptfs/inode.c                           | 1 -
 fs/ext2/symlink.c                             | 2 --
 fs/ext4/symlink.c                             | 3 ---
 fs/f2fs/namei.c                               | 2 --
 fs/fuse/dir.c                                 | 1 -
 fs/gfs2/inode.c                               | 1 -
 fs/hostfs/hostfs_kern.c                       | 1 -
 fs/jffs2/symlink.c                            | 1 -
 fs/jfs/symlink.c                              | 2 --
 fs/kernfs/symlink.c                           | 1 -
 fs/libfs.c                                    | 1 -
 fs/minix/inode.c                              | 1 -
 fs/namei.c                                    | 1 -
 fs/ncpfs/inode.c                              | 1 -
 fs/nfs/symlink.c                              | 1 -
 fs/nilfs2/namei.c                             | 1 -
 fs/ocfs2/symlink.c                            | 1 -
 fs/orangefs/symlink.c                         | 1 -
 fs/overlayfs/inode.c                          | 1 -
 fs/proc/base.c                                | 2 --
 fs/proc/inode.c                               | 1 -
 fs/proc/namespaces.c                          | 1 -
 fs/proc/self.c                                | 1 -
 fs/proc/thread_self.c                         | 1 -
 fs/reiserfs/namei.c                           | 1 -
 fs/squashfs/symlink.c                         | 1 -
 fs/sysv/inode.c                               | 1 -
 fs/ubifs/file.c                               | 1 -
 fs/xfs/xfs_iops.c                             | 2 --
 mm/shmem.c                                    | 2 --
 42 files changed, 50 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/symlink.c b/drivers/staging/lustre/lustre/llite/symlink.c
index 8c8bdfe1ad71..48d37dce3229 100644
--- a/drivers/staging/lustre/lustre/llite/symlink.c
+++ b/drivers/staging/lustre/lustre/llite/symlink.c
@@ -150,7 +150,6 @@ static const char *ll_get_link(struct dentry *dentry,
 }
 
 const struct inode_operations ll_fast_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.setattr	= ll_setattr,
 	.get_link	= ll_get_link,
 	.getattr	= ll_getattr,
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 8b1999b528e9..a76d61bc809a 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -1460,7 +1460,6 @@ static const struct inode_operations v9fs_file_inode_operations = {
 };
 
 static const struct inode_operations v9fs_symlink_inode_operations = {
-	.readlink = generic_readlink,
 	.get_link = v9fs_vfs_get_link,
 	.getattr = v9fs_vfs_getattr,
 	.setattr = v9fs_vfs_setattr,
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index eeabcb0bad12..cee0e43f350b 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -985,7 +985,6 @@ const struct inode_operations v9fs_file_inode_operations_dotl = {
 };
 
 const struct inode_operations v9fs_symlink_inode_operations_dotl = {
-	.readlink = generic_readlink,
 	.get_link = v9fs_vfs_get_link_dotl,
 	.getattr = v9fs_vfs_getattr_dotl,
 	.setattr = v9fs_vfs_setattr_dotl,
diff --git a/fs/affs/symlink.c b/fs/affs/symlink.c
index 69b03dbb792f..ae622cdce142 100644
--- a/fs/affs/symlink.c
+++ b/fs/affs/symlink.c
@@ -70,7 +70,6 @@ const struct address_space_operations affs_symlink_aops = {
 };
 
 const struct inode_operations affs_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= page_get_link,
 	.setattr	= affs_notify_change,
 };
diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c
index 7d170de2ce1b..8211b5ecae94 100644
--- a/fs/afs/mntpt.c
+++ b/fs/afs/mntpt.c
@@ -34,7 +34,6 @@ const struct file_operations afs_mntpt_file_operations = {
 const struct inode_operations afs_mntpt_inode_operations = {
 	.lookup		= afs_mntpt_lookup,
 	.get_link	= page_get_link,
-	.readlink	= generic_readlink,
 	.getattr	= afs_getattr,
 };
 
diff --git a/fs/autofs4/symlink.c b/fs/autofs4/symlink.c
index 99aab00dc217..ab0b4285a202 100644
--- a/fs/autofs4/symlink.c
+++ b/fs/autofs4/symlink.c
@@ -25,6 +25,5 @@ static const char *autofs4_get_link(struct dentry *dentry,
 }
 
 const struct inode_operations autofs4_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= autofs4_get_link
 };
diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index 7cf1f445fe04..9475e9b4a85c 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -172,7 +172,6 @@ static const struct inode_operations bad_inode_ops =
 	.rmdir		= bad_inode_rmdir,
 	.mknod		= bad_inode_mknod,
 	.rename2	= bad_inode_rename2,
-	.readlink	= generic_readlink,
 	.permission	= bad_inode_permission,
 	.getattr	= bad_inode_getattr,
 	.setattr	= bad_inode_setattr,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index e6811c42e41e..fe1f416a4cad 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -10670,7 +10670,6 @@ static const struct inode_operations btrfs_special_inode_operations = {
 	.update_time	= btrfs_update_time,
 };
 static const struct inode_operations btrfs_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= page_get_link,
 	.getattr	= btrfs_getattr,
 	.setattr	= btrfs_setattr,
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index dd3a6dbf71eb..76482fa1a500 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -1881,7 +1881,6 @@ retry:
  * symlinks
  */
 static const struct inode_operations ceph_symlink_iops = {
-	.readlink = generic_readlink,
 	.get_link = simple_get_link,
 	.setattr = ceph_setattr,
 	.getattr = ceph_getattr,
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 6bbec5e784cd..cb164ef84335 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -923,7 +923,6 @@ const struct inode_operations cifs_file_inode_ops = {
 };
 
 const struct inode_operations cifs_symlink_inode_ops = {
-	.readlink = generic_readlink,
 	.get_link = cifs_get_link,
 	.permission = cifs_permission,
 	.setxattr = generic_setxattr,
diff --git a/fs/coda/cnode.c b/fs/coda/cnode.c
index 1bfb7ba4e85e..f13e09057c6b 100644
--- a/fs/coda/cnode.c
+++ b/fs/coda/cnode.c
@@ -17,7 +17,6 @@ static inline int coda_fideq(struct CodaFid *fid1, struct CodaFid *fid2)
 }
 
 static const struct inode_operations coda_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= page_get_link,
 	.setattr	= coda_setattr,
 };
diff --git a/fs/configfs/symlink.c b/fs/configfs/symlink.c
index db6d69289608..a6ab012a2c6a 100644
--- a/fs/configfs/symlink.c
+++ b/fs/configfs/symlink.c
@@ -305,7 +305,6 @@ static const char *configfs_get_link(struct dentry *dentry,
 
 const struct inode_operations configfs_symlink_inode_operations = {
 	.get_link = configfs_get_link,
-	.readlink = generic_readlink,
 	.setattr = configfs_setattr,
 };
 
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 9885a8f88260..fb2d831b7030 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -1082,7 +1082,6 @@ out:
 }
 
 const struct inode_operations ecryptfs_symlink_iops = {
-	.readlink = generic_readlink,
 	.get_link = ecryptfs_get_link,
 	.permission = ecryptfs_permission,
 	.setattr = ecryptfs_setattr,
diff --git a/fs/ext2/symlink.c b/fs/ext2/symlink.c
index 3495d8ae4b33..b77bb8198bb9 100644
--- a/fs/ext2/symlink.c
+++ b/fs/ext2/symlink.c
@@ -21,7 +21,6 @@
 #include "xattr.h"
 
 const struct inode_operations ext2_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= page_get_link,
 	.setattr	= ext2_setattr,
 #ifdef CONFIG_EXT2_FS_XATTR
@@ -33,7 +32,6 @@ const struct inode_operations ext2_symlink_inode_operations = {
 };
  
 const struct inode_operations ext2_fast_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= simple_get_link,
 	.setattr	= ext2_setattr,
 #ifdef CONFIG_EXT2_FS_XATTR
diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c
index 4d83d9e05f2e..96ab1e69e8ad 100644
--- a/fs/ext4/symlink.c
+++ b/fs/ext4/symlink.c
@@ -87,7 +87,6 @@ errout:
 }
 
 const struct inode_operations ext4_encrypted_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= ext4_encrypted_get_link,
 	.setattr	= ext4_setattr,
 	.setxattr	= generic_setxattr,
@@ -97,7 +96,6 @@ const struct inode_operations ext4_encrypted_symlink_inode_operations = {
 };
 
 const struct inode_operations ext4_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= page_get_link,
 	.setattr	= ext4_setattr,
 	.setxattr	= generic_setxattr,
@@ -107,7 +105,6 @@ const struct inode_operations ext4_symlink_inode_operations = {
 };
 
 const struct inode_operations ext4_fast_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= simple_get_link,
 	.setattr	= ext4_setattr,
 	.setxattr	= generic_setxattr,
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 73fa356f8fbb..c19de60dee5d 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -1072,7 +1072,6 @@ errout:
 }
 
 const struct inode_operations f2fs_encrypted_symlink_inode_operations = {
-	.readlink       = generic_readlink,
 	.get_link       = f2fs_encrypted_get_link,
 	.getattr	= f2fs_getattr,
 	.setattr	= f2fs_setattr,
@@ -1108,7 +1107,6 @@ const struct inode_operations f2fs_dir_inode_operations = {
 };
 
 const struct inode_operations f2fs_symlink_inode_operations = {
-	.readlink       = generic_readlink,
 	.get_link       = f2fs_get_link,
 	.getattr	= f2fs_getattr,
 	.setattr	= f2fs_setattr,
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index c47b7780ce37..5e18012fd662 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1914,7 +1914,6 @@ static const struct inode_operations fuse_common_inode_operations = {
 static const struct inode_operations fuse_symlink_inode_operations = {
 	.setattr	= fuse_setattr,
 	.get_link	= fuse_get_link,
-	.readlink	= generic_readlink,
 	.getattr	= fuse_getattr,
 	.setxattr	= fuse_setxattr,
 	.getxattr	= fuse_getxattr,
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index e4da0ecd3285..ff48245848e7 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2069,7 +2069,6 @@ const struct inode_operations gfs2_dir_iops = {
 };
 
 const struct inode_operations gfs2_symlink_iops = {
-	.readlink = generic_readlink,
 	.get_link = gfs2_get_link,
 	.permission = gfs2_permission,
 	.setattr = gfs2_setattr,
diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
index 90e46cd752fe..10a512450575 100644
--- a/fs/hostfs/hostfs_kern.c
+++ b/fs/hostfs/hostfs_kern.c
@@ -920,7 +920,6 @@ static const char *hostfs_get_link(struct dentry *dentry,
 }
 
 static const struct inode_operations hostfs_link_iops = {
-	.readlink	= generic_readlink,
 	.get_link	= hostfs_get_link,
 };
 
diff --git a/fs/jffs2/symlink.c b/fs/jffs2/symlink.c
index 2cabd649d4fb..18da3700e848 100644
--- a/fs/jffs2/symlink.c
+++ b/fs/jffs2/symlink.c
@@ -13,7 +13,6 @@
 
 const struct inode_operations jffs2_symlink_inode_operations =
 {
-	.readlink =	generic_readlink,
 	.get_link =	simple_get_link,
 	.setattr =	jffs2_setattr,
 	.setxattr =	jffs2_setxattr,
diff --git a/fs/jfs/symlink.c b/fs/jfs/symlink.c
index c94c7e4a1323..a9e3c856cc0c 100644
--- a/fs/jfs/symlink.c
+++ b/fs/jfs/symlink.c
@@ -22,7 +22,6 @@
 #include "jfs_xattr.h"
 
 const struct inode_operations jfs_fast_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= simple_get_link,
 	.setattr	= jfs_setattr,
 	.setxattr	= generic_setxattr,
@@ -32,7 +31,6 @@ const struct inode_operations jfs_fast_symlink_inode_operations = {
 };
 
 const struct inode_operations jfs_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= page_get_link,
 	.setattr	= jfs_setattr,
 	.setxattr	= generic_setxattr,
diff --git a/fs/kernfs/symlink.c b/fs/kernfs/symlink.c
index 117b8b3416f9..e4a4aee2d74f 100644
--- a/fs/kernfs/symlink.c
+++ b/fs/kernfs/symlink.c
@@ -138,7 +138,6 @@ const struct inode_operations kernfs_symlink_iops = {
 	.removexattr	= kernfs_iop_removexattr,
 	.getxattr	= kernfs_iop_getxattr,
 	.listxattr	= kernfs_iop_listxattr,
-	.readlink	= generic_readlink,
 	.get_link	= kernfs_iop_get_link,
 	.setattr	= kernfs_iop_setattr,
 	.getattr	= kernfs_iop_getattr,
diff --git a/fs/libfs.c b/fs/libfs.c
index 74dc8b9e7f53..7a0844e01e3f 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1124,7 +1124,6 @@ EXPORT_SYMBOL(simple_get_link);
 
 const struct inode_operations simple_symlink_inode_operations = {
 	.get_link = simple_get_link,
-	.readlink = generic_readlink
 };
 EXPORT_SYMBOL(simple_symlink_inode_operations);
 
diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index f975d667c539..e7d9bf86d975 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -434,7 +434,6 @@ static const struct address_space_operations minix_aops = {
 };
 
 static const struct inode_operations minix_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= page_get_link,
 	.getattr	= minix_getattr,
 };
diff --git a/fs/namei.c b/fs/namei.c
index f72c405d1a27..a93424178634 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4765,7 +4765,6 @@ int page_symlink(struct inode *inode, const char *symname, int len)
 EXPORT_SYMBOL(page_symlink);
 
 const struct inode_operations page_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= page_get_link,
 };
 EXPORT_SYMBOL(page_symlink_inode_operations);
diff --git a/fs/ncpfs/inode.c b/fs/ncpfs/inode.c
index 1af15fcbe57b..86717e51cf1d 100644
--- a/fs/ncpfs/inode.c
+++ b/fs/ncpfs/inode.c
@@ -243,7 +243,6 @@ static void ncp_set_attr(struct inode *inode, struct ncp_entry_info *nwinfo)
 
 #if defined(CONFIG_NCPFS_EXTRAS) || defined(CONFIG_NCPFS_NFS_NS)
 static const struct inode_operations ncp_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= page_get_link,
 	.setattr	= ncp_notify_change,
 };
diff --git a/fs/nfs/symlink.c b/fs/nfs/symlink.c
index 4fe3eead3868..5a1d0ded8979 100644
--- a/fs/nfs/symlink.c
+++ b/fs/nfs/symlink.c
@@ -77,7 +77,6 @@ static const char *nfs_get_link(struct dentry *dentry,
  * symlinks can't do much...
  */
 const struct inode_operations nfs_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= nfs_get_link,
 	.getattr	= nfs_getattr,
 	.setattr	= nfs_setattr,
diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
index dbcf1dc93a51..c7eb03be9723 100644
--- a/fs/nilfs2/namei.c
+++ b/fs/nilfs2/namei.c
@@ -564,7 +564,6 @@ const struct inode_operations nilfs_special_inode_operations = {
 };
 
 const struct inode_operations nilfs_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= page_get_link,
 	.permission     = nilfs_permission,
 };
diff --git a/fs/ocfs2/symlink.c b/fs/ocfs2/symlink.c
index 6c2a3e3c521c..1a2ba465e291 100644
--- a/fs/ocfs2/symlink.c
+++ b/fs/ocfs2/symlink.c
@@ -87,7 +87,6 @@ const struct address_space_operations ocfs2_fast_symlink_aops = {
 };
 
 const struct inode_operations ocfs2_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= page_get_link,
 	.getattr	= ocfs2_getattr,
 	.setattr	= ocfs2_setattr,
diff --git a/fs/orangefs/symlink.c b/fs/orangefs/symlink.c
index 8fecf823f5ba..20c8b4193cf3 100644
--- a/fs/orangefs/symlink.c
+++ b/fs/orangefs/symlink.c
@@ -9,7 +9,6 @@
 #include "orangefs-bufmap.h"
 
 const struct inode_operations orangefs_symlink_inode_operations = {
-	.readlink = generic_readlink,
 	.get_link = simple_get_link,
 	.setattr = orangefs_setattr,
 	.getattr = orangefs_getattr,
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 3b636e3acda4..db8975f59021 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -359,7 +359,6 @@ static const struct inode_operations ovl_file_inode_operations = {
 static const struct inode_operations ovl_symlink_inode_operations = {
 	.setattr	= ovl_setattr,
 	.get_link	= ovl_get_link,
-	.readlink	= generic_readlink,
 	.getattr	= ovl_getattr,
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 84769c763afe..efd87ce587e6 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1615,7 +1615,6 @@ out:
 
 
 const struct inode_operations proc_pid_link_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= proc_pid_get_link,
 	.setattr	= proc_setattr,
 };
@@ -1950,7 +1949,6 @@ proc_map_files_get_link(struct dentry *dentry,
  * Identical to proc_pid_link_inode_operations except for get_link()
  */
 static const struct inode_operations proc_map_files_link_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= proc_map_files_get_link,
 	.setattr	= proc_setattr,
 };
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index c1b72388e571..43b228b8cb21 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -411,7 +411,6 @@ static const char *proc_get_link(struct dentry *dentry,
 }
 
 const struct inode_operations proc_link_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= proc_get_link,
 };
 
diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c
index 9c7209734b05..ea5f1fac3987 100644
--- a/fs/proc/namespaces.c
+++ b/fs/proc/namespaces.c
@@ -68,7 +68,6 @@ out:
 }
 
 static const struct inode_operations proc_ns_link_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= proc_ns_get_link,
 	.setattr	= proc_setattr,
 };
diff --git a/fs/proc/self.c b/fs/proc/self.c
index 80bc52fd7846..3ce34845a174 100644
--- a/fs/proc/self.c
+++ b/fs/proc/self.c
@@ -26,7 +26,6 @@ static const char *proc_self_get_link(struct dentry *dentry,
 }
 
 static const struct inode_operations proc_self_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= proc_self_get_link,
 };
 
diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
index 55053d5280ff..75bbdd7f7c69 100644
--- a/fs/proc/thread_self.c
+++ b/fs/proc/thread_self.c
@@ -27,7 +27,6 @@ static const char *proc_thread_self_get_link(struct dentry *dentry,
 }
 
 static const struct inode_operations proc_thread_self_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= proc_thread_self_get_link,
 };
 
diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index 8a36696d6df9..adbf154ef4a2 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -1664,7 +1664,6 @@ const struct inode_operations reiserfs_dir_inode_operations = {
  * stuff added
  */
 const struct inode_operations reiserfs_symlink_inode_operations = {
-	.readlink = generic_readlink,
 	.get_link	= page_get_link,
 	.setattr = reiserfs_setattr,
 	.setxattr = generic_setxattr,
diff --git a/fs/squashfs/symlink.c b/fs/squashfs/symlink.c
index d688ef42a6a1..853b8516ba44 100644
--- a/fs/squashfs/symlink.c
+++ b/fs/squashfs/symlink.c
@@ -118,7 +118,6 @@ const struct address_space_operations squashfs_symlink_aops = {
 };
 
 const struct inode_operations squashfs_symlink_inode_ops = {
-	.readlink = generic_readlink,
 	.get_link = page_get_link,
 	.getxattr = generic_getxattr,
 	.listxattr = squashfs_listxattr
diff --git a/fs/sysv/inode.c b/fs/sysv/inode.c
index d62c423a5a2d..858fb72f9e0f 100644
--- a/fs/sysv/inode.c
+++ b/fs/sysv/inode.c
@@ -145,7 +145,6 @@ static inline void write3byte(struct sysv_sb_info *sbi,
 }
 
 static const struct inode_operations sysv_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= page_get_link,
 	.getattr	= sysv_getattr,
 };
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 7bbf420d1289..62f69c392e17 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1631,7 +1631,6 @@ const struct inode_operations ubifs_file_inode_operations = {
 };
 
 const struct inode_operations ubifs_symlink_inode_operations = {
-	.readlink    = generic_readlink,
 	.get_link    = simple_get_link,
 	.setattr     = ubifs_setattr,
 	.getattr     = ubifs_getattr,
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index b24c3102fa93..b679efa7ae09 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1101,7 +1101,6 @@ static const struct inode_operations xfs_dir_ci_inode_operations = {
 };
 
 static const struct inode_operations xfs_symlink_inode_operations = {
-	.readlink		= generic_readlink,
 	.get_link		= xfs_vn_get_link,
 	.getattr		= xfs_vn_getattr,
 	.setattr		= xfs_vn_setattr,
@@ -1113,7 +1112,6 @@ static const struct inode_operations xfs_symlink_inode_operations = {
 };
 
 static const struct inode_operations xfs_inline_symlink_inode_operations = {
-	.readlink		= generic_readlink,
 	.get_link		= xfs_vn_get_link_inline,
 	.getattr		= xfs_vn_getattr,
 	.setattr		= xfs_vn_setattr,
diff --git a/mm/shmem.c b/mm/shmem.c
index fd8b2b5741b1..68162abd85e6 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3284,7 +3284,6 @@ static ssize_t shmem_listxattr(struct dentry *dentry, char *buffer, size_t size)
 #endif /* CONFIG_TMPFS_XATTR */
 
 static const struct inode_operations shmem_short_symlink_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= simple_get_link,
 #ifdef CONFIG_TMPFS_XATTR
 	.setxattr	= generic_setxattr,
@@ -3295,7 +3294,6 @@ static const struct inode_operations shmem_short_symlink_operations = {
 };
 
 static const struct inode_operations shmem_symlink_inode_operations = {
-	.readlink	= generic_readlink,
 	.get_link	= shmem_get_link,
 #ifdef CONFIG_TMPFS_XATTR
 	.setxattr	= generic_setxattr,
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 13/17] vfs: remove unused i_op->readlink
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (11 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 12/17] vfs: remove ".readlink = generic_readlink" assignments Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 14/17] vfs: remove unused generic_readlink() Miklos Szeredi
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

This removes the iop->readlink method, which is now completely unused.

Documentation is updated and a note added to "porting".

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 Documentation/filesystems/Locking |  2 --
 Documentation/filesystems/porting | 15 +++++++++++++++
 Documentation/filesystems/vfs.txt | 25 ++++++++++++-------------
 include/linux/fs.h                |  3 ---
 4 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index d30fb2cb5066..1aa8894ef551 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -54,7 +54,6 @@ prototypes:
 			struct inode *, struct dentry *);
 	int (*rename2) (struct inode *, struct dentry *,
 			struct inode *, struct dentry *, unsigned int);
-	int (*readlink) (struct dentry *, char __user *,int);
 	const char *(*get_link) (struct dentry *, struct inode *, void **);
 	void (*truncate) (struct inode *);
 	int (*permission) (struct inode *, int, unsigned int);
@@ -85,7 +84,6 @@ unlink:		yes (both)
 rmdir:		yes (both)	(see below)
 rename:		yes (all)	(see below)
 rename2:	yes (all)	(see below)
-readlink:	no
 get_link:	no
 setattr:	yes
 permission:	no (may not block if called in rcu-walk mode)
diff --git a/Documentation/filesystems/porting b/Documentation/filesystems/porting
index b1bd05ea66b2..dcd2b77a9f90 100644
--- a/Documentation/filesystems/porting
+++ b/Documentation/filesystems/porting
@@ -592,3 +592,18 @@ in your dentry operations instead.
 	work just as well; if it's something more complicated, use dentry->d_parent.
 	Just be careful not to assume that fetching it more than once will yield
 	the same value - in RCU mode it could change under you.
+--
+[mandatory]
+	->readlink() i_op is gone, all of its functionality taken over by
+	->get_link().  If the filesystem set .readlink to generic_readlink,
+	then it doesn't need to do anything besides removing this
+	assignment.
+
+	In the unlikely case that ->get_link() calls nd_jump_link(), then
+	it needs to do that conditionally, only if is_following_link()
+	returns true.  Otherwise it should return the string representation
+	of the symlink, as the old ->readlink() implementation did.
+
+	If the filesystem set .readlink but not .get_link, then it needs to
+	port the readlink implementation to the get_link interface and set
+	IOP_NOFOLLOW in inode->i_opflags to prevent following the symlink.
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 9ace359d6cc5..3914dc728151 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -349,7 +349,6 @@ struct inode_operations {
 			struct inode *, struct dentry *);
 	int (*rename2) (struct inode *, struct dentry *,
 			struct inode *, struct dentry *, unsigned int);
-	int (*readlink) (struct dentry *, char __user *,int);
 	const char *(*get_link) (struct dentry *, struct inode *,
 				 struct delayed_call *);
 	int (*permission) (struct inode *, int);
@@ -430,19 +429,19 @@ otherwise noted.
 	exist; this is checked by the VFS.  Unlike plain rename,
 	source and target may be of different type.
 
-  readlink: called by the readlink(2) system call. Only required if
-	you want to support reading symbolic links
-
-  get_link: called by the VFS to follow a symbolic link to the
+  get_link: called by the VFS to read or follow a symbolic link to the
 	inode it points to.  Only required if you want to support
-	symbolic links.  This method returns the symlink body
-	to traverse (and possibly resets the current position with
-	nd_jump_link()).  If the body won't go away until the inode
-	is gone, nothing else is needed; if it needs to be otherwise
-	pinned, arrange for its release by having get_link(..., ..., done)
-	do set_delayed_call(done, destructor, argument).
-	In that case destructor(argument) will be called once VFS is
-	done with the body you've returned.
+	symbolic links.  This method returns the symlink body (and if
+	is_following_link() is true, then possibly resets the current
+	position with nd_jump_link()).
+
+	If the body won't go away until the inode is gone, nothing else
+	is needed; if it needs to be otherwise pinned, arrange for its
+	release by having get_link(..., ..., done) do
+	set_delayed_call(done, destructor, argument).  In that case
+	destructor(argument) will be called once VFS is done with the
+	body you've returned.
+
 	May be called in RCU mode; that is indicated by NULL dentry
 	argument.  If request can't be handled without leaving RCU mode,
 	have it return ERR_PTR(-ECHILD).
diff --git a/include/linux/fs.h b/include/linux/fs.h
index bcb0bc774cb6..31e940cb7a4f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1722,9 +1722,6 @@ struct inode_operations {
 	const char * (*get_link) (struct dentry *, struct inode *, struct delayed_call *);
 	int (*permission) (struct inode *, int);
 	struct posix_acl * (*get_acl)(struct inode *, int);
-
-	int (*readlink) (struct dentry *, char __user *,int);
-
 	int (*create) (struct inode *,struct dentry *, umode_t, bool);
 	int (*link) (struct dentry *,struct inode *,struct dentry *);
 	int (*unlink) (struct inode *,struct dentry *);
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 14/17] vfs: remove unused generic_readlink()
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (12 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 13/17] vfs: remove unused i_op->readlink Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 15/17] vfs: add vfs_get_link() helper Miklos Szeredi
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

Which is now known as vfs_readlink().

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/namei.c         | 22 ++++++++++++++++------
 include/linux/fs.h |  7 +------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index a93424178634..76d1f061de3c 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4659,12 +4659,22 @@ out:
 	return len;
 }
 
-/*
- * A helper for ->readlink().  This should be used *ONLY* for symlinks that
- * have ->get_link() not calling nd_jump_link().  Using (or not using) it
- * for any given inode is up to filesystem.
+/**
+ * vfs_readlink - read symlink body
+ * @dentry: read symlink from this dentry
+ * @buffer: userspace buffer
+ * @buflen: size of userspace buffer
+ *
+ * Copy contents of symlink to userspace buffer.  If the buffer is smaller than
+ * the length of the symlink, then the symlink is truncated.  On success the
+ * function returns the number of bytes actually copied to the buffer.  The
+ * terminating null character is never copied.
+ *
+ * The inode's access time is not updated.  That's up to the caller if
+ * necessary.  The caller also needs to make sure that i_op->get_link is
+ * non-NULL before calling this helper.
  */
-int generic_readlink(struct dentry *dentry, char __user *buffer, int buflen)
+int vfs_readlink(struct dentry *dentry, char __user *buffer, int buflen)
 {
 	DEFINE_DELAYED_CALL(done);
 	struct inode *inode = d_inode(dentry);
@@ -4684,7 +4694,7 @@ int generic_readlink(struct dentry *dentry, char __user *buffer, int buflen)
 	do_delayed_call(&done);
 	return res;
 }
-EXPORT_SYMBOL(generic_readlink);
+EXPORT_SYMBOL(vfs_readlink);
 
 /* get the link contents into pagecache */
 const char *page_get_link(struct dentry *dentry, struct inode *inode,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 31e940cb7a4f..f71e70e3017b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2895,7 +2895,6 @@ extern int __page_symlink(struct inode *inode, const char *symname, int len,
 extern int page_symlink(struct inode *inode, const char *symname, int len);
 extern const struct inode_operations page_symlink_inode_operations;
 extern void kfree_link(void *);
-extern int generic_readlink(struct dentry *, char __user *, int);
 extern void generic_fillattr(struct inode *, struct kstat *);
 int vfs_getattr_nosec(struct path *path, struct kstat *stat);
 extern int vfs_getattr(struct path *, struct kstat *);
@@ -2916,11 +2915,7 @@ extern int vfs_lstat(const char __user *, struct kstat *);
 extern int vfs_fstat(unsigned int, struct kstat *);
 extern int vfs_fstatat(int , const char __user *, struct kstat *, int);
 
-static inline int vfs_readlink(struct dentry *dentry, char __user *buffer,
-			       int buflen)
-{
-	return generic_readlink(dentry, buffer, buflen);
-}
+extern int vfs_readlink(struct dentry *, char __user *, int);
 
 extern int __generic_block_fiemap(struct inode *inode,
 				  struct fiemap_extent_info *fieinfo,
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 15/17] vfs: add vfs_get_link() helper
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (13 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 14/17] vfs: remove unused generic_readlink() Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 16/17] ovl: use vfs_get_link() Miklos Szeredi
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

This helper is for filesystems that want to read the symlink and are better
off with the get_link() interface (returning a char *) rather than the
readlink() interface (copy into a userspace buffer).

Also call the LSM hook for readlink (not get_link) since this is for
symlink reading not following.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/namei.c         | 47 ++++++++++++++++++++++++++++++++++++++++++-----
 include/linux/fs.h |  2 ++
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 76d1f061de3c..23c3fb8cec0a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4659,6 +4659,47 @@ out:
 	return len;
 }
 
+static const char *do_get_link(struct dentry *dentry, struct inode *inode,
+			       struct delayed_call *done)
+{
+	const char *link = inode->i_op->get_link(dentry, inode, done);
+
+	/* "jumping" is unacceptable, warn and return error */
+	if (!IS_ERR(link) && WARN_ON_ONCE(!link))
+		link = ERR_PTR(-EIO);
+
+	return link;
+}
+
+/**
+ * vfs_get_link - get symbolic link
+ * @dentry: dentry
+ * @inode: inode on which to get symbolic link
+ * @done: caller needs to free returned data with this
+ *
+ * Calls security hook and i_op->get_link() on the supplied inode.
+ *
+ * It does not touch atime.  That's up to the caller if necessary.
+ *
+ * Don't call this from RCU lookup mode (yet)
+ */
+const char *vfs_get_link(struct dentry *dentry, struct inode *inode,
+			 struct delayed_call *done)
+{
+	const char *res = ERR_PTR(-EINVAL);
+
+	if (inode->i_op->get_link) {
+		int error = security_inode_readlink(dentry);
+
+		res = ERR_PTR(error);
+		if (!error)
+			res = do_get_link(dentry, inode, done);
+	}
+
+	return res;
+}
+EXPORT_SYMBOL(vfs_get_link);
+
 /**
  * vfs_readlink - read symlink body
  * @dentry: read symlink from this dentry
@@ -4682,13 +4723,9 @@ int vfs_readlink(struct dentry *dentry, char __user *buffer, int buflen)
 	int res;
 
 	if (!link) {
-		link = inode->i_op->get_link(dentry, inode, &done);
+		link = do_get_link(dentry, inode, &done);
 		if (IS_ERR(link))
 			return PTR_ERR(link);
-
-		/* "jumping" is unacceptable, warn and return error */
-		if (WARN_ON_ONCE(!link))
-			return -EIO;
 	}
 	res = readlink_copy(buffer, buflen, link);
 	do_delayed_call(&done);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f71e70e3017b..58f42e979b12 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2916,6 +2916,8 @@ extern int vfs_fstat(unsigned int, struct kstat *);
 extern int vfs_fstatat(int , const char __user *, struct kstat *, int);
 
 extern int vfs_readlink(struct dentry *, char __user *, int);
+extern const char *vfs_get_link(struct dentry *, struct inode *,
+				struct delayed_call *);
 
 extern int __generic_block_fiemap(struct inode *inode,
 				  struct fiemap_extent_info *fieinfo,
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 16/17] ovl: use vfs_get_link()
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (14 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 15/17] vfs: add vfs_get_link() helper Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-12 19:29 ` [PATCH 17/17] ecryptfs: " Miklos Szeredi
  2016-09-27  3:10 ` [PATCH 00/17] clean up readlinks Al Viro
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro

Resulting in a complete removal of a function basically implementing the
inverse of vfs_readlink().

As a bonus, now the proper security hook is also called.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/overlayfs/copy_up.c | 45 ++++++---------------------------------------
 fs/overlayfs/inode.c   |  5 +----
 2 files changed, 7 insertions(+), 43 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index a2d4c10dd74d..c3371d62d931 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -167,39 +167,6 @@ out_fput:
 	return error;
 }
 
-static char *ovl_read_symlink(struct dentry *realdentry)
-{
-	int res;
-	char *buf;
-	struct inode *inode = realdentry->d_inode;
-	mm_segment_t old_fs;
-
-	res = -EINVAL;
-	if (!inode->i_op->get_link)
-		goto err;
-
-	res = -ENOMEM;
-	buf = (char *) __get_free_page(GFP_KERNEL);
-	if (!buf)
-		goto err;
-
-	old_fs = get_fs();
-	set_fs(get_ds());
-	/* The cast to a user pointer is valid due to the set_fs() */
-	res = vfs_readlink(realdentry, (char __user *)buf, PAGE_SIZE - 1);
-	set_fs(old_fs);
-	if (res < 0) {
-		free_page((unsigned long) buf);
-		goto err;
-	}
-	buf[res] = '\0';
-
-	return buf;
-
-err:
-	return ERR_PTR(res);
-}
-
 static int ovl_set_timestamps(struct dentry *upperdentry, struct kstat *stat)
 {
 	struct iattr attr = {
@@ -331,19 +298,21 @@ out_cleanup:
 int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
 		    struct path *lowerpath, struct kstat *stat)
 {
+	DEFINE_DELAYED_CALL(done);
 	struct dentry *workdir = ovl_workdir(dentry);
 	int err;
 	struct kstat pstat;
 	struct path parentpath;
+	struct dentry *lowerdentry = lowerpath->dentry;
 	struct dentry *upperdir;
 	struct dentry *upperdentry;
 	const struct cred *old_cred;
-	char *link = NULL;
+	const char *link = NULL;
 
 	if (WARN_ON(!workdir))
 		return -EROFS;
 
-	ovl_do_check_copy_up(lowerpath->dentry);
+	ovl_do_check_copy_up(lowerdentry);
 
 	ovl_path_upper(parent, &parentpath);
 	upperdir = parentpath.dentry;
@@ -353,7 +322,7 @@ int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
 		return err;
 
 	if (S_ISLNK(stat->mode)) {
-		link = ovl_read_symlink(lowerpath->dentry);
+		link = vfs_get_link(lowerdentry, d_inode(lowerdentry), &done);
 		if (IS_ERR(link))
 			return PTR_ERR(link);
 	}
@@ -381,9 +350,7 @@ int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
 out_unlock:
 	unlock_rename(workdir, upperdir);
 	revert_creds(old_cred);
-
-	if (link)
-		free_page((unsigned long) link);
+	do_delayed_call(&done);
 
 	return err;
 }
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index db8975f59021..6be9bb470fd9 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -164,11 +164,8 @@ static const char *ovl_get_link(struct dentry *dentry,
 	realdentry = ovl_dentry_real(dentry);
 	realinode = realdentry->d_inode;
 
-	if (WARN_ON(!realinode->i_op->get_link))
-		return ERR_PTR(-EPERM);
-
 	old_cred = ovl_override_creds(dentry->d_sb);
-	p = realinode->i_op->get_link(realdentry, realinode, done);
+	p = vfs_get_link(realdentry, realinode, done);
 	revert_creds(old_cred);
 	return p;
 }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 17/17] ecryptfs: use vfs_get_link()
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (15 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 16/17] ovl: use vfs_get_link() Miklos Szeredi
@ 2016-09-12 19:29 ` Miklos Szeredi
  2016-09-27  3:10 ` [PATCH 00/17] clean up readlinks Al Viro
  17 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-12 19:29 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel, Al Viro, Tyler Hicks

Here again we are copying form one buffer to another, while jumping through
hoops to make kernel memory look like userspace memory.

For no good reason, since vfs_get_link() provides exactly what is needed.

As a bonus, now the security hook for readlink is also called on the
underlying inode.

Note: this can be called from link-following context.  But this is okay:

 - not in RCU mode

 - e54ad7f1ee26 ("proc: prevent stacking filesystems on top")

 - ecryptfs is *reading* the underlying symlink not following it, so the
   right security hook is being called

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Tyler Hicks <tyhicks@canonical.com>
---
 fs/ecryptfs/inode.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index fb2d831b7030..95e51b2af2de 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -627,26 +627,23 @@ out_lock:
 
 static char *ecryptfs_readlink_lower(struct dentry *dentry, size_t *bufsiz)
 {
+	DEFINE_DELAYED_CALL(done);
 	struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
-	char *lower_buf;
+	const char *link;
 	char *buf;
-	mm_segment_t old_fs;
 	int rc;
 
-	lower_buf = kmalloc(PATH_MAX, GFP_KERNEL);
-	if (!lower_buf)
-		return ERR_PTR(-ENOMEM);
-	old_fs = get_fs();
-	set_fs(get_ds());
-	rc = vfs_readlink(lower_dentry, (char __user *)lower_buf, PATH_MAX);
-	set_fs(old_fs);
-	if (rc < 0)
-		goto out;
+	link = vfs_get_link(lower_dentry, d_inode(lower_dentry), &done);
+	if (IS_ERR(link))
+		return ERR_CAST(link);
+
 	rc = ecryptfs_decode_and_decrypt_filename(&buf, bufsiz, dentry->d_sb,
-						  lower_buf, rc);
-out:
-	kfree(lower_buf);
-	return rc ? ERR_PTR(rc) : buf;
+						  link, strlen(link));
+	do_delayed_call(&done);
+	if (rc)
+		return ERR_PTR(rc);
+
+	return buf;
 }
 
 static const char *ecryptfs_get_link(struct dentry *dentry,
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 00/17] clean up readlinks
  2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
                   ` (16 preceding siblings ...)
  2016-09-12 19:29 ` [PATCH 17/17] ecryptfs: " Miklos Szeredi
@ 2016-09-27  3:10 ` Al Viro
  2016-09-27  9:38   ` Miklos Szeredi
  17 siblings, 1 reply; 22+ messages in thread
From: Al Viro @ 2016-09-27  3:10 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel, David Howells, Tyler Hicks

On Mon, Sep 12, 2016 at 09:29:02PM +0200, Miklos Szeredi wrote:
> The first patch is actually a bug fix, but I put it into this bunch for
> simplicity...
> 
> The rest are really cleanups as well as minor bugfixes that are byproducts
> of the cleanups.
> 
> This series builds on the fact that i_op.readlink is already set to
> generic_readlink() in 43/50 of the cases.  And of those 7 only 4 are doing
> something special.  So more than 90% of readlinks are/could actually just
> call back into get_link.
> 
> The interesting cases are:
> 
>  - AFS, which has readlink but not get_link
>  - proc, that allow jumping while following symlinks
> 
> The first is handled by setting IOP_NOFOLLOW on the inode by the fs.
> 
> The second one is handled by introducing is_following_link() which returns
> a bool depending on whether current->nameidata is NULL or not.  If it
> returns false ->get_link() should behave as ->readlink() did.  Otherwise it
> should behave as id did previously.
> 
> Builds and boots.  Can even read symlinks.

	I have no problem with "let's get rid of generic_readlink" - not that
it bought us much, but sure, if you want to have decision made based upon
the combination of flags, let's do it.  Just make NULL ->readlink + non-NULL
->get_link() mean generic_readlink(), and we are done.  Especially if you
do the usual "set the flag on inode the first time we need to check".
I also have no problem with overlayfs and ecryptfs assuming that we only deal
with normal symlinks.

	Overloading ->get_link() for procfs-style ones is just plain wrong,
though.  Your current->nameidata != NULL thing is bloody brittle - what
happens if some code triggers those readlinks when called by something
during pathname resolution?  Sure, right now existing callers won't.
But it doesn't take much to grow such a place _and_ have the implications
go unnoticed for quite a while.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 00/17] clean up readlinks
  2016-09-27  3:10 ` [PATCH 00/17] clean up readlinks Al Viro
@ 2016-09-27  9:38   ` Miklos Szeredi
  2016-09-28  2:17     ` Al Viro
  0 siblings, 1 reply; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-27  9:38 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, lkml, David Howells, Tyler Hicks

On Tue, Sep 27, 2016 at 5:10 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Mon, Sep 12, 2016 at 09:29:02PM +0200, Miklos Szeredi wrote:
>> The first patch is actually a bug fix, but I put it into this bunch for
>> simplicity...
>>
>> The rest are really cleanups as well as minor bugfixes that are byproducts
>> of the cleanups.
>>
>> This series builds on the fact that i_op.readlink is already set to
>> generic_readlink() in 43/50 of the cases.  And of those 7 only 4 are doing
>> something special.  So more than 90% of readlinks are/could actually just
>> call back into get_link.
>>
>> The interesting cases are:
>>
>>  - AFS, which has readlink but not get_link
>>  - proc, that allow jumping while following symlinks
>>
>> The first is handled by setting IOP_NOFOLLOW on the inode by the fs.
>>
>> The second one is handled by introducing is_following_link() which returns
>> a bool depending on whether current->nameidata is NULL or not.  If it
>> returns false ->get_link() should behave as ->readlink() did.  Otherwise it
>> should behave as id did previously.
>>
>> Builds and boots.  Can even read symlinks.
>
>         I have no problem with "let's get rid of generic_readlink" - not that
> it bought us much, but sure, if you want to have decision made based upon
> the combination of flags, let's do it.  Just make NULL ->readlink + non-NULL
> ->get_link() mean generic_readlink(), and we are done.

Indeed.  Except it really should be the other way round:

- .get_link always returning the symlink body
- only proc setting .jump_link to do its thing
- RIP .readlink

But that's an extra branch in the symlink following.  I was worried
about that and hence gone for the unification of the two.

>
>         Overloading ->get_link() for procfs-style ones is just plain wrong,
> though.  Your current->nameidata != NULL thing is bloody brittle - what
> happens if some code triggers those readlinks when called by something
> during pathname resolution?  Sure, right now existing callers won't.
> But it doesn't take much to grow such a place _and_ have the implications
> go unnoticed for quite a while.

Yeah.  We can do your above suggestion, it's certainly less brittle.
But I think it's rather confusing, having ->get_link normally do
readlink, except for proc, where readlink is done by ->readlink.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 00/17] clean up readlinks
  2016-09-27  9:38   ` Miklos Szeredi
@ 2016-09-28  2:17     ` Al Viro
  2016-09-28 14:47       ` Miklos Szeredi
  0 siblings, 1 reply; 22+ messages in thread
From: Al Viro @ 2016-09-28  2:17 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, lkml, David Howells, Tyler Hicks

On Tue, Sep 27, 2016 at 11:38:33AM +0200, Miklos Szeredi wrote:
> >         I have no problem with "let's get rid of generic_readlink" - not that
> > it bought us much, but sure, if you want to have decision made based upon
> > the combination of flags, let's do it.  Just make NULL ->readlink + non-NULL
> > ->get_link() mean generic_readlink(), and we are done.
> 
> Indeed.  Except it really should be the other way round:
> 
> - .get_link always returning the symlink body
> - only proc setting .jump_link to do its thing
> - RIP .readlink

> But that's an extra branch in the symlink following.  I was worried
> about that and hence gone for the unification of the two.

Symlink traversal is a much hotter path than readlink() would ever be.
What's more, we do have jumps on normal symlink traversal - after all,
absolute symlinks are exactly that; it's "jump to root, then traverse
the following sequence of components".  So having ->get_link() that
includes jumps is not that much of a stretch (note that it could both
jump and return a relative pathname to traverse after that; none of the
procfs-style ones do that, but there's no reason to prohibit that).

What I'd prefer is
	* it's a symlink iff it has ->get_link()
	* readlink(2) on a symlink is normally just using generic_readlink()
	* that can be overridden by supplying a ->readlink() method.
	* the first time readlink() hits a symlink it will check both
->get_link() and ->readlink() presence.  Then, if it's a normal symlink,
the inode will get marked as such and all subsequent calls will just call
generic_readlink().  IOW, I would go for
	if (unlikely(!marked)) {
		if ->readlink is present
			call ->readlink and return
		if ->get_link is absent
			fail
		mark
	}
	call generic_readlink

> Yeah.  We can do your above suggestion, it's certainly less brittle.
> But I think it's rather confusing, having ->get_link normally do
> readlink, except for proc, where readlink is done by ->readlink.

	->readlink() is just an override for the cases when readlink(2) wants
to fake something (or, as in case of AFS ugliness, is used on non-symlinks).
The primary function of symlinks is traversal, not readlink...

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 00/17] clean up readlinks
  2016-09-28  2:17     ` Al Viro
@ 2016-09-28 14:47       ` Miklos Szeredi
  0 siblings, 0 replies; 22+ messages in thread
From: Miklos Szeredi @ 2016-09-28 14:47 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, lkml, David Howells, Tyler Hicks

On Wed, Sep 28, 2016 at 4:17 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:

> Symlink traversal is a much hotter path than readlink() would ever be.
> What's more, we do have jumps on normal symlink traversal - after all,
> absolute symlinks are exactly that; it's "jump to root, then traverse
> the following sequence of components".  So having ->get_link() that
> includes jumps is not that much of a stretch (note that it could both
> jump and return a relative pathname to traverse after that; none of the
> procfs-style ones do that, but there's no reason to prohibit that).
>
> What I'd prefer is
>         * it's a symlink iff it has ->get_link()
>         * readlink(2) on a symlink is normally just using generic_readlink()
>         * that can be overridden by supplying a ->readlink() method.
>         * the first time readlink() hits a symlink it will check both
> ->get_link() and ->readlink() presence.  Then, if it's a normal symlink,
> the inode will get marked as such and all subsequent calls will just call
> generic_readlink().  IOW, I would go for
>         if (unlikely(!marked)) {
>                 if ->readlink is present
>                         call ->readlink and return
>                 if ->get_link is absent
>                         fail
>                 mark
>         }
>         call generic_readlink

Redid patchset confirming to the above.

Also moved the overlayfs/ecryptfs fixes to the front of the queue, as
they are independent of the rest of the cleanups.

The last part is new: changing ->readlink signature to match that of
->get_link.  This moves the user buffer handling out of filesystems,
simplifying the implementation in the process.

Force pushed new set to:

  git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git #readlink

It's got a couple of trivial conflicts against #rename2 and
#overlayfs-next.  I can do the merge and send a big pull request if
you want.

Thanks,
Miklos

---
Miklos Szeredi (14):
      vfs: add vfs_get_link() helper
      ovl: use vfs_get_link()
      ecryptfs: use vfs_get_link()
      ovl: use generic_readlink
      proc/self: use generic_readlink
      bad_inode: use generic_readlink
      vfs: replace calling i_op->readlink with vfs_readlink()
      vfs: default to generic_readlink()
      vfs: remove ".readlink = generic_readlink" assignments
      vfs: make generic_readlink() static
      vfs: convert ->readlink to same signature as ->get_link
      vfs: remove page_readlink()
      vfs: make readlink_copy() static
      nsfs: clean up ns_get_name() interface

---
 Documentation/filesystems/Locking             |  6 +-
 Documentation/filesystems/porting             |  7 +++
 Documentation/filesystems/vfs.txt             | 14 +++--
 drivers/staging/lustre/lustre/llite/symlink.c |  1 -
 fs/9p/vfs_inode.c                             |  1 -
 fs/9p/vfs_inode_dotl.c                        |  1 -
 fs/affs/symlink.c                             |  1 -
 fs/afs/mntpt.c                                |  2 +-
 fs/autofs4/symlink.c                          |  1 -
 fs/bad_inode.c                                |  7 ---
 fs/btrfs/inode.c                              |  1 -
 fs/ceph/inode.c                               |  1 -
 fs/cifs/cifsfs.c                              |  1 -
 fs/coda/cnode.c                               |  1 -
 fs/configfs/symlink.c                         |  1 -
 fs/ecryptfs/inode.c                           | 30 ++++------
 fs/ext2/symlink.c                             |  2 -
 fs/ext4/symlink.c                             |  3 -
 fs/f2fs/namei.c                               |  2 -
 fs/fuse/dir.c                                 |  1 -
 fs/gfs2/inode.c                               |  1 -
 fs/hostfs/hostfs_kern.c                       |  1 -
 fs/jffs2/symlink.c                            |  1 -
 fs/jfs/symlink.c                              |  2 -
 fs/kernfs/symlink.c                           |  1 -
 fs/libfs.c                                    |  1 -
 fs/minix/inode.c                              |  1 -
 fs/namei.c                                    | 83 +++++++++++++++++++--------
 fs/ncpfs/inode.c                              |  1 -
 fs/nfs/symlink.c                              |  1 -
 fs/nfsd/nfs4xdr.c                             |  8 +--
 fs/nfsd/vfs.c                                 |  6 +-
 fs/nilfs2/namei.c                             |  1 -
 fs/nsfs.c                                     | 17 ++++--
 fs/ocfs2/symlink.c                            |  1 -
 fs/orangefs/symlink.c                         |  1 -
 fs/overlayfs/copy_up.c                        | 46 ++-------------
 fs/overlayfs/inode.c                          | 30 +---------
 fs/proc/base.c                                | 57 +++++++-----------
 fs/proc/inode.c                               |  1 -
 fs/proc/namespaces.c                          | 15 +++--
 fs/proc/self.c                                | 13 -----
 fs/proc/thread_self.c                         | 14 -----
 fs/reiserfs/namei.c                           |  1 -
 fs/squashfs/symlink.c                         |  1 -
 fs/stat.c                                     |  8 ++-
 fs/sysv/inode.c                               |  1 -
 fs/ubifs/file.c                               |  1 -
 fs/xfs/xfs_ioctl.c                            |  4 +-
 fs/xfs/xfs_iops.c                             |  2 -
 include/linux/fs.h                            | 10 ++--
 include/linux/proc_ns.h                       |  4 +-
 mm/shmem.c                                    |  2 -
 53 files changed, 157 insertions(+), 264 deletions(-)

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2016-09-28 14:47 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-12 19:29 [PATCH 00/17] clean up readlinks Miklos Szeredi
2016-09-12 19:29 ` [PATCH 01/17] bad_inode: add missing i_op initializers Miklos Szeredi
2016-09-12 19:29 ` [PATCH 02/17] ovl: use generic_readlink Miklos Szeredi
2016-09-12 19:29 ` [PATCH 03/17] proc/self: " Miklos Szeredi
2016-09-12 19:29 ` [PATCH 04/17] afs: " Miklos Szeredi
2016-09-12 19:29 ` [PATCH 05/17] bad_inode: " Miklos Szeredi
2016-09-12 19:29 ` [PATCH 06/17] vfs: remove page_readlink() Miklos Szeredi
2016-09-12 19:29 ` [PATCH 07/17] vfs: add is_following_link() helper Miklos Szeredi
2016-09-12 19:29 ` [PATCH 08/17] proc: merge proc_pid_readlink() into proc_pid_get_link() Miklos Szeredi
2016-09-12 19:29 ` [PATCH 09/17] proc: merge proc_ns_readlink() into proc_ns_get_link() Miklos Szeredi
2016-09-12 19:29 ` [PATCH 10/17] nsfs: clean up ns_get_name() interface Miklos Szeredi
2016-09-12 19:29 ` [PATCH 11/17] vfs: replace calling i_op->readlink with vfs_readlink() Miklos Szeredi
2016-09-12 19:29 ` [PATCH 12/17] vfs: remove ".readlink = generic_readlink" assignments Miklos Szeredi
2016-09-12 19:29 ` [PATCH 13/17] vfs: remove unused i_op->readlink Miklos Szeredi
2016-09-12 19:29 ` [PATCH 14/17] vfs: remove unused generic_readlink() Miklos Szeredi
2016-09-12 19:29 ` [PATCH 15/17] vfs: add vfs_get_link() helper Miklos Szeredi
2016-09-12 19:29 ` [PATCH 16/17] ovl: use vfs_get_link() Miklos Szeredi
2016-09-12 19:29 ` [PATCH 17/17] ecryptfs: " Miklos Szeredi
2016-09-27  3:10 ` [PATCH 00/17] clean up readlinks Al Viro
2016-09-27  9:38   ` Miklos Szeredi
2016-09-28  2:17     ` Al Viro
2016-09-28 14:47       ` Miklos Szeredi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).