* [PATCH 0/4] ovl: fix IS_APPEND() checks
@ 2017-04-06 8:13 Amir Goldstein
2017-04-06 8:13 ` [PATCH 1/4] vfs: ftruncate freeze protect backing inode Amir Goldstein
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Amir Goldstein @ 2017-04-06 8:13 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Jan Kara, Al Viro, linux-unionfs, linux-fsdevel
Miklos,
This series fixes the append-only violations found by the new
xfstest overlay/030.
I did it to see how bad it would be compared to propagating
the S_APPEND/S_IMMUTABLE flags to overlay inode.
IMO, the [f]truncate patches look ok, but the last vfs_open()
patch less so (?).
Do you think we can get away without propagating the flags?
If getflags()/setflags() were standard vfs ops, we could have
handled this nicer...
Those "fs specific flags" have long been a standard, so perhaps
its time to make the setflags/getflags a vfs api?
Amir.
Amir Goldstein (4):
vfs: ftruncate freeze protect backing inode
vfs: ftruncate check IS_APPEND() on backing inode
vfs: truncate check IS_APPEND() on backing inode
vfs: open check IS_APPEND() on backing inode
fs/open.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] vfs: ftruncate freeze protect backing inode
2017-04-06 8:13 [PATCH 0/4] ovl: fix IS_APPEND() checks Amir Goldstein
@ 2017-04-06 8:13 ` Amir Goldstein
2017-04-06 15:42 ` Amir Goldstein
2017-04-06 8:13 ` [PATCH 2/4] vfs: ftruncate check IS_APPEND() on " Amir Goldstein
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2017-04-06 8:13 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Jan Kara, Al Viro, linux-unionfs, linux-fsdevel
ftruncate an overlayfs inode was wrongly freeze protecting the
overlay file system instead of the backing file system.
Use file_start_write() instead of sb_start_write(), which
does the right thing and will also freeze protect both overlay and
backing fs when the time comes.
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
fs/open.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/open.c b/fs/open.c
index 949cef2..53b1b33 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -196,13 +196,13 @@ static long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
if (IS_APPEND(inode))
goto out_putf;
- sb_start_write(inode->i_sb);
+ file_start_write(f.file);
error = locks_verify_truncate(inode, f.file, length);
if (!error)
error = security_path_truncate(&f.file->f_path);
if (!error)
error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, f.file);
- sb_end_write(inode->i_sb);
+ file_end_write(f.file);
out_putf:
fdput(f);
out:
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] vfs: ftruncate check IS_APPEND() on backing inode
2017-04-06 8:13 [PATCH 0/4] ovl: fix IS_APPEND() checks Amir Goldstein
2017-04-06 8:13 ` [PATCH 1/4] vfs: ftruncate freeze protect backing inode Amir Goldstein
@ 2017-04-06 8:13 ` Amir Goldstein
2017-04-06 8:13 ` [PATCH 3/4] vfs: truncate " Amir Goldstein
2017-04-06 8:13 ` [PATCH 4/4] vfs: open " Amir Goldstein
3 siblings, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2017-04-06 8:13 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Jan Kara, Al Viro, linux-unionfs, linux-fsdevel
ftruncate an overlayfs inode was checking IS_APPEND() on
overlay inode, but overlay inode does not have the S_APPEND
flag and IS_APPEND() is always checked on backing inode in
other places.
Set 'inode' var to file_inode() so all checks are performed on
backing inode and use locks_inode() explicitly for calling
locks_verify_truncate().
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
fs/open.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/open.c b/fs/open.c
index 53b1b33..0e3c12b 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -165,7 +165,6 @@ COMPAT_SYSCALL_DEFINE2(truncate, const char __user *, path, compat_off_t, length
static long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
{
struct inode *inode;
- struct dentry *dentry;
struct fd f;
int error;
@@ -181,8 +180,7 @@ static long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
if (f.file->f_flags & O_LARGEFILE)
small = 0;
- dentry = f.file->f_path.dentry;
- inode = dentry->d_inode;
+ inode = file_inode(f.file);
error = -EINVAL;
if (!S_ISREG(inode->i_mode) || !(f.file->f_mode & FMODE_WRITE))
goto out_putf;
@@ -197,11 +195,12 @@ static long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
goto out_putf;
file_start_write(f.file);
- error = locks_verify_truncate(inode, f.file, length);
+ error = locks_verify_truncate(locks_inode(f.file), f.file, length);
if (!error)
error = security_path_truncate(&f.file->f_path);
if (!error)
- error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, f.file);
+ error = do_truncate(f.file->f_path.dentry, length,
+ ATTR_MTIME|ATTR_CTIME, f.file);
file_end_write(f.file);
out_putf:
fdput(f);
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] vfs: truncate check IS_APPEND() on backing inode
2017-04-06 8:13 [PATCH 0/4] ovl: fix IS_APPEND() checks Amir Goldstein
2017-04-06 8:13 ` [PATCH 1/4] vfs: ftruncate freeze protect backing inode Amir Goldstein
2017-04-06 8:13 ` [PATCH 2/4] vfs: ftruncate check IS_APPEND() on " Amir Goldstein
@ 2017-04-06 8:13 ` Amir Goldstein
2017-04-06 8:13 ` [PATCH 4/4] vfs: open " Amir Goldstein
3 siblings, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2017-04-06 8:13 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Jan Kara, Al Viro, linux-unionfs, linux-fsdevel
truncate an overlayfs inode was checking IS_APPEND() on
overlay inode, but overlay inode does not have the S_APPEND
flag and IS_APPEND() is always checked on backing inode in
other places.
Move the IS_APPEND() check to after we have the upperdentry
and use the d_backing_inode() macro to explicitly highlight
the places where the backing inode is used.
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
fs/open.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/open.c b/fs/open.c
index 0e3c12b..baf67cd 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -87,10 +87,6 @@ long vfs_truncate(const struct path *path, loff_t length)
if (error)
goto mnt_drop_write_and_out;
- error = -EPERM;
- if (IS_APPEND(inode))
- goto mnt_drop_write_and_out;
-
/*
* If this is an overlayfs then do as if opening the file so we get
* write access on the upper inode, not on the overlay inode. For
@@ -101,7 +97,11 @@ long vfs_truncate(const struct path *path, loff_t length)
if (IS_ERR(upperdentry))
goto mnt_drop_write_and_out;
- error = get_write_access(upperdentry->d_inode);
+ error = -EPERM;
+ if (IS_APPEND(d_backing_inode(upperdentry)))
+ goto mnt_drop_write_and_out;
+
+ error = get_write_access(d_backing_inode(upperdentry));
if (error)
goto mnt_drop_write_and_out;
@@ -120,7 +120,7 @@ long vfs_truncate(const struct path *path, loff_t length)
error = do_truncate(path->dentry, length, 0, NULL);
put_write_and_out:
- put_write_access(upperdentry->d_inode);
+ put_write_access(d_backing_inode(upperdentry));
mnt_drop_write_and_out:
mnt_drop_write(path->mnt);
out:
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] vfs: open check IS_APPEND() on backing inode
2017-04-06 8:13 [PATCH 0/4] ovl: fix IS_APPEND() checks Amir Goldstein
` (2 preceding siblings ...)
2017-04-06 8:13 ` [PATCH 3/4] vfs: truncate " Amir Goldstein
@ 2017-04-06 8:13 ` Amir Goldstein
3 siblings, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2017-04-06 8:13 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Jan Kara, Al Viro, linux-unionfs, linux-fsdevel
For overlay file open, check IS_APPEND() on real backing inode
inside vfs_open(), because the overlay inode does not have the
S_APPEND flag.
This seems like the wrong place for this check.
Probably better to propagate S_APPEND flag to overlay inode.
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
fs/open.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/fs/open.c b/fs/open.c
index baf67cd..8f1754d 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -859,6 +859,15 @@ int vfs_open(const struct path *path, struct file *file,
if (IS_ERR(dentry))
return PTR_ERR(dentry);
+ /* Check append-only open flags also against real inode */
+ if (dentry != path->dentry && IS_APPEND(d_backing_inode(dentry))) {
+ if ((file->f_flags & O_ACCMODE) != O_RDONLY &&
+ !(file->f_flags & O_APPEND))
+ return -EPERM;
+ if (file->f_flags & O_TRUNC)
+ return -EPERM;
+ }
+
file->f_path = *path;
return do_dentry_open(file, d_backing_inode(dentry), NULL, cred);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/4] vfs: ftruncate freeze protect backing inode
2017-04-06 8:13 ` [PATCH 1/4] vfs: ftruncate freeze protect backing inode Amir Goldstein
@ 2017-04-06 15:42 ` Amir Goldstein
0 siblings, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2017-04-06 15:42 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Jan Kara, Al Viro, linux-unionfs, linux-fsdevel
On Thu, Apr 6, 2017 at 11:13 AM, Amir Goldstein <amir73il@gmail.com> wrote:
> ftruncate an overlayfs inode was wrongly freeze protecting the
> overlay file system instead of the backing file system.
>
> Use file_start_write() instead of sb_start_write(), which
> does the right thing and will also freeze protect both overlay and
> backing fs when the time comes.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> fs/open.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/open.c b/fs/open.c
> index 949cef2..53b1b33 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -196,13 +196,13 @@ static long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
> if (IS_APPEND(inode))
> goto out_putf;
>
> - sb_start_write(inode->i_sb);
> + file_start_write(f.file);
Nah! this is wrong.
Should be mnt_want_write_file(f.file)
At the time of this patch mnt_want_write_file(f.file) translated to
sb_start_write(inode->i_sb)
but when next patch is going to change inode = file_inode(f.file)
sb_start_write(inode->i_sb) will no longer be correct
and mnt_want_write_file(f.file) is nicer than sb_start_write(locks_inode(f.file)
> error = locks_verify_truncate(inode, f.file, length);
> if (!error)
> error = security_path_truncate(&f.file->f_path);
> if (!error)
> error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, f.file);
> - sb_end_write(inode->i_sb);
> + file_end_write(f.file);
> out_putf:
> fdput(f);
> out:
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-06 15:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-06 8:13 [PATCH 0/4] ovl: fix IS_APPEND() checks Amir Goldstein
2017-04-06 8:13 ` [PATCH 1/4] vfs: ftruncate freeze protect backing inode Amir Goldstein
2017-04-06 15:42 ` Amir Goldstein
2017-04-06 8:13 ` [PATCH 2/4] vfs: ftruncate check IS_APPEND() on " Amir Goldstein
2017-04-06 8:13 ` [PATCH 3/4] vfs: truncate " Amir Goldstein
2017-04-06 8:13 ` [PATCH 4/4] vfs: open " Amir Goldstein
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.