All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Overlayfs: Wrap ->d_inode
@ 2015-04-16 14:42 David Howells
  2015-04-16 14:42 ` [PATCH 1/4] Overlayfs: Convert S_ISDIR(dentry->d_inode) to d_is_dir()/d_can_lookup() David Howells
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: David Howells @ 2015-04-16 14:42 UTC (permalink / raw)
  To: viro, miklos; +Cc: dhowells, linux-fsdevel, linux-unionfs


Hi Al, Miklos,

Here are four patches that wrap ->d_inode inside of overlayfs.

The first patch wraps a number of S_ISDIR(dentry->d_inode) to d_is_dir(dentry)
or d_can_lookup(dentry), making the assumption that a directory in a lower
layer will always correspond to a directory in the union layer - and thus the
test works for both the overlay's own inodes and the overlay's subordinate
inodes.

The second patch wraps accesses to an overlay superblock's own inodes.  These
require the use of d_inode() and d_really_is_xxx().

The third and fourth patch wraps accesses that an overlay superblock does to
its subordinate inodes.  These require the use of d_backing_inode() and
d_is_xxx().

It is assumed that an overlay will _not_ be used as the top layer in a
unionmount.

The patches can also be found here:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=file-pin-devel-2

under the overlay-pin-20150416 tag.

David
---
David Howells (4):
      Overlayfs: Convert S_ISDIR(dentry->d_inode) to d_is_dir()/d_can_lookup()
      Overlayfs: Convert own ->d_inode to d_inode() or d_really_is_positive/negative()
      Overlayfs: Wrap accesses to ->d_inode on subordinate filesystems
      Overlayfs: Wrap RCU-mode accesses to ->d_inode on subordinate filesystems


 fs/overlayfs/copy_up.c |   18 ++++++-------
 fs/overlayfs/dir.c     |   67 ++++++++++++++++++++++++------------------------
 fs/overlayfs/inode.c   |   16 ++++++-----
 fs/overlayfs/readdir.c |    6 ++--
 fs/overlayfs/super.c   |   39 ++++++++++++++--------------
 include/linux/dcache.h |   18 +++++++++++++
 6 files changed, 92 insertions(+), 72 deletions(-)


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

* [PATCH 1/4] Overlayfs: Convert S_ISDIR(dentry->d_inode) to d_is_dir()/d_can_lookup()
  2015-04-16 14:42 [PATCH 0/4] Overlayfs: Wrap ->d_inode David Howells
@ 2015-04-16 14:42 ` David Howells
  2015-04-16 14:43 ` [PATCH 2/4] Overlayfs: Convert own ->d_inode to d_inode() or d_really_is_positive/negative() David Howells
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: David Howells @ 2015-04-16 14:42 UTC (permalink / raw)
  To: viro, miklos; +Cc: dhowells, linux-fsdevel, linux-unionfs

Convert S_ISDIR(dentry->d_inode) to d_is_dir(dentry) or d_can_lookup(dentry)
as appropriate.  This is slightly complicated as there are two options for
directories.  The difference is whether the directory in question is a real
dir with a ->lookup op or whether it's a fake dir with a ->d_automount op.

Note that if there is a directory at this position, in both overlayfs and
unionmount it must be represented as a directory inode in the union layer and
will not be represented as a dentry with NULL ->d_inode with a fallthrough.
This means that the d_is_dir() and d_can_lookup() tests are safe no matter
whether we're looking at overlayfs's own inodes or its subordinate inodes.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/overlayfs/inode.c |    2 +-
 fs/overlayfs/super.c |    8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 04f124884687..8c8f024edcc0 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -239,7 +239,7 @@ static bool ovl_need_xattr_filter(struct dentry *dentry,
 				  enum ovl_path_type type)
 {
 	if ((type & (__OVL_PATH_PURE | __OVL_PATH_UPPER)) == __OVL_PATH_UPPER)
-		return S_ISDIR(dentry->d_inode->i_mode);
+		return d_is_dir(dentry);
 	else
 		return false;
 }
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 5f0d1993e6e3..d5f7117caf5e 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -76,7 +76,7 @@ enum ovl_path_type ovl_path_type(struct dentry *dentry)
 		type = __OVL_PATH_UPPER;
 
 		if (oe->numlower) {
-			if (S_ISDIR(dentry->d_inode->i_mode))
+			if (d_is_dir(dentry))
 				type |= __OVL_PATH_MERGE;
 		} else if (!oe->opaque) {
 			type |= __OVL_PATH_PURE;
@@ -395,8 +395,8 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		if (i < poe->numlower - 1 && ovl_is_opaquedir(this))
 			opaque = true;
 
-		if (prev && (!S_ISDIR(prev->d_inode->i_mode) ||
-			     !S_ISDIR(this->d_inode->i_mode))) {
+		if (prev && (!d_is_dir(prev) ||
+			     !d_is_dir(this))) {
 			/*
 			 * FIXME: check for upper-opaqueness maybe better done
 			 * in remove code.
@@ -731,7 +731,7 @@ static int ovl_mount_dir_noesc(const char *name, struct path *path)
 		pr_err("overlayfs: filesystem on '%s' not supported\n", name);
 		goto out_put;
 	}
-	if (!S_ISDIR(path->dentry->d_inode->i_mode)) {
+	if (!d_can_lookup(path->dentry)) {
 		pr_err("overlayfs: '%s' not a directory\n", name);
 		goto out_put;
 	}


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

* [PATCH 2/4] Overlayfs: Convert own ->d_inode to d_inode() or d_really_is_positive/negative()
  2015-04-16 14:42 [PATCH 0/4] Overlayfs: Wrap ->d_inode David Howells
  2015-04-16 14:42 ` [PATCH 1/4] Overlayfs: Convert S_ISDIR(dentry->d_inode) to d_is_dir()/d_can_lookup() David Howells
@ 2015-04-16 14:43 ` David Howells
  2015-04-16 14:43 ` [PATCH 3/4] Overlayfs: Wrap accesses to ->d_inode on subordinate filesystems David Howells
  2015-04-16 14:43 ` [PATCH 4/4] Overlayfs: Wrap RCU-mode " David Howells
  3 siblings, 0 replies; 6+ messages in thread
From: David Howells @ 2015-04-16 14:43 UTC (permalink / raw)
  To: viro, miklos; +Cc: dhowells, linux-fsdevel, linux-unionfs

Convert instances of dentry->d_inode that refer to an overlay filesystem's own
inodes to d_inode(dentry), d_really_is_positive() or d_really_is_negative() as
appropriate.

Where dentry->d_inode refers to a subordinate filesystem, even if that
subordinate is itself an overlay, other accessors should be used.  These will
be addressed in a separate patch.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/overlayfs/dir.c   |   15 ++++++++-------
 fs/overlayfs/super.c |   10 +++++-----
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index d139405d2bfa..7b15d88d704e 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -54,7 +54,7 @@ static struct dentry *ovl_whiteout(struct dentry *workdir,
 {
 	int err;
 	struct dentry *whiteout;
-	struct inode *wdir = workdir->d_inode;
+	struct inode *wdir = d_inode(workdir);
 
 	whiteout = ovl_lookup_temp(workdir, dentry);
 	if (IS_ERR(whiteout))
@@ -75,7 +75,7 @@ int ovl_create_real(struct inode *dir, struct dentry *newdentry,
 {
 	int err;
 
-	if (newdentry->d_inode)
+	if (d_really_is_positive(newdentry))
 		return -ESTALE;
 
 	if (hardlink) {
@@ -106,7 +106,8 @@ int ovl_create_real(struct inode *dir, struct dentry *newdentry,
 			err = -EPERM;
 		}
 	}
-	if (!err && WARN_ON(!newdentry->d_inode)) {
+
+	if (!err && WARN_ON(d_really_is_negative(newdentry))) {
 		/*
 		 * Not quite sure if non-instantiated dentry is legal or not.
 		 * VFS doesn't seem to care so check and warn here.
@@ -145,7 +146,7 @@ static int ovl_dir_getattr(struct vfsmount *mnt, struct dentry *dentry,
 		return err;
 
 	stat->dev = dentry->d_sb->s_dev;
-	stat->ino = dentry->d_inode->i_ino;
+	stat->ino = d_inode(dentry)->i_ino;
 
 	/*
 	 * It's probably not worth it to count subdirs to get the
@@ -715,7 +716,7 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old,
 	if (OVL_TYPE_MERGE_OR_LOWER(old_type) && is_dir)
 		goto out;
 
-	if (new->d_inode) {
+	if (d_really_is_positive(new)) {
 		err = ovl_check_sticky(new);
 		if (err)
 			goto out;
@@ -798,14 +799,14 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old,
 
 	if (overwrite) {
 		if (old_opaque) {
-			if (new->d_inode || !new_opaque) {
+			if (d_really_is_positive(new) || !new_opaque) {
 				/* Whiteout source */
 				flags |= RENAME_WHITEOUT;
 			} else {
 				/* Switch whiteouts */
 				flags |= RENAME_EXCHANGE;
 			}
-		} else if (is_dir && !new->d_inode && new_opaque) {
+		} else if (is_dir && d_really_is_negative(new) && new_opaque) {
 			flags |= RENAME_EXCHANGE;
 			cleanup_whiteout = true;
 		}
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index d5f7117caf5e..c1ae2b5e1f4e 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -224,7 +224,7 @@ void ovl_dentry_version_inc(struct dentry *dentry)
 {
 	struct ovl_entry *oe = dentry->d_fsdata;
 
-	WARN_ON(!mutex_is_locked(&dentry->d_inode->i_mutex));
+	WARN_ON(!mutex_is_locked(&d_inode(dentry)->i_mutex));
 	oe->version++;
 }
 
@@ -232,13 +232,13 @@ u64 ovl_dentry_version_get(struct dentry *dentry)
 {
 	struct ovl_entry *oe = dentry->d_fsdata;
 
-	WARN_ON(!mutex_is_locked(&dentry->d_inode->i_mutex));
+	WARN_ON(!mutex_is_locked(&d_inode(dentry)->i_mutex));
 	return oe->version;
 }
 
 bool ovl_is_whiteout(struct dentry *dentry)
 {
-	struct inode *inode = dentry->d_inode;
+	struct inode *inode = d_inode(dentry);
 
 	return inode && IS_WHITEOUT(inode);
 }
@@ -247,7 +247,7 @@ static bool ovl_is_opaquedir(struct dentry *dentry)
 {
 	int res;
 	char val;
-	struct inode *inode = dentry->d_inode;
+	struct inode *inode = d_inode(dentry);
 
 	if (!S_ISDIR(inode->i_mode) || !inode->i_op->getxattr)
 		return false;
@@ -409,7 +409,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		/*
 		 * If this is a non-directory then stop here.
 		 */
-		if (!S_ISDIR(this->d_inode->i_mode))
+		if (!S_ISDIR(d_inode(this)->i_mode))
 			opaque = true;
 
 		stack[ctr].dentry = this;


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

* [PATCH 3/4] Overlayfs: Wrap accesses to ->d_inode on subordinate filesystems
  2015-04-16 14:42 [PATCH 0/4] Overlayfs: Wrap ->d_inode David Howells
  2015-04-16 14:42 ` [PATCH 1/4] Overlayfs: Convert S_ISDIR(dentry->d_inode) to d_is_dir()/d_can_lookup() David Howells
  2015-04-16 14:43 ` [PATCH 2/4] Overlayfs: Convert own ->d_inode to d_inode() or d_really_is_positive/negative() David Howells
@ 2015-04-16 14:43 ` David Howells
  2015-04-17  8:53   ` Miklos Szeredi
  2015-04-16 14:43 ` [PATCH 4/4] Overlayfs: Wrap RCU-mode " David Howells
  3 siblings, 1 reply; 6+ messages in thread
From: David Howells @ 2015-04-16 14:43 UTC (permalink / raw)
  To: viro, miklos; +Cc: dhowells, linux-fsdevel, linux-unionfs

Convert ->d_inode to d_backing_inode() or d_is_xxx() when it is being used to
access an inode on a subordinate filesystem of an overlay - even if that
subordinate is itself an overlay.

Accesses to an overlay's own inodes should be accessed using d_inode() and
d_really_is_positive/negative().

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/overlayfs/copy_up.c |   18 ++++++++---------
 fs/overlayfs/dir.c     |   52 ++++++++++++++++++++++++------------------------
 fs/overlayfs/inode.c   |   12 ++++++-----
 fs/overlayfs/readdir.c |    6 +++---
 fs/overlayfs/super.c   |   21 ++++++++++---------
 5 files changed, 55 insertions(+), 54 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 24f640441bd9..217c604b7b81 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -26,8 +26,8 @@ int ovl_copy_xattr(struct dentry *old, struct dentry *new)
 	char *buf, *name, *value;
 	int error;
 
-	if (!old->d_inode->i_op->getxattr ||
-	    !new->d_inode->i_op->getxattr)
+	if (!d_backing_inode(old)->i_op->getxattr ||
+	    !d_backing_inode(new)->i_op->getxattr)
 		return 0;
 
 	list_size = vfs_listxattr(old, NULL, 0);
@@ -126,7 +126,7 @@ static char *ovl_read_symlink(struct dentry *realdentry)
 {
 	int res;
 	char *buf;
-	struct inode *inode = realdentry->d_inode;
+	struct inode *inode = d_backing_inode(realdentry);
 	mm_segment_t old_fs;
 
 	res = -EINVAL;
@@ -198,8 +198,8 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
 			      struct kstat *stat, struct iattr *attr,
 			      const char *link)
 {
-	struct inode *wdir = workdir->d_inode;
-	struct inode *udir = upperdir->d_inode;
+	struct inode *wdir = d_backing_inode(workdir);
+	struct inode *udir = d_backing_inode(upperdir);
 	struct dentry *newdentry = NULL;
 	struct dentry *upper = NULL;
 	umode_t mode = stat->mode;
@@ -238,11 +238,11 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
 	if (err)
 		goto out_cleanup;
 
-	mutex_lock(&newdentry->d_inode->i_mutex);
+	mutex_lock(&d_backing_inode(newdentry)->i_mutex);
 	err = ovl_set_attr(newdentry, stat);
 	if (!err && attr)
 		err = notify_change(newdentry, attr, NULL);
-	mutex_unlock(&newdentry->d_inode->i_mutex);
+	mutex_unlock(&d_backing_inode(newdentry)->i_mutex);
 	if (err)
 		goto out_cleanup;
 
@@ -346,9 +346,9 @@ int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
 		err = 0;
 		/* Raced with another copy-up?  Do the setattr here */
 		if (attr) {
-			mutex_lock(&upperdentry->d_inode->i_mutex);
+			mutex_lock(&d_backing_inode(upperdentry)->i_mutex);
 			err = notify_change(upperdentry, attr, NULL);
-			mutex_unlock(&upperdentry->d_inode->i_mutex);
+			mutex_unlock(&d_backing_inode(upperdentry)->i_mutex);
 		}
 		goto out_put_cred;
 	}
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 7b15d88d704e..ff4d90e482cd 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -39,7 +39,7 @@ struct dentry *ovl_lookup_temp(struct dentry *workdir, struct dentry *dentry)
 	snprintf(name, sizeof(name), "#%lx", (unsigned long) dentry);
 
 	temp = lookup_one_len(name, workdir, strlen(name));
-	if (!IS_ERR(temp) && temp->d_inode) {
+	if (!IS_ERR(temp) && d_is_positive(temp)) {
 		pr_err("overlayfs: workdir/%s already exists\n", name);
 		dput(temp);
 		temp = ERR_PTR(-EIO);
@@ -164,7 +164,7 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
 			    struct dentry *hardlink)
 {
 	struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
-	struct inode *udir = upperdir->d_inode;
+	struct inode *udir = d_backing_inode(upperdir);
 	struct dentry *newdentry;
 	int err;
 
@@ -180,7 +180,7 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
 
 	ovl_dentry_version_inc(dentry->d_parent);
 	ovl_dentry_update(dentry, newdentry);
-	ovl_copyattr(newdentry->d_inode, inode);
+	ovl_copyattr(d_backing_inode(newdentry), inode);
 	d_instantiate(dentry, inode);
 	newdentry = NULL;
 out_dput:
@@ -214,9 +214,9 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
 				      struct list_head *list)
 {
 	struct dentry *workdir = ovl_workdir(dentry);
-	struct inode *wdir = workdir->d_inode;
+	struct inode *wdir = d_backing_inode(workdir);
 	struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
-	struct inode *udir = upperdir->d_inode;
+	struct inode *udir = d_backing_inode(upperdir);
 	struct path upperpath;
 	struct dentry *upper;
 	struct dentry *opaquedir;
@@ -236,7 +236,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
 	if (!S_ISDIR(stat.mode))
 		goto out_unlock;
 	upper = upperpath.dentry;
-	if (upper->d_parent->d_inode != udir)
+	if (d_backing_inode(upper->d_parent) != udir)
 		goto out_unlock;
 
 	opaquedir = ovl_lookup_temp(workdir, dentry);
@@ -256,9 +256,9 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
 	if (err)
 		goto out_cleanup;
 
-	mutex_lock(&opaquedir->d_inode->i_mutex);
+	mutex_lock(&d_backing_inode(opaquedir)->i_mutex);
 	err = ovl_set_attr(opaquedir, &stat);
-	mutex_unlock(&opaquedir->d_inode->i_mutex);
+	mutex_unlock(&d_backing_inode(opaquedir)->i_mutex);
 	if (err)
 		goto out_cleanup;
 
@@ -316,9 +316,9 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
 				    struct dentry *hardlink)
 {
 	struct dentry *workdir = ovl_workdir(dentry);
-	struct inode *wdir = workdir->d_inode;
+	struct inode *wdir = d_backing_inode(workdir);
 	struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
-	struct inode *udir = upperdir->d_inode;
+	struct inode *udir = d_backing_inode(upperdir);
 	struct dentry *upper;
 	struct dentry *newdentry;
 	int err;
@@ -360,7 +360,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
 	}
 	ovl_dentry_version_inc(dentry->d_parent);
 	ovl_dentry_update(dentry, newdentry);
-	ovl_copyattr(newdentry->d_inode, inode);
+	ovl_copyattr(d_backing_inode(newdentry), inode);
 	d_instantiate(dentry, inode);
 	newdentry = NULL;
 out_dput2:
@@ -488,7 +488,7 @@ static int ovl_link(struct dentry *old, struct inode *newdir,
 		goto out_drop_write;
 
 	upper = ovl_dentry_upper(old);
-	err = ovl_create_or_link(new, upper->d_inode->i_mode, 0, NULL, upper);
+	err = ovl_create_or_link(new, d_backing_inode(upper)->i_mode, 0, NULL, upper);
 
 out_drop_write:
 	ovl_drop_write(old);
@@ -499,9 +499,9 @@ out:
 static int ovl_remove_and_whiteout(struct dentry *dentry, bool is_dir)
 {
 	struct dentry *workdir = ovl_workdir(dentry);
-	struct inode *wdir = workdir->d_inode;
+	struct inode *wdir = d_backing_inode(workdir);
 	struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
-	struct inode *udir = upperdir->d_inode;
+	struct inode *udir = d_backing_inode(upperdir);
 	struct dentry *whiteout;
 	struct dentry *upper;
 	struct dentry *opaquedir = NULL;
@@ -573,7 +573,7 @@ kill_whiteout:
 static int ovl_remove_upper(struct dentry *dentry, bool is_dir)
 {
 	struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
-	struct inode *dir = upperdir->d_inode;
+	struct inode *dir = d_backing_inode(upperdir);
 	struct dentry *upper = ovl_dentry_upper(dentry);
 	int err;
 
@@ -604,8 +604,8 @@ static int ovl_remove_upper(struct dentry *dentry, bool is_dir)
 
 static inline int ovl_check_sticky(struct dentry *dentry)
 {
-	struct inode *dir = ovl_dentry_real(dentry->d_parent)->d_inode;
-	struct inode *inode = ovl_dentry_real(dentry)->d_inode;
+	struct inode *dir = d_backing_inode(ovl_dentry_real(dentry->d_parent));
+	struct inode *inode = d_backing_inode(ovl_dentry_real(dentry));
 
 	if (check_sticky(dir, inode))
 		return -EPERM;
@@ -731,13 +731,13 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old,
 
 		err = 0;
 		if (!OVL_TYPE_UPPER(new_type) && !OVL_TYPE_UPPER(old_type)) {
-			if (ovl_dentry_lower(old)->d_inode ==
-			    ovl_dentry_lower(new)->d_inode)
+			if (d_backing_inode(ovl_dentry_lower(old)) ==
+			    d_backing_inode(ovl_dentry_lower(new)))
 				goto out;
 		}
 		if (OVL_TYPE_UPPER(new_type) && OVL_TYPE_UPPER(old_type)) {
-			if (ovl_dentry_upper(old)->d_inode ==
-			    ovl_dentry_upper(new)->d_inode)
+			if (d_backing_inode(ovl_dentry_upper(old)) ==
+			    d_backing_inode(ovl_dentry_upper(new)))
 				goto out;
 		}
 	} else {
@@ -857,14 +857,14 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old,
 	}
 
 	if (old_opaque || new_opaque) {
-		err = ovl_do_rename(old_upperdir->d_inode, olddentry,
-				    new_upperdir->d_inode, newdentry,
+		err = ovl_do_rename(d_backing_inode(old_upperdir), olddentry,
+				    d_backing_inode(new_upperdir), newdentry,
 				    flags);
 	} else {
 		/* No debug for the plain case */
 		BUG_ON(flags & ~RENAME_EXCHANGE);
-		err = vfs_rename(old_upperdir->d_inode, olddentry,
-				 new_upperdir->d_inode, newdentry,
+		err = vfs_rename(d_backing_inode(old_upperdir), olddentry,
+				 d_backing_inode(new_upperdir), newdentry,
 				 NULL, flags);
 	}
 
@@ -888,7 +888,7 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old,
 	}
 
 	if (cleanup_whiteout)
-		ovl_cleanup(old_upperdir->d_inode, newdentry);
+		ovl_cleanup(d_backing_inode(old_upperdir), newdentry);
 
 	ovl_dentry_version_inc(old->d_parent);
 	ovl_dentry_version_inc(new->d_parent);
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 8c8f024edcc0..ead6a7a6fe9d 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -51,9 +51,9 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr)
 
 	upperdentry = ovl_dentry_upper(dentry);
 	if (upperdentry) {
-		mutex_lock(&upperdentry->d_inode->i_mutex);
+		mutex_lock(&d_backing_inode(upperdentry)->i_mutex);
 		err = notify_change(upperdentry, attr, NULL);
-		mutex_unlock(&upperdentry->d_inode->i_mutex);
+		mutex_unlock(&d_backing_inode(upperdentry)->i_mutex);
 	} else {
 		err = ovl_copy_up_last(dentry, attr, false);
 	}
@@ -147,7 +147,7 @@ static void *ovl_follow_link(struct dentry *dentry, struct nameidata *nd)
 	struct inode *realinode;
 
 	realdentry = ovl_dentry_real(dentry);
-	realinode = realdentry->d_inode;
+	realinode = d_backing_inode(realdentry);
 
 	if (WARN_ON(!realinode->i_op->follow_link))
 		return ERR_PTR(-EPERM);
@@ -181,7 +181,7 @@ static void ovl_put_link(struct dentry *dentry, struct nameidata *nd, void *c)
 	if (!data)
 		return;
 
-	realinode = data->realdentry->d_inode;
+	realinode = d_backing_inode(data->realdentry);
 	realinode->i_op->put_link(data->realdentry, nd, data->cookie);
 	kfree(data);
 }
@@ -192,7 +192,7 @@ static int ovl_readlink(struct dentry *dentry, char __user *buf, int bufsiz)
 	struct inode *realinode;
 
 	ovl_path_real(dentry, &realpath);
-	realinode = realpath.dentry->d_inode;
+	realinode = d_backing_inode(realpath.dentry);
 
 	if (!realinode->i_op->readlink)
 		return -EINVAL;
@@ -327,7 +327,7 @@ static bool ovl_open_need_copy_up(int flags, enum ovl_path_type type,
 	if (OVL_TYPE_UPPER(type))
 		return false;
 
-	if (special_file(realdentry->d_inode->i_mode))
+	if (d_is_special(realdentry))
 		return false;
 
 	if (!(OPEN_FMODE(flags) & FMODE_WRITE) && !(flags & O_TRUNC))
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 907870e81a72..af175de51bad 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -536,7 +536,7 @@ void ovl_cleanup_whiteouts(struct dentry *upper, struct list_head *list)
 {
 	struct ovl_cache_entry *p;
 
-	mutex_lock_nested(&upper->d_inode->i_mutex, I_MUTEX_CHILD);
+	mutex_lock_nested(&d_backing_inode(upper)->i_mutex, I_MUTEX_CHILD);
 	list_for_each_entry(p, list, l_node) {
 		struct dentry *dentry;
 
@@ -550,8 +550,8 @@ void ovl_cleanup_whiteouts(struct dentry *upper, struct list_head *list)
 			       (int) PTR_ERR(dentry));
 			continue;
 		}
-		ovl_cleanup(upper->d_inode, dentry);
+		ovl_cleanup(d_backing_inode(upper), dentry);
 		dput(dentry);
 	}
-	mutex_unlock(&upper->d_inode->i_mutex);
+	mutex_unlock(&d_backing_inode(upper)->i_mutex);
 }
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index c1ae2b5e1f4e..fc6057557201 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -209,9 +209,9 @@ void ovl_dentry_update(struct dentry *dentry, struct dentry *upperdentry)
 {
 	struct ovl_entry *oe = dentry->d_fsdata;
 
-	WARN_ON(!mutex_is_locked(&upperdentry->d_parent->d_inode->i_mutex));
+	WARN_ON(!mutex_is_locked(&d_backing_inode(upperdentry->d_parent)->i_mutex));
 	WARN_ON(oe->__upperdentry);
-	BUG_ON(!upperdentry->d_inode);
+	BUG_ON(!d_backing_inode(upperdentry));
 	/*
 	 * Make sure upperdentry is consistent before making it visible to
 	 * ovl_upperdentry_dereference().
@@ -293,14 +293,14 @@ static inline struct dentry *ovl_lookup_real(struct dentry *dir,
 {
 	struct dentry *dentry;
 
-	mutex_lock(&dir->d_inode->i_mutex);
+	mutex_lock(&d_backing_inode(dir)->i_mutex);
 	dentry = lookup_one_len(name->name, dir, name->len);
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&d_backing_inode(dir)->i_mutex);
 
 	if (IS_ERR(dentry)) {
 		if (PTR_ERR(dentry) == -ENOENT)
 			dentry = NULL;
-	} else if (!dentry->d_inode) {
+	} else if (d_is_miss(dentry)) {
 		dput(dentry);
 		dentry = NULL;
 	}
@@ -427,15 +427,16 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 
 	if (upperdentry || ctr) {
 		struct dentry *realdentry;
+		struct inode *realinode;
 
 		realdentry = upperdentry ? upperdentry : stack[0].dentry;
+		realinode = d_backing_inode(realdentry);
 
 		err = -ENOMEM;
-		inode = ovl_new_inode(dentry->d_sb, realdentry->d_inode->i_mode,
-				      oe);
+		inode = ovl_new_inode(dentry->d_sb, realinode->i_mode, oe);
 		if (!inode)
 			goto out_free_oe;
-		ovl_copyattr(realdentry->d_inode, inode);
+		ovl_copyattr(realinode, inode);
 	}
 
 	oe->opaque = upperopaque;
@@ -635,7 +636,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 static struct dentry *ovl_workdir_create(struct vfsmount *mnt,
 					 struct dentry *dentry)
 {
-	struct inode *dir = dentry->d_inode;
+	struct inode *dir = d_backing_inode(dentry);
 	struct dentry *work;
 	int err;
 	bool retried = false;
@@ -654,7 +655,7 @@ retry:
 			.mode = S_IFDIR | 0,
 		};
 
-		if (work->d_inode) {
+		if (d_backing_inode(work)) {
 			err = -EEXIST;
 			if (retried)
 				goto out_dput;


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

* [PATCH 4/4] Overlayfs: Wrap RCU-mode accesses to ->d_inode on subordinate filesystems
  2015-04-16 14:42 [PATCH 0/4] Overlayfs: Wrap ->d_inode David Howells
                   ` (2 preceding siblings ...)
  2015-04-16 14:43 ` [PATCH 3/4] Overlayfs: Wrap accesses to ->d_inode on subordinate filesystems David Howells
@ 2015-04-16 14:43 ` David Howells
  3 siblings, 0 replies; 6+ messages in thread
From: David Howells @ 2015-04-16 14:43 UTC (permalink / raw)
  To: viro, miklos; +Cc: dhowells, linux-fsdevel, linux-unionfs

There is a place in overlayfs where it may touch a lower-layer inode during
RCU-mode pathwalk.  In this situation the combination of d_backing_inode() and
ACCESS_ONCE() does not compile, so institute and use a d_backing_inode_rcu()
function that combines these.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/overlayfs/inode.c   |    2 +-
 include/linux/dcache.h |   18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index ead6a7a6fe9d..7e3a7e3ff095 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -99,7 +99,7 @@ int ovl_permission(struct inode *inode, int mask)
 	realdentry = ovl_entry_real(oe, &is_upper);
 
 	/* Careful in RCU walk mode */
-	realinode = ACCESS_ONCE(realdentry->d_inode);
+	realinode = d_backing_inode_rcu(realdentry);
 	if (!realinode) {
 		WARN_ON(!(mask & MAY_NOT_BLOCK));
 		err = -ENOENT;
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index df334cbacc6d..50004f2429a1 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -562,6 +562,24 @@ static inline struct inode *d_backing_inode(const struct dentry *upper)
 }
 
 /**
+ * d_backing_inode_rcu - Get upper or lower inode we should be using with ACCESS_ONCE()
+ * @upper: The upper layer
+ *
+ * This is the helper that should be used to get at the inode that will be used
+ * if this dentry were to be opened as a file.  The inode may be on the upper
+ * dentry or it may be on a lower dentry pinned by the upper.  Further, the
+ * upper dentry might not have an inode set.
+ *
+ * Normal filesystems should not use this to access their own inodes.
+ */
+static inline struct inode *d_backing_inode_rcu(const struct dentry *upper)
+{
+	struct inode *inode = ACCESS_ONCE(upper->d_inode);
+
+	return inode;
+}
+
+/**
  * d_backing_dentry - Get upper or lower dentry we should be using
  * @upper: The upper layer
  *


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

* Re: [PATCH 3/4] Overlayfs: Wrap accesses to ->d_inode on subordinate filesystems
  2015-04-16 14:43 ` [PATCH 3/4] Overlayfs: Wrap accesses to ->d_inode on subordinate filesystems David Howells
@ 2015-04-17  8:53   ` Miklos Szeredi
  0 siblings, 0 replies; 6+ messages in thread
From: Miklos Szeredi @ 2015-04-17  8:53 UTC (permalink / raw)
  To: David Howells; +Cc: Al Viro, Linux-Fsdevel, linux-unionfs

On Thu, Apr 16, 2015 at 4:43 PM, David Howells <dhowells@redhat.com> wrote:
> Convert ->d_inode to d_backing_inode() or d_is_xxx() when it is being used to
> access an inode on a subordinate filesystem of an overlay - even if that
> subordinate is itself an overlay.

Why?

What if foo is on another overlay and there's a copy-up between
mutex_lock(d_backing_inode(foo)) and
mutex_unlock(d_backing_inode(foo))? Copy-up is currently not
serialized on lower inode's i_mutex in overlayfs, and I don't see why
it would need to be.

To be honest, I find any use of d_backing_inode() outside of LSM's
highly dubious.  And each of those need would need careful scrutiny.

Why not start out simple and switch everything mindlessly to d_inode(), etc?

Then add the infrastructure for d_backing_inode() to make it actually
work, and e.g. convert selinux to use it.   Keeping everything else
unconverted will result in much less likelihood of something
accidentally breaking, like the above.

Thanks,
Miklos

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

end of thread, other threads:[~2015-04-17  8:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-16 14:42 [PATCH 0/4] Overlayfs: Wrap ->d_inode David Howells
2015-04-16 14:42 ` [PATCH 1/4] Overlayfs: Convert S_ISDIR(dentry->d_inode) to d_is_dir()/d_can_lookup() David Howells
2015-04-16 14:43 ` [PATCH 2/4] Overlayfs: Convert own ->d_inode to d_inode() or d_really_is_positive/negative() David Howells
2015-04-16 14:43 ` [PATCH 3/4] Overlayfs: Wrap accesses to ->d_inode on subordinate filesystems David Howells
2015-04-17  8:53   ` Miklos Szeredi
2015-04-16 14:43 ` [PATCH 4/4] Overlayfs: Wrap RCU-mode " David Howells

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.