* [PATCH] ovl: fix missing upper fs freeze protection on copy up for ioctl
@ 2019-01-22 5:01 Amir Goldstein
2019-01-22 16:28 ` Vivek Goyal
2019-03-01 9:29 ` Amir Goldstein
0 siblings, 2 replies; 7+ messages in thread
From: Amir Goldstein @ 2019-01-22 5:01 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Vivek Goyal, linux-unionfs
Generalize the helper ovl_open_maybe_copy_up() and use it to copy up
file with data before FS_IOC_SETFLAGS ioctl.
Fixes: dab5ca8fd9dd ("ovl: add lsattr/chattr support")
Cc: <stable@vger.kernel.org> # v4.19
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
fs/overlayfs/copy_up.c | 6 +++---
fs/overlayfs/file.c | 5 ++---
fs/overlayfs/overlayfs.h | 2 +-
3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 48119b23375b..0c0e11d529d0 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -885,14 +885,14 @@ static bool ovl_open_need_copy_up(struct dentry *dentry, int flags)
return true;
}
-int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags)
+int ovl_maybe_copy_up(struct dentry *dentry, int flags)
{
int err = 0;
- if (ovl_open_need_copy_up(dentry, file_flags)) {
+ if (ovl_open_need_copy_up(dentry, flags)) {
err = ovl_want_write(dentry);
if (!err) {
- err = ovl_copy_up_flags(dentry, file_flags);
+ err = ovl_copy_up_flags(dentry, flags);
ovl_drop_write(dentry);
}
}
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 84dd957efa24..50e4407398d8 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -116,11 +116,10 @@ static int ovl_real_fdget(const struct file *file, struct fd *real)
static int ovl_open(struct inode *inode, struct file *file)
{
- struct dentry *dentry = file_dentry(file);
struct file *realfile;
int err;
- err = ovl_open_maybe_copy_up(dentry, file->f_flags);
+ err = ovl_maybe_copy_up(file_dentry(file), file->f_flags);
if (err)
return err;
@@ -390,7 +389,7 @@ static long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
if (ret)
return ret;
- ret = ovl_copy_up_with_data(file_dentry(file));
+ ret = ovl_maybe_copy_up(file_dentry(file), O_WRONLY);
if (!ret) {
ret = ovl_real_ioctl(file, cmd, arg);
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 5e45cb3630a0..767eeaf73bf1 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -419,7 +419,7 @@ extern const struct file_operations ovl_file_operations;
int ovl_copy_up(struct dentry *dentry);
int ovl_copy_up_with_data(struct dentry *dentry);
int ovl_copy_up_flags(struct dentry *dentry, int flags);
-int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags);
+int ovl_maybe_copy_up(struct dentry *dentry, int flags);
int ovl_copy_xattr(struct dentry *old, struct dentry *new);
int ovl_set_attr(struct dentry *upper, struct kstat *stat);
struct ovl_fh *ovl_encode_real_fh(struct dentry *real, bool is_upper);
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] ovl: fix missing upper fs freeze protection on copy up for ioctl
2019-01-22 5:01 [PATCH] ovl: fix missing upper fs freeze protection on copy up for ioctl Amir Goldstein
@ 2019-01-22 16:28 ` Vivek Goyal
2019-01-22 17:49 ` Amir Goldstein
2019-03-01 9:29 ` Amir Goldstein
1 sibling, 1 reply; 7+ messages in thread
From: Vivek Goyal @ 2019-01-22 16:28 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Miklos Szeredi, linux-unionfs
On Tue, Jan 22, 2019 at 07:01:39AM +0200, Amir Goldstein wrote:
> Generalize the helper ovl_open_maybe_copy_up() and use it to copy up
> file with data before FS_IOC_SETFLAGS ioctl.
>
> Fixes: dab5ca8fd9dd ("ovl: add lsattr/chattr support")
> Cc: <stable@vger.kernel.org> # v4.19
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
What upper fs freeze protection you are referring to. I am unable to
see it in this new path.
Vivek
> ---
> fs/overlayfs/copy_up.c | 6 +++---
> fs/overlayfs/file.c | 5 ++---
> fs/overlayfs/overlayfs.h | 2 +-
> 3 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 48119b23375b..0c0e11d529d0 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -885,14 +885,14 @@ static bool ovl_open_need_copy_up(struct dentry *dentry, int flags)
> return true;
> }
>
> -int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags)
> +int ovl_maybe_copy_up(struct dentry *dentry, int flags)
> {
> int err = 0;
>
> - if (ovl_open_need_copy_up(dentry, file_flags)) {
> + if (ovl_open_need_copy_up(dentry, flags)) {
> err = ovl_want_write(dentry);
> if (!err) {
> - err = ovl_copy_up_flags(dentry, file_flags);
> + err = ovl_copy_up_flags(dentry, flags);
> ovl_drop_write(dentry);
> }
> }
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index 84dd957efa24..50e4407398d8 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -116,11 +116,10 @@ static int ovl_real_fdget(const struct file *file, struct fd *real)
>
> static int ovl_open(struct inode *inode, struct file *file)
> {
> - struct dentry *dentry = file_dentry(file);
> struct file *realfile;
> int err;
>
> - err = ovl_open_maybe_copy_up(dentry, file->f_flags);
> + err = ovl_maybe_copy_up(file_dentry(file), file->f_flags);
> if (err)
> return err;
>
> @@ -390,7 +389,7 @@ static long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> if (ret)
> return ret;
>
> - ret = ovl_copy_up_with_data(file_dentry(file));
> + ret = ovl_maybe_copy_up(file_dentry(file), O_WRONLY);
> if (!ret) {
> ret = ovl_real_ioctl(file, cmd, arg);
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 5e45cb3630a0..767eeaf73bf1 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -419,7 +419,7 @@ extern const struct file_operations ovl_file_operations;
> int ovl_copy_up(struct dentry *dentry);
> int ovl_copy_up_with_data(struct dentry *dentry);
> int ovl_copy_up_flags(struct dentry *dentry, int flags);
> -int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags);
> +int ovl_maybe_copy_up(struct dentry *dentry, int flags);
> int ovl_copy_xattr(struct dentry *old, struct dentry *new);
> int ovl_set_attr(struct dentry *upper, struct kstat *stat);
> struct ovl_fh *ovl_encode_real_fh(struct dentry *real, bool is_upper);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ovl: fix missing upper fs freeze protection on copy up for ioctl
2019-01-22 16:28 ` Vivek Goyal
@ 2019-01-22 17:49 ` Amir Goldstein
2019-01-22 18:47 ` Vivek Goyal
0 siblings, 1 reply; 7+ messages in thread
From: Amir Goldstein @ 2019-01-22 17:49 UTC (permalink / raw)
To: Vivek Goyal; +Cc: Miklos Szeredi, overlayfs
On Tue, Jan 22, 2019 at 6:28 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> On Tue, Jan 22, 2019 at 07:01:39AM +0200, Amir Goldstein wrote:
> > Generalize the helper ovl_open_maybe_copy_up() and use it to copy up
> > file with data before FS_IOC_SETFLAGS ioctl.
> >
> > Fixes: dab5ca8fd9dd ("ovl: add lsattr/chattr support")
> > Cc: <stable@vger.kernel.org> # v4.19
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> What upper fs freeze protection you are referring to. I am unable to
> see it in this new path.
>
Changed from calling ovl_copy_up_with_data() to calling
ovl_maybe_copy_up().
The former doesn't have ovl_want_write()/ovl_drop_write().
The FS_IOC_SETFLAGS ioctl is a bit of an odd ball in vfs,
which probably caused the confusion.
File may be open O_RDONLY, but ioctl modifies the file.
VFS does not call mnt_want_write_file() nor lock inode mutex,
but fs-specific code for FS_IOC_SETFLAGS does.
So ovl_ioctl() calls mnt_want_write_file() for the overlay file,
and fs-specific code calls mnt_want_write_file() for upper fs
file, but there was no call for ovl_want_write() for copy up
duration which prevents overlayfs from copying up on a
frozen upper fs.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ovl: fix missing upper fs freeze protection on copy up for ioctl
2019-01-22 17:49 ` Amir Goldstein
@ 2019-01-22 18:47 ` Vivek Goyal
0 siblings, 0 replies; 7+ messages in thread
From: Vivek Goyal @ 2019-01-22 18:47 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Miklos Szeredi, overlayfs
On Tue, Jan 22, 2019 at 07:49:42PM +0200, Amir Goldstein wrote:
> On Tue, Jan 22, 2019 at 6:28 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > On Tue, Jan 22, 2019 at 07:01:39AM +0200, Amir Goldstein wrote:
> > > Generalize the helper ovl_open_maybe_copy_up() and use it to copy up
> > > file with data before FS_IOC_SETFLAGS ioctl.
> > >
> > > Fixes: dab5ca8fd9dd ("ovl: add lsattr/chattr support")
> > > Cc: <stable@vger.kernel.org> # v4.19
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >
> > What upper fs freeze protection you are referring to. I am unable to
> > see it in this new path.
> >
>
> Changed from calling ovl_copy_up_with_data() to calling
> ovl_maybe_copy_up().
> The former doesn't have ovl_want_write()/ovl_drop_write().
>
> The FS_IOC_SETFLAGS ioctl is a bit of an odd ball in vfs,
> which probably caused the confusion.
> File may be open O_RDONLY, but ioctl modifies the file.
> VFS does not call mnt_want_write_file() nor lock inode mutex,
> but fs-specific code for FS_IOC_SETFLAGS does.
> So ovl_ioctl() calls mnt_want_write_file() for the overlay file,
> and fs-specific code calls mnt_want_write_file() for upper fs
> file, but there was no call for ovl_want_write() for copy up
> duration which prevents overlayfs from copying up on a
> frozen upper fs.
Ok, thanks for the explanaiton. I understand it better now.
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Vivek
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ovl: fix missing upper fs freeze protection on copy up for ioctl
2019-01-22 5:01 [PATCH] ovl: fix missing upper fs freeze protection on copy up for ioctl Amir Goldstein
2019-01-22 16:28 ` Vivek Goyal
@ 2019-03-01 9:29 ` Amir Goldstein
2019-03-27 6:37 ` Amir Goldstein
1 sibling, 1 reply; 7+ messages in thread
From: Amir Goldstein @ 2019-03-01 9:29 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Vivek Goyal, overlayfs
On Tue, Jan 22, 2019 at 7:01 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> Generalize the helper ovl_open_maybe_copy_up() and use it to copy up
> file with data before FS_IOC_SETFLAGS ioctl.
>
> Fixes: dab5ca8fd9dd ("ovl: add lsattr/chattr support")
> Cc: <stable@vger.kernel.org> # v4.19
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
Miklos,
Ping for this bug fix.
Thanks,
Amir.
> fs/overlayfs/copy_up.c | 6 +++---
> fs/overlayfs/file.c | 5 ++---
> fs/overlayfs/overlayfs.h | 2 +-
> 3 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 48119b23375b..0c0e11d529d0 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -885,14 +885,14 @@ static bool ovl_open_need_copy_up(struct dentry *dentry, int flags)
> return true;
> }
>
> -int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags)
> +int ovl_maybe_copy_up(struct dentry *dentry, int flags)
> {
> int err = 0;
>
> - if (ovl_open_need_copy_up(dentry, file_flags)) {
> + if (ovl_open_need_copy_up(dentry, flags)) {
> err = ovl_want_write(dentry);
> if (!err) {
> - err = ovl_copy_up_flags(dentry, file_flags);
> + err = ovl_copy_up_flags(dentry, flags);
> ovl_drop_write(dentry);
> }
> }
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index 84dd957efa24..50e4407398d8 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -116,11 +116,10 @@ static int ovl_real_fdget(const struct file *file, struct fd *real)
>
> static int ovl_open(struct inode *inode, struct file *file)
> {
> - struct dentry *dentry = file_dentry(file);
> struct file *realfile;
> int err;
>
> - err = ovl_open_maybe_copy_up(dentry, file->f_flags);
> + err = ovl_maybe_copy_up(file_dentry(file), file->f_flags);
> if (err)
> return err;
>
> @@ -390,7 +389,7 @@ static long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> if (ret)
> return ret;
>
> - ret = ovl_copy_up_with_data(file_dentry(file));
> + ret = ovl_maybe_copy_up(file_dentry(file), O_WRONLY);
> if (!ret) {
> ret = ovl_real_ioctl(file, cmd, arg);
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 5e45cb3630a0..767eeaf73bf1 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -419,7 +419,7 @@ extern const struct file_operations ovl_file_operations;
> int ovl_copy_up(struct dentry *dentry);
> int ovl_copy_up_with_data(struct dentry *dentry);
> int ovl_copy_up_flags(struct dentry *dentry, int flags);
> -int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags);
> +int ovl_maybe_copy_up(struct dentry *dentry, int flags);
> int ovl_copy_xattr(struct dentry *old, struct dentry *new);
> int ovl_set_attr(struct dentry *upper, struct kstat *stat);
> struct ovl_fh *ovl_encode_real_fh(struct dentry *real, bool is_upper);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ovl: fix missing upper fs freeze protection on copy up for ioctl
2019-03-01 9:29 ` Amir Goldstein
@ 2019-03-27 6:37 ` Amir Goldstein
2019-05-06 11:56 ` Miklos Szeredi
0 siblings, 1 reply; 7+ messages in thread
From: Amir Goldstein @ 2019-03-27 6:37 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Vivek Goyal, overlayfs
On Fri, Mar 1, 2019 at 11:29 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Tue, Jan 22, 2019 at 7:01 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > Generalize the helper ovl_open_maybe_copy_up() and use it to copy up
> > file with data before FS_IOC_SETFLAGS ioctl.
> >
> > Fixes: dab5ca8fd9dd ("ovl: add lsattr/chattr support")
> > Cc: <stable@vger.kernel.org> # v4.19
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
>
> Miklos,
>
> Ping for this bug fix.
>
PING^2
> Thanks,
> Amir.
>
> > fs/overlayfs/copy_up.c | 6 +++---
> > fs/overlayfs/file.c | 5 ++---
> > fs/overlayfs/overlayfs.h | 2 +-
> > 3 files changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> > index 48119b23375b..0c0e11d529d0 100644
> > --- a/fs/overlayfs/copy_up.c
> > +++ b/fs/overlayfs/copy_up.c
> > @@ -885,14 +885,14 @@ static bool ovl_open_need_copy_up(struct dentry *dentry, int flags)
> > return true;
> > }
> >
> > -int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags)
> > +int ovl_maybe_copy_up(struct dentry *dentry, int flags)
> > {
> > int err = 0;
> >
> > - if (ovl_open_need_copy_up(dentry, file_flags)) {
> > + if (ovl_open_need_copy_up(dentry, flags)) {
> > err = ovl_want_write(dentry);
> > if (!err) {
> > - err = ovl_copy_up_flags(dentry, file_flags);
> > + err = ovl_copy_up_flags(dentry, flags);
> > ovl_drop_write(dentry);
> > }
> > }
> > diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> > index 84dd957efa24..50e4407398d8 100644
> > --- a/fs/overlayfs/file.c
> > +++ b/fs/overlayfs/file.c
> > @@ -116,11 +116,10 @@ static int ovl_real_fdget(const struct file *file, struct fd *real)
> >
> > static int ovl_open(struct inode *inode, struct file *file)
> > {
> > - struct dentry *dentry = file_dentry(file);
> > struct file *realfile;
> > int err;
> >
> > - err = ovl_open_maybe_copy_up(dentry, file->f_flags);
> > + err = ovl_maybe_copy_up(file_dentry(file), file->f_flags);
> > if (err)
> > return err;
> >
> > @@ -390,7 +389,7 @@ static long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> > if (ret)
> > return ret;
> >
> > - ret = ovl_copy_up_with_data(file_dentry(file));
> > + ret = ovl_maybe_copy_up(file_dentry(file), O_WRONLY);
> > if (!ret) {
> > ret = ovl_real_ioctl(file, cmd, arg);
> >
> > diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> > index 5e45cb3630a0..767eeaf73bf1 100644
> > --- a/fs/overlayfs/overlayfs.h
> > +++ b/fs/overlayfs/overlayfs.h
> > @@ -419,7 +419,7 @@ extern const struct file_operations ovl_file_operations;
> > int ovl_copy_up(struct dentry *dentry);
> > int ovl_copy_up_with_data(struct dentry *dentry);
> > int ovl_copy_up_flags(struct dentry *dentry, int flags);
> > -int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags);
> > +int ovl_maybe_copy_up(struct dentry *dentry, int flags);
> > int ovl_copy_xattr(struct dentry *old, struct dentry *new);
> > int ovl_set_attr(struct dentry *upper, struct kstat *stat);
> > struct ovl_fh *ovl_encode_real_fh(struct dentry *real, bool is_upper);
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ovl: fix missing upper fs freeze protection on copy up for ioctl
2019-03-27 6:37 ` Amir Goldstein
@ 2019-05-06 11:56 ` Miklos Szeredi
0 siblings, 0 replies; 7+ messages in thread
From: Miklos Szeredi @ 2019-05-06 11:56 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Vivek Goyal, overlayfs
On Wed, Mar 27, 2019 at 2:37 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Fri, Mar 1, 2019 at 11:29 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Tue, Jan 22, 2019 at 7:01 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > Generalize the helper ovl_open_maybe_copy_up() and use it to copy up
> > > file with data before FS_IOC_SETFLAGS ioctl.
> > >
> > > Fixes: dab5ca8fd9dd ("ovl: add lsattr/chattr support")
> > > Cc: <stable@vger.kernel.org> # v4.19
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Thanks, applied.
Miklos
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-05-06 11:56 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-22 5:01 [PATCH] ovl: fix missing upper fs freeze protection on copy up for ioctl Amir Goldstein
2019-01-22 16:28 ` Vivek Goyal
2019-01-22 17:49 ` Amir Goldstein
2019-01-22 18:47 ` Vivek Goyal
2019-03-01 9:29 ` Amir Goldstein
2019-03-27 6:37 ` Amir Goldstein
2019-05-06 11:56 ` Miklos Szeredi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox