All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] overlayfs: vfs fixes
@ 2017-09-05  7:46 Miklos Szeredi
  2017-09-05  7:46 ` [PATCH 1/3] vfs: add flags to d_real() Miklos Szeredi
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Miklos Szeredi @ 2017-09-05  7:46 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Al Viro, linux-kernel, linux-unionfs

Following patches fix overlayfs bugs.  The patches impact vfs code, so
posting for review to a wider audience.

They can also be found on the overlayfs-next branch:

 git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next

Hoping to get these bugfixes into 4.14.

Thanks,
Miklos
---

Miklos Szeredi (3):
  vfs: add flags to d_real()
  ovl: fix relatime for directories
  ovl: don't allow writing ioctl on lower layer

 Documentation/filesystems/Locking |  2 +-
 Documentation/filesystems/vfs.txt |  2 +-
 fs/inode.c                        |  3 +-
 fs/internal.h                     |  2 ++
 fs/namespace.c                    | 64 +++++++++++++++++++++++++++++++++++++--
 fs/open.c                         |  8 ++---
 fs/overlayfs/super.c              | 10 ++++--
 fs/xattr.c                        |  9 +++---
 include/linux/dcache.h            | 15 ++++++---
 include/linux/fs.h                |  2 +-
 10 files changed, 95 insertions(+), 22 deletions(-)

-- 
2.5.5

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

* [PATCH 1/3] vfs: add flags to d_real()
  2017-09-05  7:46 [PATCH 0/3] overlayfs: vfs fixes Miklos Szeredi
@ 2017-09-05  7:46 ` Miklos Szeredi
  2017-09-05  8:55   ` Amir Goldstein
  2017-09-05  7:46 ` [PATCH 2/3] ovl: fix relatime for directories Miklos Szeredi
  2017-09-05  7:46 ` [PATCH 3/3] ovl: don't allow writing ioctl on lower layer Miklos Szeredi
  2 siblings, 1 reply; 7+ messages in thread
From: Miklos Szeredi @ 2017-09-05  7:46 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Al Viro, linux-kernel, linux-unionfs

Add a separate flags argument (in addition to the open flags) to control
the behavior of d_real().

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 Documentation/filesystems/Locking |  2 +-
 Documentation/filesystems/vfs.txt |  2 +-
 fs/open.c                         |  4 ++--
 fs/overlayfs/super.c              |  4 ++--
 include/linux/dcache.h            | 11 ++++++-----
 include/linux/fs.h                |  2 +-
 6 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index fe25787ff6d4..75d2d57e2c44 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -22,7 +22,7 @@ prototypes:
 	struct vfsmount *(*d_automount)(struct path *path);
 	int (*d_manage)(const struct path *, bool);
 	struct dentry *(*d_real)(struct dentry *, const struct inode *,
-				 unsigned int);
+				 unsigned int, unsigned int);
 
 locking rules:
 		rename_lock	->d_lock	may block	rcu-walk
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 73e7d91f03dc..7f20c1bdfb67 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -990,7 +990,7 @@ struct dentry_operations {
 	struct vfsmount *(*d_automount)(struct path *);
 	int (*d_manage)(const struct path *, bool);
 	struct dentry *(*d_real)(struct dentry *, const struct inode *,
-				 unsigned int);
+				 unsigned int, unsigned int);
 };
 
   d_revalidate: called when the VFS needs to revalidate a dentry. This
diff --git a/fs/open.c b/fs/open.c
index 35bb784763a4..6d5c9a9b8c8d 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -96,7 +96,7 @@ long vfs_truncate(const struct path *path, loff_t length)
 	 * write access on the upper inode, not on the overlay inode.  For
 	 * non-overlay filesystems d_real() is an identity function.
 	 */
-	upperdentry = d_real(path->dentry, NULL, O_WRONLY);
+	upperdentry = d_real(path->dentry, NULL, O_WRONLY, 0);
 	error = PTR_ERR(upperdentry);
 	if (IS_ERR(upperdentry))
 		goto mnt_drop_write_and_out;
@@ -857,7 +857,7 @@ EXPORT_SYMBOL(file_path);
 int vfs_open(const struct path *path, struct file *file,
 	     const struct cred *cred)
 {
-	struct dentry *dentry = d_real(path->dentry, NULL, file->f_flags);
+	struct dentry *dentry = d_real(path->dentry, NULL, file->f_flags, 0);
 
 	if (IS_ERR(dentry))
 		return PTR_ERR(dentry);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index c0c02cca776b..19e89ce39017 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -70,7 +70,7 @@ static int ovl_check_append_only(struct inode *inode, int flag)
 
 static struct dentry *ovl_d_real(struct dentry *dentry,
 				 const struct inode *inode,
-				 unsigned int open_flags)
+				 unsigned int open_flags, unsigned int flags)
 {
 	struct dentry *real;
 	int err;
@@ -102,7 +102,7 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
 		goto bug;
 
 	/* Handle recursion */
-	real = d_real(real, inode, open_flags);
+	real = d_real(real, inode, open_flags, 0);
 
 	if (!inode || inode == d_inode(real))
 		return real;
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index aae1cdb76851..fd0721e520f4 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -147,7 +147,7 @@ struct dentry_operations {
 	struct vfsmount *(*d_automount)(struct path *);
 	int (*d_manage)(const struct path *, bool);
 	struct dentry *(*d_real)(struct dentry *, const struct inode *,
-				 unsigned int);
+				 unsigned int, unsigned int);
 } ____cacheline_aligned;
 
 /*
@@ -566,7 +566,8 @@ static inline struct dentry *d_backing_dentry(struct dentry *upper)
  * d_real - Return the real dentry
  * @dentry: the dentry to query
  * @inode: inode to select the dentry from multiple layers (can be NULL)
- * @flags: open flags to control copy-up behavior
+ * @open_flags: open flags to control copy-up behavior
+ * @flags: flags to control what is returned by this function
  *
  * If dentry is on a union/overlay, then return the underlying, real dentry.
  * Otherwise return the dentry itself.
@@ -575,10 +576,10 @@ static inline struct dentry *d_backing_dentry(struct dentry *upper)
  */
 static inline struct dentry *d_real(struct dentry *dentry,
 				    const struct inode *inode,
-				    unsigned int flags)
+				    unsigned int open_flags, unsigned int flags)
 {
 	if (unlikely(dentry->d_flags & DCACHE_OP_REAL))
-		return dentry->d_op->d_real(dentry, inode, flags);
+		return dentry->d_op->d_real(dentry, inode, open_flags, flags);
 	else
 		return dentry;
 }
@@ -593,7 +594,7 @@ static inline struct dentry *d_real(struct dentry *dentry,
 static inline struct inode *d_real_inode(const struct dentry *dentry)
 {
 	/* This usage of d_real() results in const dentry */
-	return d_backing_inode(d_real((struct dentry *) dentry, NULL, 0));
+	return d_backing_inode(d_real((struct dentry *) dentry, NULL, 0, 0));
 }
 
 struct name_snapshot {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6e1fd5d21248..ee1db83c39cb 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1233,7 +1233,7 @@ static inline struct inode *file_inode(const struct file *f)
 
 static inline struct dentry *file_dentry(const struct file *file)
 {
-	return d_real(file->f_path.dentry, file_inode(file), 0);
+	return d_real(file->f_path.dentry, file_inode(file), 0, 0);
 }
 
 static inline int locks_lock_file_wait(struct file *filp, struct file_lock *fl)
-- 
2.5.5

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

* [PATCH 2/3] ovl: fix relatime for directories
  2017-09-05  7:46 [PATCH 0/3] overlayfs: vfs fixes Miklos Szeredi
  2017-09-05  7:46 ` [PATCH 1/3] vfs: add flags to d_real() Miklos Szeredi
@ 2017-09-05  7:46 ` Miklos Szeredi
  2017-09-05 12:53   ` Miklos Szeredi
  2017-09-05  7:46 ` [PATCH 3/3] ovl: don't allow writing ioctl on lower layer Miklos Szeredi
  2 siblings, 1 reply; 7+ messages in thread
From: Miklos Szeredi @ 2017-09-05  7:46 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Al Viro, linux-kernel, linux-unionfs

Need to treat non-regular overlayfs files the same as regular files when
checking for an atime update.

Add a d_real() flag to make it return the real dentry for all file types.

Reported-by: "zhangyi (F)" <yi.zhang@huawei.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/inode.c             | 3 ++-
 fs/overlayfs/super.c   | 3 +++
 include/linux/dcache.h | 3 +++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/inode.c b/fs/inode.c
index 50370599e371..16e85d3a46d1 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1570,7 +1570,8 @@ static void update_ovl_inode_times(struct dentry *dentry, struct inode *inode,
 			       bool rcu)
 {
 	if (!rcu) {
-		struct inode *realinode = d_real_inode(dentry);
+		struct inode *realinode =
+			d_inode(d_real(dentry, NULL, 0, D_REAL_ALL));
 
 		if (unlikely(inode != realinode) &&
 		    (!timespec_equal(&inode->i_mtime, &realinode->i_mtime) ||
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 19e89ce39017..059ee2f19361 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -75,6 +75,9 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
 	struct dentry *real;
 	int err;
 
+	if (flags & D_REAL_ALL)
+		return ovl_dentry_real(dentry);
+
 	if (!d_is_reg(dentry)) {
 		if (!inode || inode == d_inode(dentry))
 			return dentry;
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index fd0721e520f4..9594ef3382ff 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -562,6 +562,9 @@ static inline struct dentry *d_backing_dentry(struct dentry *upper)
 	return upper;
 }
 
+/* d_real() flags */
+#define D_REAL_ALL	0x1	/* return real dentry for all file types */
+
 /**
  * d_real - Return the real dentry
  * @dentry: the dentry to query
-- 
2.5.5

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

* [PATCH 3/3] ovl: don't allow writing ioctl on lower layer
  2017-09-05  7:46 [PATCH 0/3] overlayfs: vfs fixes Miklos Szeredi
  2017-09-05  7:46 ` [PATCH 1/3] vfs: add flags to d_real() Miklos Szeredi
  2017-09-05  7:46 ` [PATCH 2/3] ovl: fix relatime for directories Miklos Szeredi
@ 2017-09-05  7:46 ` Miklos Szeredi
  2 siblings, 0 replies; 7+ messages in thread
From: Miklos Szeredi @ 2017-09-05  7:46 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Al Viro, linux-kernel, linux-unionfs

Problem with ioctl() is that it's a file operation, yet often used as an
inode operation (i.e. modify the inode despite the file being opened for
read-only).

mnt_want_write_file() is used by filesystems in such cases to get write
access on an arbitrary open file.

Since overlayfs lets filesystems do all file operations, including ioctl,
this can lead to mnt_want_write_file() returning OK for a lower file and
modification of that lower file.

This patch prevents modification by checking if the file is from an
overlayfs lower layer and returning EPERM in that case.

Need to introduce a mnt_want_write_file_path() variant that still does the
old thing for inode operations that can do the copy up + modification
correctly in such cases (fchown, fsetxattr, fremovexattr).

This does not address the correctness of such ioctls on overlayfs (the
correct way would be to copy up and attempt to perform ioctl on upper
file).

In theory this could be a regression.  We very much hope that nobody is
relying on such a hack in any sane setup.

While this patch meddles in VFS code, it has no effect on non-overlayfs
filesystems.

Reported-by: "zhangyi (F)" <yi.zhang@huawei.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/internal.h          |  2 ++
 fs/namespace.c         | 64 +++++++++++++++++++++++++++++++++++++++++++++++---
 fs/open.c              |  4 ++--
 fs/overlayfs/super.c   |  3 +++
 fs/xattr.c             |  9 +++----
 include/linux/dcache.h |  1 +
 6 files changed, 74 insertions(+), 9 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 9676fe11c093..60cdbcd2887b 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -71,8 +71,10 @@ extern void __init mnt_init(void);
 
 extern int __mnt_want_write(struct vfsmount *);
 extern int __mnt_want_write_file(struct file *);
+extern int mnt_want_write_file_path(struct file *);
 extern void __mnt_drop_write(struct vfsmount *);
 extern void __mnt_drop_write_file(struct file *);
+extern void mnt_drop_write_file_path(struct file *);
 
 /*
  * fs_struct.c
diff --git a/fs/namespace.c b/fs/namespace.c
index f8893dc6a989..df0f7521979a 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -431,13 +431,18 @@ int __mnt_want_write_file(struct file *file)
 }
 
 /**
- * mnt_want_write_file - get write access to a file's mount
+ * mnt_want_write_file_path - get write access to a file's mount
  * @file: the file who's mount on which to take a write
  *
  * This is like mnt_want_write, but it takes a file and can
  * do some optimisations if the file is open for write already
+ *
+ * Called by the vfs for cases when we have an open file at hand, but will do an
+ * inode operation on it (important distinction for files opened on overlayfs,
+ * since the file operations will come from the real underlying file, while
+ * inode operations come from the overlay).
  */
-int mnt_want_write_file(struct file *file)
+int mnt_want_write_file_path(struct file *file)
 {
 	int ret;
 
@@ -447,6 +452,53 @@ int mnt_want_write_file(struct file *file)
 		sb_end_write(file->f_path.mnt->mnt_sb);
 	return ret;
 }
+
+static inline int may_write_real(struct file *file)
+{
+	struct dentry *dentry = file->f_path.dentry;
+	struct dentry *upperdentry;
+
+	/* Writable file? */
+	if (file->f_mode & FMODE_WRITER)
+		return 0;
+
+	/* Not overlayfs? */
+	if (likely(!(dentry->d_flags & DCACHE_OP_REAL)))
+		return 0;
+
+	/* File refers to upper, writable layer? */
+	upperdentry = d_real(dentry, NULL, 0, D_REAL_UPPER);
+	if (upperdentry && file_inode(file) == d_inode(upperdentry))
+		return 0;
+
+	/* Lower layer: can't write to real file, sorry... */
+	return -EPERM;
+}
+
+/**
+ * mnt_want_write_file - get write access to a file's mount
+ * @file: the file who's mount on which to take a write
+ *
+ * This is like mnt_want_write, but it takes a file and can
+ * do some optimisations if the file is open for write already
+ *
+ * Mostly called by filesystems from their ioctl operation before performing
+ * modification.  On overlayfs this needs to check if the file is on a read-only
+ * lower layer and deny access in that case.
+ */
+int mnt_want_write_file(struct file *file)
+{
+	int ret;
+
+	ret = may_write_real(file);
+	if (!ret) {
+		sb_start_write(file_inode(file)->i_sb);
+		ret = __mnt_want_write_file(file);
+		if (ret)
+			sb_end_write(file_inode(file)->i_sb);
+	}
+	return ret;
+}
 EXPORT_SYMBOL_GPL(mnt_want_write_file);
 
 /**
@@ -484,10 +536,16 @@ void __mnt_drop_write_file(struct file *file)
 	__mnt_drop_write(file->f_path.mnt);
 }
 
-void mnt_drop_write_file(struct file *file)
+void mnt_drop_write_file_path(struct file *file)
 {
 	mnt_drop_write(file->f_path.mnt);
 }
+
+void mnt_drop_write_file(struct file *file)
+{
+	__mnt_drop_write(file->f_path.mnt);
+	sb_end_write(file_inode(file)->i_sb);
+}
 EXPORT_SYMBOL(mnt_drop_write_file);
 
 static int mnt_make_readonly(struct mount *mnt)
diff --git a/fs/open.c b/fs/open.c
index 6d5c9a9b8c8d..7ea118471dce 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -670,12 +670,12 @@ SYSCALL_DEFINE3(fchown, unsigned int, fd, uid_t, user, gid_t, group)
 	if (!f.file)
 		goto out;
 
-	error = mnt_want_write_file(f.file);
+	error = mnt_want_write_file_path(f.file);
 	if (error)
 		goto out_fput;
 	audit_file(f.file);
 	error = chown_common(&f.file->f_path, user, group);
-	mnt_drop_write_file(f.file);
+	mnt_drop_write_file_path(f.file);
 out_fput:
 	fdput(f);
 out:
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 059ee2f19361..0d53373b2ffc 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -78,6 +78,9 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
 	if (flags & D_REAL_ALL)
 		return ovl_dentry_real(dentry);
 
+	if (flags & D_REAL_UPPER)
+		return ovl_dentry_upper(dentry);
+
 	if (!d_is_reg(dentry)) {
 		if (!inode || inode == d_inode(dentry))
 			return dentry;
diff --git a/fs/xattr.c b/fs/xattr.c
index 464c94bf65f9..d7c2cf7817bb 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -23,6 +23,7 @@
 #include <linux/posix_acl_xattr.h>
 
 #include <linux/uaccess.h>
+#include "internal.h"
 
 static const char *
 strcmp_prefix(const char *a, const char *a_prefix)
@@ -496,10 +497,10 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
 	if (!f.file)
 		return error;
 	audit_file(f.file);
-	error = mnt_want_write_file(f.file);
+	error = mnt_want_write_file_path(f.file);
 	if (!error) {
 		error = setxattr(f.file->f_path.dentry, name, value, size, flags);
-		mnt_drop_write_file(f.file);
+		mnt_drop_write_file_path(f.file);
 	}
 	fdput(f);
 	return error;
@@ -728,10 +729,10 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 	if (!f.file)
 		return error;
 	audit_file(f.file);
-	error = mnt_want_write_file(f.file);
+	error = mnt_want_write_file_path(f.file);
 	if (!error) {
 		error = removexattr(f.file->f_path.dentry, name);
-		mnt_drop_write_file(f.file);
+		mnt_drop_write_file_path(f.file);
 	}
 	fdput(f);
 	return error;
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 9594ef3382ff..bf5b7d57414b 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -564,6 +564,7 @@ static inline struct dentry *d_backing_dentry(struct dentry *upper)
 
 /* d_real() flags */
 #define D_REAL_ALL	0x1	/* return real dentry for all file types */
+#define D_REAL_UPPER	0x2	/* return upper dentry or NULL if non-upper */
 
 /**
  * d_real - Return the real dentry
-- 
2.5.5

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

* Re: [PATCH 1/3] vfs: add flags to d_real()
  2017-09-05  7:46 ` [PATCH 1/3] vfs: add flags to d_real() Miklos Szeredi
@ 2017-09-05  8:55   ` Amir Goldstein
  2017-09-05  9:02     ` Miklos Szeredi
  0 siblings, 1 reply; 7+ messages in thread
From: Amir Goldstein @ 2017-09-05  8:55 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, Al Viro, linux-kernel, overlayfs

On Tue, Sep 5, 2017 at 10:46 AM, Miklos Szeredi <mszeredi@redhat.com> wrote:
> Add a separate flags argument (in addition to the open flags) to control
> the behavior of d_real().
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
...
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
>
>  static struct dentry *ovl_d_real(struct dentry *dentry,
>                                  const struct inode *inode,
> -                                unsigned int open_flags)
> +                                unsigned int open_flags, unsigned int flags)
>  {
>         struct dentry *real;
>         int err;
> @@ -102,7 +102,7 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
>                 goto bug;
>
>         /* Handle recursion */
> -       real = d_real(real, inode, open_flags);
> +       real = d_real(real, inode, open_flags, 0);
>

Shouldn't recursion pass on flags?
The answer is probably per flag.
The 2 currently proposed flags don't end up in recursion anyway,
although it is arguable that D_REAL_ALL should end up in recursion
because according to comment it should behave the same as
d_real for regular files.

For the purpose for which D_REAL_ALL was proposed (atime update)
the recursion case doesn't really matter.

Maybe a flag D_REAL_NORECURSE and then for
update_ovl_inode_times() use D_REAL_ALL|D_REAL_NORECURSE

Alternatively, update_ovl_inode_times() could use D_REAL_UPPER
and then we explicitly say that we don't care about lower mtime/ctime
modifications.

Amir.

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

* Re: [PATCH 1/3] vfs: add flags to d_real()
  2017-09-05  8:55   ` Amir Goldstein
@ 2017-09-05  9:02     ` Miklos Szeredi
  0 siblings, 0 replies; 7+ messages in thread
From: Miklos Szeredi @ 2017-09-05  9:02 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: linux-fsdevel, Al Viro, linux-kernel, overlayfs

On Tue, Sep 5, 2017 at 10:55 AM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Tue, Sep 5, 2017 at 10:46 AM, Miklos Szeredi <mszeredi@redhat.com> wrote:
>> Add a separate flags argument (in addition to the open flags) to control
>> the behavior of d_real().
>>
>> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
>> ---
> ...
>> --- a/fs/overlayfs/super.c
>> +++ b/fs/overlayfs/super.c
>>
>>  static struct dentry *ovl_d_real(struct dentry *dentry,
>>                                  const struct inode *inode,
>> -                                unsigned int open_flags)
>> +                                unsigned int open_flags, unsigned int flags)
>>  {
>>         struct dentry *real;
>>         int err;
>> @@ -102,7 +102,7 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
>>                 goto bug;
>>
>>         /* Handle recursion */
>> -       real = d_real(real, inode, open_flags);
>> +       real = d_real(real, inode, open_flags, 0);
>>
>
> Shouldn't recursion pass on flags?
> The answer is probably per flag.
> The 2 currently proposed flags don't end up in recursion anyway,
> although it is arguable that D_REAL_ALL should end up in recursion
> because according to comment it should behave the same as
> d_real for regular files.
>
> For the purpose for which D_REAL_ALL was proposed (atime update)
> the recursion case doesn't really matter.
>
> Maybe a flag D_REAL_NORECURSE and then for
> update_ovl_inode_times() use D_REAL_ALL|D_REAL_NORECURSE
>
> Alternatively, update_ovl_inode_times() could use D_REAL_UPPER
> and then we explicitly say that we don't care about lower mtime/ctime
> modifications.

Well, D_REAL_ALL imples nonrecurse.  Maybe that needs better
documentation (comment above in ovl_d_real()) but I don't otherwise
see a problem with the current state of affairs.

Thanks,
Miklos

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

* Re: [PATCH 2/3] ovl: fix relatime for directories
  2017-09-05  7:46 ` [PATCH 2/3] ovl: fix relatime for directories Miklos Szeredi
@ 2017-09-05 12:53   ` Miklos Szeredi
  0 siblings, 0 replies; 7+ messages in thread
From: Miklos Szeredi @ 2017-09-05 12:53 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Al Viro, linux-kernel, linux-unionfs, Miklos Szeredi

On Tue, Sep 5, 2017 at 10:55 AM, Amir Goldstein <amir73il@gmail.com> wrote:
[...]
> Alternatively, update_ovl_inode_times() could use D_REAL_UPPER
> and then we explicitly say that we don't care about lower mtime/ctime
> modifications.

Right, you convinced me that this is the right way to go.  Updated patch below
(and pushed to overlayfs-next).

Thanks,
Miklos
---

From: Miklos Szeredi <mszeredi@redhat.com>
Subject: ovl: fix relatime for directories

Need to treat non-regular overlayfs files the same as regular files when
checking for an atime update.

Add a d_real() flag to make it return the upper dentry for all file types.

Reported-by: "zhangyi (F)" <yi.zhang@huawei.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/inode.c             |   21 +++++++++++++++++----
 fs/overlayfs/super.c   |    3 +++
 include/linux/dcache.h |    3 +++
 3 files changed, 23 insertions(+), 4 deletions(-)

--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1569,11 +1569,24 @@ EXPORT_SYMBOL(bmap);
 static void update_ovl_inode_times(struct dentry *dentry, struct inode *inode,
 			       bool rcu)
 {
-	if (!rcu) {
-		struct inode *realinode = d_real_inode(dentry);
+	struct dentry *upperdentry;
 
-		if (unlikely(inode != realinode) &&
-		    (!timespec_equal(&inode->i_mtime, &realinode->i_mtime) ||
+	/*
+	 * Nothing to do if in rcu or if non-overlayfs
+	 */
+	if (rcu || likely(!(dentry->d_flags & DCACHE_OP_REAL)))
+		return;
+
+	upperdentry = d_real(dentry, NULL, 0, D_REAL_UPPER);
+
+	/*
+	 * If file is on lower then we can't update atime, so no worries about
+	 * stale mtime/ctime.
+	 */
+	if (upperdentry) {
+		struct inode *realinode = d_inode(upperdentry);
+
+		if ((!timespec_equal(&inode->i_mtime, &realinode->i_mtime) ||
 		     !timespec_equal(&inode->i_ctime, &realinode->i_ctime))) {
 			inode->i_mtime = realinode->i_mtime;
 			inode->i_ctime = realinode->i_ctime;
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -75,6 +75,9 @@ static struct dentry *ovl_d_real(struct
 	struct dentry *real;
 	int err;
 
+	if (flags & D_REAL_UPPER)
+		return ovl_dentry_upper(dentry);
+
 	if (!d_is_reg(dentry)) {
 		if (!inode || inode == d_inode(dentry))
 			return dentry;
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -562,6 +562,9 @@ static inline struct dentry *d_backing_d
 	return upper;
 }
 
+/* d_real() flags */
+#define D_REAL_UPPER	0x2	/* return upper dentry or NULL if non-upper */
+
 /**
  * d_real - Return the real dentry
  * @dentry: the dentry to query

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

end of thread, other threads:[~2017-09-05 12:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-05  7:46 [PATCH 0/3] overlayfs: vfs fixes Miklos Szeredi
2017-09-05  7:46 ` [PATCH 1/3] vfs: add flags to d_real() Miklos Szeredi
2017-09-05  8:55   ` Amir Goldstein
2017-09-05  9:02     ` Miklos Szeredi
2017-09-05  7:46 ` [PATCH 2/3] ovl: fix relatime for directories Miklos Szeredi
2017-09-05 12:53   ` Miklos Szeredi
2017-09-05  7:46 ` [PATCH 3/3] ovl: don't allow writing ioctl on lower layer Miklos Szeredi

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.