Attempting to unshare extents beyond EOF will trigger the need zeroing case, which in turn triggers a warning. Therefore, let's skip the unshare process if extents are beyond EOF.
Reported-and-tested-by: syzbot+296b1c84b9cbf306e5a0@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=296b1c84b9cbf306e5a0 Fixes: 32a38a499104 ("iomap: use write_begin to read pages to unshare") Inspired-by: Dave Chinner david@fromorbit.com Signed-off-by: Julian Sun sunjunchao2870@gmail.com --- fs/xfs/xfs_reflink.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c index 6fde6ec8092f..65509ff6aba0 100644 --- a/fs/xfs/xfs_reflink.c +++ b/fs/xfs/xfs_reflink.c @@ -3,6 +3,7 @@ * Copyright (C) 2016 Oracle. All Rights Reserved. * Author: Darrick J. Wong darrick.wong@oracle.com */ +#include "linux/fs.h" #include "xfs.h" #include "xfs_fs.h" #include "xfs_shared.h" @@ -1669,6 +1670,9 @@ xfs_reflink_unshare(
if (!xfs_is_reflink_inode(ip)) return 0; + /* don't try to unshare any ranges beyond EOF. */ + if (offset + len > i_size_read(inode)) + len = i_size_read(inode) - offset;
trace_xfs_reflink_unshare(ip, offset, len);
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH 1/3] xfs: Do not unshare ranges beyond EOF Link: https://lore.kernel.org/stable/20240920122621.215397-1-sunjunchao2870%40gmai...
On Fri, Sep 20, 2024 at 08:26:21PM +0800, Julian Sun wrote:
Attempting to unshare extents beyond EOF will trigger the need zeroing case, which in turn triggers a warning. Therefore, let's skip the unshare process if extents are beyond EOF.
Reported-and-tested-by: syzbot+296b1c84b9cbf306e5a0@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=296b1c84b9cbf306e5a0 Fixes: 32a38a499104 ("iomap: use write_begin to read pages to unshare") Inspired-by: Dave Chinner david@fromorbit.com Signed-off-by: Julian Sun sunjunchao2870@gmail.com
fs/xfs/xfs_reflink.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c index 6fde6ec8092f..65509ff6aba0 100644 --- a/fs/xfs/xfs_reflink.c +++ b/fs/xfs/xfs_reflink.c @@ -3,6 +3,7 @@
- Copyright (C) 2016 Oracle. All Rights Reserved.
- Author: Darrick J. Wong darrick.wong@oracle.com
*/ +#include "linux/fs.h"
This really should not be needed (and is the wrong way to include non-local headers anyway).
#include "xfs.h" #include "xfs_fs.h" #include "xfs_shared.h" @@ -1669,6 +1670,9 @@ xfs_reflink_unshare( if (!xfs_is_reflink_inode(ip)) return 0;
- /* don't try to unshare any ranges beyond EOF. */
- if (offset + len > i_size_read(inode))
len = i_size_read(inode) - offset;
So i_size is a byte granularity value, but later on iomap_file_unshare operates on blocks. If you reduce the value like this here this means we can't ever unshare the last block of a file if the file size is not block aligned, which feels odd.
Christoph Hellwig hch@infradead.org 于2024年9月20日周五 22:23写道：
On Fri, Sep 20, 2024 at 08:26:21PM +0800, Julian Sun wrote:
Attempting to unshare extents beyond EOF will trigger the need zeroing case, which in turn triggers a warning. Therefore, let's skip the unshare process if extents are beyond EOF.
Reported-and-tested-by: syzbot+296b1c84b9cbf306e5a0@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=296b1c84b9cbf306e5a0 Fixes: 32a38a499104 ("iomap: use write_begin to read pages to unshare") Inspired-by: Dave Chinner david@fromorbit.com Signed-off-by: Julian Sun sunjunchao2870@gmail.com
fs/xfs/xfs_reflink.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c index 6fde6ec8092f..65509ff6aba0 100644 --- a/fs/xfs/xfs_reflink.c +++ b/fs/xfs/xfs_reflink.c @@ -3,6 +3,7 @@
- Copyright (C) 2016 Oracle. All Rights Reserved.
- Author: Darrick J. Wong darrick.wong@oracle.com
*/ +#include "linux/fs.h"
This really should not be needed (and is the wrong way to include non-local headers anyway).
Yes, it was added automatically by vscode... I will recheck the patch before sending it.
#include "xfs.h" #include "xfs_fs.h" #include "xfs_shared.h" @@ -1669,6 +1670,9 @@ xfs_reflink_unshare(
if (!xfs_is_reflink_inode(ip)) return 0;
/* don't try to unshare any ranges beyond EOF. */if (offset + len > i_size_read(inode))len = i_size_read(inode) - offset;So i_size is a byte granularity value, but later on iomap_file_unshare operates on blocks. If you reduce the value like this here this means we can't ever unshare the last block of a file if the file size is not block aligned, which feels odd.
Got it, will fix it in patch v2. Thanks for your review and explantion.
Thanks,
linux-stable-mirror@lists.linaro.org
-
Christoph Hellwig -
Julian Sun -
kernel test robot