Re: Patch "ovl: fix missing upper fs freeze protection on copy up for ioctl" has been added to the 4.19-stable tree
On Thu, May 23, 2019 at 12:30 PM gregkh@linuxfoundation.org wrote:
This is a note to let you know that I've just added the patch titled
ovl: fix missing upper fs freeze protection on copy up for ioctlto the 4.19-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: ovl-fix-missing-upper-fs-freeze-protection-on-copy-up-for-ioctl.patch and it can be found in the queue-4.19 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From 3428030da004a1128cbdcf93dc03e16f184d845b Mon Sep 17 00:00:00 2001 From: Amir Goldstein amir73il@gmail.com Date: Tue, 22 Jan 2019 07:01:39 +0200 Subject: ovl: fix missing upper fs freeze protection on copy up for ioctl
From: Amir Goldstein amir73il@gmail.com
commit 3428030da004a1128cbdcf93dc03e16f184d845b upstream.
Generalize the helper ovl_open_maybe_copy_up() and use it to copy up file with data before FS_IOC_SETFLAGS ioctl.
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.
Fixes: dab5ca8fd9dd ("ovl: add lsattr/chattr support") Cc: stable@vger.kernel.org # v4.19 Signed-off-by: Amir Goldstein amir73il@gmail.com Acked-by: Vivek Goyal vgoyal@redhat.com Signed-off-by: Miklos Szeredi mszeredi@redhat.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
fs/overlayfs/copy_up.c | 6 +++--- fs/overlayfs/file.c | 5 ++--- fs/overlayfs/overlayfs.h | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-)
--- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -878,14 +878,14 @@ static bool ovl_open_need_copy_up(struct 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); } }--- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -116,11 +116,10 @@ static int ovl_real_fdget(const struct f
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, 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);--- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -411,7 +411,7 @@ extern const struct file_operations ovl_ 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);
Patches currently in stable-queue which might be from amir73il@gmail.com are
queue-4.19/ovl-fix-missing-upper-fs-freeze-protection-on-copy-up-for-ioctl.patch
This patch is fine for stable, but I have a process question. All these patches from overlayfs 5.2-rc1 are also v4.9 stable candidates:
1. acf3062a7e1c - ovl: relax WARN_ON() for overlapping layers use case 2. 98487de318a6 - ovl: check the capability before cred overridden 3. d989903058a8 - ovl: do not generate duplicate fsnotify events for "fake" path 4. 9e46b840c705 - ovl: support stacked SEEK_HOLE/SEEK_DATA
#2 wasn't properly marked for stable, but the other are marked with Fixes: and Reported-by:
Are those marks not sufficient to get selected for stable trees these days?
I must admit that #1 in borderline stable. Not sure if eliminating an unjust WARN_ON qualified, but syzbot did report a bug..
Just asking in order to improve the process, but in any case, please pick those patches for v4.9+ (unless anyone objects?) They all already have LTP/xfstests/syzkaller tests that cover them.
Thanks, Amir.
On Thu, May 23, 2019 at 10:51:58PM +0300, Amir Goldstein wrote:
On Thu, May 23, 2019 at 12:30 PM gregkh@linuxfoundation.org wrote:
This is a note to let you know that I've just added the patch titled
ovl: fix missing upper fs freeze protection on copy up for ioctlto the 4.19-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: ovl-fix-missing-upper-fs-freeze-protection-on-copy-up-for-ioctl.patch and it can be found in the queue-4.19 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From 3428030da004a1128cbdcf93dc03e16f184d845b Mon Sep 17 00:00:00 2001 From: Amir Goldstein amir73il@gmail.com Date: Tue, 22 Jan 2019 07:01:39 +0200 Subject: ovl: fix missing upper fs freeze protection on copy up for ioctl
From: Amir Goldstein amir73il@gmail.com
commit 3428030da004a1128cbdcf93dc03e16f184d845b upstream.
Generalize the helper ovl_open_maybe_copy_up() and use it to copy up file with data before FS_IOC_SETFLAGS ioctl.
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.
Fixes: dab5ca8fd9dd ("ovl: add lsattr/chattr support") Cc: stable@vger.kernel.org # v4.19 Signed-off-by: Amir Goldstein amir73il@gmail.com Acked-by: Vivek Goyal vgoyal@redhat.com Signed-off-by: Miklos Szeredi mszeredi@redhat.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
fs/overlayfs/copy_up.c | 6 +++--- fs/overlayfs/file.c | 5 ++--- fs/overlayfs/overlayfs.h | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-)
--- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -878,14 +878,14 @@ static bool ovl_open_need_copy_up(struct 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); } }--- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -116,11 +116,10 @@ static int ovl_real_fdget(const struct f
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, 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);--- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -411,7 +411,7 @@ extern const struct file_operations ovl_ 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);
Patches currently in stable-queue which might be from amir73il@gmail.com are
queue-4.19/ovl-fix-missing-upper-fs-freeze-protection-on-copy-up-for-ioctl.patch
This patch is fine for stable, but I have a process question. All these patches from overlayfs 5.2-rc1 are also v4.9 stable candidates:
- acf3062a7e1c - ovl: relax WARN_ON() for overlapping layers use case
- 98487de318a6 - ovl: check the capability before cred overridden
- d989903058a8 - ovl: do not generate duplicate fsnotify events for "fake" path
- 9e46b840c705 - ovl: support stacked SEEK_HOLE/SEEK_DATA
#2 wasn't properly marked for stable, but the other are marked with Fixes: and Reported-by:
Are those marks not sufficient to get selected for stable trees these days?
Not by default, no. Sometimes they might get picked up if we get bored, or the auto-bot notices them.
I must admit that #1 in borderline stable. Not sure if eliminating an unjust WARN_ON qualified, but syzbot did report a bug..
syzbot things are good to fix in stable kernels, so that syzbot can continue to find real things in stable kernels. So yes, that is fine to backport.
Just asking in order to improve the process, but in any case, please pick those patches for v4.9+ (unless anyone objects?) They all already have LTP/xfstests/syzkaller tests that cover them.
I'll queue them up for the next round after this, thanks.
greg k-h
This patch is fine for stable, but I have a process question. All these patches from overlayfs 5.2-rc1 are also v4.9 stable candidates:
- acf3062a7e1c - ovl: relax WARN_ON() for overlapping layers use case
- 98487de318a6 - ovl: check the capability before cred overridden
- d989903058a8 - ovl: do not generate duplicate fsnotify events for "fake" path
- 9e46b840c705 - ovl: support stacked SEEK_HOLE/SEEK_DATA
#2 wasn't properly marked for stable, but the other are marked with Fixes: and Reported-by:
Are those marks not sufficient to get selected for stable trees these days?
Not by default, no. Sometimes they might get picked up if we get bored, or the auto-bot notices them.
I must admit that #1 in borderline stable. Not sure if eliminating an unjust WARN_ON qualified, but syzbot did report a bug..
syzbot things are good to fix in stable kernels, so that syzbot can continue to find real things in stable kernels. So yes, that is fine to backport.
Just asking in order to improve the process, but in any case, please pick those patches for v4.9+ (unless anyone objects?) They all already have LTP/xfstests/syzkaller tests that cover them.
I'll queue them up for the next round after this, thanks.
Hi Greg,
I forgot to follow up on those patches. Now I look at linux-4.19.y, I only see patch #1 (ovl: relax WARN_ON()..) and not the 3 other patches I listed as stable candidates. Was there any issue with those patches?
Thanks, Amir.
On Mon, Jun 10, 2019 at 11:28:46PM +0300, Amir Goldstein wrote:
This patch is fine for stable, but I have a process question. All these patches from overlayfs 5.2-rc1 are also v4.9 stable candidates:
- acf3062a7e1c - ovl: relax WARN_ON() for overlapping layers use case
- 98487de318a6 - ovl: check the capability before cred overridden
- d989903058a8 - ovl: do not generate duplicate fsnotify events for "fake" path
- 9e46b840c705 - ovl: support stacked SEEK_HOLE/SEEK_DATA
#2 wasn't properly marked for stable, but the other are marked with Fixes: and Reported-by:
Are those marks not sufficient to get selected for stable trees these days?
Not by default, no. Sometimes they might get picked up if we get bored, or the auto-bot notices them.
I must admit that #1 in borderline stable. Not sure if eliminating an unjust WARN_ON qualified, but syzbot did report a bug..
syzbot things are good to fix in stable kernels, so that syzbot can continue to find real things in stable kernels. So yes, that is fine to backport.
Just asking in order to improve the process, but in any case, please pick those patches for v4.9+ (unless anyone objects?) They all already have LTP/xfstests/syzkaller tests that cover them.
I'll queue them up for the next round after this, thanks.
Hi Greg,
I forgot to follow up on those patches. Now I look at linux-4.19.y, I only see patch #1 (ovl: relax WARN_ON()..) and not the 3 other patches I listed as stable candidates. Was there any issue with those patches?
Sorry, didn't get to them.
I did now, and they all do not apply to all kernel versions. Most of them do not go back to 4.14 or 4.9 as the code just isn't there.
So, after this next round of kernel releases, can you send backported versions of any missing patches so that we are sure to apply them correctly?
thanks,
greg k-h
On Thu, Jun 13, 2019 at 10:48 AM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jun 10, 2019 at 11:28:46PM +0300, Amir Goldstein wrote:
This patch is fine for stable, but I have a process question. All these patches from overlayfs 5.2-rc1 are also v4.9 stable candidates:
- acf3062a7e1c - ovl: relax WARN_ON() for overlapping layers use case
- 98487de318a6 - ovl: check the capability before cred overridden
- d989903058a8 - ovl: do not generate duplicate fsnotify events for "fake" path
- 9e46b840c705 - ovl: support stacked SEEK_HOLE/SEEK_DATA
#2 wasn't properly marked for stable, but the other are marked with Fixes: and Reported-by:
Are those marks not sufficient to get selected for stable trees these days?
Not by default, no. Sometimes they might get picked up if we get bored, or the auto-bot notices them.
I must admit that #1 in borderline stable. Not sure if eliminating an unjust WARN_ON qualified, but syzbot did report a bug..
syzbot things are good to fix in stable kernels, so that syzbot can continue to find real things in stable kernels. So yes, that is fine to backport.
Just asking in order to improve the process, but in any case, please pick those patches for v4.9+ (unless anyone objects?) They all already have LTP/xfstests/syzkaller tests that cover them.
I'll queue them up for the next round after this, thanks.
Hi Greg,
I forgot to follow up on those patches. Now I look at linux-4.19.y, I only see patch #1 (ovl: relax WARN_ON()..) and not the 3 other patches I listed as stable candidates. Was there any issue with those patches?
Sorry, didn't get to them.
I did now, and they all do not apply to all kernel versions. Most of them do not go back to 4.14 or 4.9 as the code just isn't there.
So, after this next round of kernel releases, can you send backported versions of any missing patches so that we are sure to apply them correctly?
Is happens to be that all those patches you picked since 4.19 most of the, whether tagged properly or not are: Fixes: d1d04ef8572b ("ovl: stack file ops") Cc: stable@vger.kernel.org # v4.19
There was a very big change in v4.19 ("ovl: stack file ops") removing lots of "VFS hacks" and resulting in more than one behavior change. Things that used to work pre-4.19 with the "VFS hacks" that do not work with "ovl: stack file ops".
There have been very few overlayfs patches since v4.20, only a trickle of v4.19 stable fixes.
Also, the code has changes quite considerably since 4.14, so overlayfs patches that could be easily backported, even if applicable to 4.14 are rare these days.
Thanks! Amir.
linux-stable-mirror@lists.linaro.org
-
Amir Goldstein -
Greg KH