2020-04-27 09:59:44

by Christoph Hellwig

[permalink] [raw]
Subject: fix fiemap for ext4 bitmap files (+ cleanups)

Hi all,

the first two patches should fix the issue where ext4 doesn't
properly check the max file size for bitmap files in fiemap.

The rest cleans up the fiemap support in ext4 and in general.


2020-04-27 09:59:44

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 1/8] ext4: fix EXT4_MAX_LOGICAL_BLOCK macro

From: Ritesh Harjani <[email protected]>

ext4 supports max number of logical blocks in a file to be 0xffffffff.
(This is since ext4_extent's ee_block is __le32).
This means that EXT4_MAX_LOGICAL_BLOCK should be 0xfffffffe (starting
from 0 logical offset). This patch fixes this.

The issue was seen when ext4 moved to iomap_fiemap API and when
overlayfs was mounted on top of ext4. Since overlayfs was missing
filemap_check_ranges(), so it could pass a arbitrary huge length which
lead to overflow of map.m_len logic.

This patch fixes that.

Fixes: d3b6f23f7167 ("ext4: move ext4_fiemap to use iomap framework")
Reported-by: [email protected]
Signed-off-by: Ritesh Harjani <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/ext4/ext4.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 91eb4381cae5b..ad2dbf6e49245 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -722,7 +722,7 @@ enum {
#define EXT4_MAX_BLOCK_FILE_PHYS 0xFFFFFFFF

/* Max logical block we can support */
-#define EXT4_MAX_LOGICAL_BLOCK 0xFFFFFFFF
+#define EXT4_MAX_LOGICAL_BLOCK 0xFFFFFFFE

/*
* Structure of an inode on the disk
--
2.26.1

2020-04-27 09:59:44

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 2/8] ext4: fix fiemap size checks for bitmap files

Add an extra validation of the len parameter, as for ext4 some files
might have smaller file size limits than others. This also means the
redundant size check in ext4_ioctl_get_es_cache can go away, as all
size checking is done in the shared fiemap handler.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/ext4/extents.c | 31 +++++++++++++++++++++++++++++++
fs/ext4/ioctl.c | 33 ++-------------------------------
2 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index f2b577b315a09..2b4b94542e34d 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4832,6 +4832,28 @@ static const struct iomap_ops ext4_iomap_xattr_ops = {
.iomap_begin = ext4_iomap_xattr_begin,
};

+static int ext4_fiemap_check_ranges(struct inode *inode, u64 start, u64 *len)
+{
+ u64 maxbytes;
+
+ if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
+ maxbytes = inode->i_sb->s_maxbytes;
+ else
+ maxbytes = EXT4_SB(inode->i_sb)->s_bitmap_maxbytes;
+
+ if (*len == 0)
+ return -EINVAL;
+ if (start > maxbytes)
+ return -EFBIG;
+
+ /*
+ * Shrink request scope to what the fs can actually handle.
+ */
+ if (*len > maxbytes || (maxbytes - *len) < start)
+ *len = maxbytes - start;
+ return 0;
+}
+
static int _ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
__u64 start, __u64 len, bool from_es_cache)
{
@@ -4852,6 +4874,15 @@ static int _ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
if (fiemap_check_flags(fieinfo, ext4_fiemap_flags))
return -EBADR;

+ /*
+ * For bitmap files the maximum size limit could be smaller than
+ * s_maxbytes, so check len here manually instead of just relying on the
+ * generic check.
+ */
+ error = ext4_fiemap_check_ranges(inode, start, &len);
+ if (error)
+ return error;
+
if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
fieinfo->fi_flags &= ~FIEMAP_FLAG_XATTR;
error = iomap_fiemap(inode, fieinfo, start, len,
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index bfc1281fc4cbc..0746532ba463d 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -733,29 +733,6 @@ static void ext4_fill_fsxattr(struct inode *inode, struct fsxattr *fa)
fa->fsx_projid = from_kprojid(&init_user_ns, ei->i_projid);
}

-/* copied from fs/ioctl.c */
-static int fiemap_check_ranges(struct super_block *sb,
- u64 start, u64 len, u64 *new_len)
-{
- u64 maxbytes = (u64) sb->s_maxbytes;
-
- *new_len = len;
-
- if (len == 0)
- return -EINVAL;
-
- if (start > maxbytes)
- return -EFBIG;
-
- /*
- * Shrink request scope to what the fs can actually handle.
- */
- if (len > maxbytes || (maxbytes - len) < start)
- *new_len = maxbytes - start;
-
- return 0;
-}
-
/* So that the fiemap access checks can't overflow on 32 bit machines. */
#define FIEMAP_MAX_EXTENTS (UINT_MAX / sizeof(struct fiemap_extent))

@@ -765,8 +742,6 @@ static int ext4_ioctl_get_es_cache(struct file *filp, unsigned long arg)
struct fiemap __user *ufiemap = (struct fiemap __user *) arg;
struct fiemap_extent_info fieinfo = { 0, };
struct inode *inode = file_inode(filp);
- struct super_block *sb = inode->i_sb;
- u64 len;
int error;

if (copy_from_user(&fiemap, ufiemap, sizeof(fiemap)))
@@ -775,11 +750,6 @@ static int ext4_ioctl_get_es_cache(struct file *filp, unsigned long arg)
if (fiemap.fm_extent_count > FIEMAP_MAX_EXTENTS)
return -EINVAL;

- error = fiemap_check_ranges(sb, fiemap.fm_start, fiemap.fm_length,
- &len);
- if (error)
- return error;
-
fieinfo.fi_flags = fiemap.fm_flags;
fieinfo.fi_extents_max = fiemap.fm_extent_count;
fieinfo.fi_extents_start = ufiemap->fm_extents;
@@ -792,7 +762,8 @@ static int ext4_ioctl_get_es_cache(struct file *filp, unsigned long arg)
if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
filemap_write_and_wait(inode->i_mapping);

- error = ext4_get_es_cache(inode, &fieinfo, fiemap.fm_start, len);
+ error = ext4_get_es_cache(inode, &fieinfo, fiemap.fm_start,
+ fiemap.fm_length);
fiemap.fm_flags = fieinfo.fi_flags;
fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
--
2.26.1

2020-04-27 09:59:47

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 3/8] ext4: split _ext4_fiemap

The fiemap and EXT4_IOC_GET_ES_CACHE cases share almost no code, so split
them into entirely separate functions.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/ext4/extents.c | 72 +++++++++++++++++++++++------------------------
1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 2b4b94542e34d..d2a2a3ba5c44a 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4854,11 +4854,9 @@ static int ext4_fiemap_check_ranges(struct inode *inode, u64 start, u64 *len)
return 0;
}

-static int _ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
- __u64 start, __u64 len, bool from_es_cache)
+int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
+ u64 start, u64 len)
{
- ext4_lblk_t start_blk;
- u32 ext4_fiemap_flags = FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR;
int error = 0;

if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
@@ -4868,10 +4866,7 @@ static int _ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
fieinfo->fi_flags &= ~FIEMAP_FLAG_CACHE;
}

- if (from_es_cache)
- ext4_fiemap_flags &= FIEMAP_FLAG_XATTR;
-
- if (fiemap_check_flags(fieinfo, ext4_fiemap_flags))
+ if (fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR))
return -EBADR;

/*
@@ -4885,40 +4880,20 @@ static int _ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,

if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
fieinfo->fi_flags &= ~FIEMAP_FLAG_XATTR;
- error = iomap_fiemap(inode, fieinfo, start, len,
- &ext4_iomap_xattr_ops);
- } else if (!from_es_cache) {
- error = iomap_fiemap(inode, fieinfo, start, len,
- &ext4_iomap_report_ops);
- } else {
- ext4_lblk_t len_blks;
- __u64 last_blk;
-
- start_blk = start >> inode->i_sb->s_blocksize_bits;
- last_blk = (start + len - 1) >> inode->i_sb->s_blocksize_bits;
- if (last_blk >= EXT_MAX_BLOCKS)
- last_blk = EXT_MAX_BLOCKS-1;
- len_blks = ((ext4_lblk_t) last_blk) - start_blk + 1;
-
- /*
- * Walk the extent tree gathering extent information
- * and pushing extents back to the user.
- */
- error = ext4_fill_es_cache_info(inode, start_blk, len_blks,
- fieinfo);
+ return iomap_fiemap(inode, fieinfo, start, len,
+ &ext4_iomap_xattr_ops);
}
- return error;
-}

-int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
- __u64 start, __u64 len)
-{
- return _ext4_fiemap(inode, fieinfo, start, len, false);
+ return iomap_fiemap(inode, fieinfo, start, len, &ext4_iomap_report_ops);
}

int ext4_get_es_cache(struct inode *inode, struct fiemap_extent_info *fieinfo,
__u64 start, __u64 len)
{
+ ext4_lblk_t start_blk, len_blks;
+ __u64 last_blk;
+ int error = 0;
+
if (ext4_has_inline_data(inode)) {
int has_inline;

@@ -4929,9 +4904,32 @@ int ext4_get_es_cache(struct inode *inode, struct fiemap_extent_info *fieinfo,
return 0;
}

- return _ext4_fiemap(inode, fieinfo, start, len, true);
-}
+ if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
+ error = ext4_ext_precache(inode);
+ if (error)
+ return error;
+ fieinfo->fi_flags &= ~FIEMAP_FLAG_CACHE;
+ }
+
+ if (fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC))
+ return -EBADR;

+ error = ext4_fiemap_check_ranges(inode, start, &len);
+ if (error)
+ return error;
+
+ start_blk = start >> inode->i_sb->s_blocksize_bits;
+ last_blk = (start + len - 1) >> inode->i_sb->s_blocksize_bits;
+ if (last_blk >= EXT_MAX_BLOCKS)
+ last_blk = EXT_MAX_BLOCKS-1;
+ len_blks = ((ext4_lblk_t) last_blk) - start_blk + 1;
+
+ /*
+ * Walk the extent tree gathering extent information
+ * and pushing extents back to the user.
+ */
+ return ext4_fill_es_cache_info(inode, start_blk, len_blks, fieinfo);
+}

/*
* ext4_access_path:
--
2.26.1

2020-04-27 09:59:54

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 6/8] fs: move the fiemap definitions out of fs.h

No need to pull the fiemap definitions into almost every file in the
kernel build.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/bad_inode.c | 1 +
fs/btrfs/extent_io.h | 1 +
fs/cifs/inode.c | 1 +
fs/cifs/smb2ops.c | 1 +
fs/ext2/inode.c | 1 +
fs/ext4/ext4.h | 1 +
fs/f2fs/data.c | 1 +
fs/f2fs/inline.c | 1 +
fs/gfs2/inode.c | 1 +
fs/hpfs/file.c | 1 +
fs/ioctl.c | 1 +
fs/iomap/fiemap.c | 1 +
fs/nilfs2/inode.c | 1 +
fs/overlayfs/inode.c | 1 +
fs/xfs/xfs_iops.c | 1 +
include/linux/fiemap.h | 24 ++++++++++++++++++++++++
include/linux/fs.h | 19 +------------------
include/uapi/linux/fiemap.h | 6 +++---
18 files changed, 43 insertions(+), 21 deletions(-)
create mode 100644 include/linux/fiemap.h

diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index 8035d2a445617..54f0ce4442720 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -15,6 +15,7 @@
#include <linux/time.h>
#include <linux/namei.h>
#include <linux/poll.h>
+#include <linux/fiemap.h>

static int bad_file_open(struct inode *inode, struct file *filp)
{
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 2ed65bd0760ea..817698bc06693 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -5,6 +5,7 @@

#include <linux/rbtree.h>
#include <linux/refcount.h>
+#include <linux/fiemap.h>
#include "ulist.h"

/*
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 390d2b15ef6ef..3f276eb8ca68d 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -25,6 +25,7 @@
#include <linux/freezer.h>
#include <linux/sched/signal.h>
#include <linux/wait_bit.h>
+#include <linux/fiemap.h>

#include <asm/div64.h>
#include "cifsfs.h"
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index f829f4165d38c..09047f1ddfb66 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -12,6 +12,7 @@
#include <linux/uuid.h>
#include <linux/sort.h>
#include <crypto/aead.h>
+#include <linux/fiemap.h>
#include "cifsfs.h"
#include "cifsglob.h"
#include "smb2pdu.h"
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index c885cf7d724b4..0f12a0e8a8d97 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -36,6 +36,7 @@
#include <linux/iomap.h>
#include <linux/namei.h>
#include <linux/uio.h>
+#include <linux/fiemap.h>
#include "ext2.h"
#include "acl.h"
#include "xattr.h"
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index ad2dbf6e49245..06f97a3a943f6 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -36,6 +36,7 @@
#include <crypto/hash.h>
#include <linux/falloc.h>
#include <linux/percpu-rwsem.h>
+#include <linux/fiemap.h>
#ifdef __KERNEL__
#include <linux/compat.h>
#endif
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index cdf2f626bea7a..25abbbb65ba09 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -19,6 +19,7 @@
#include <linux/uio.h>
#include <linux/cleancache.h>
#include <linux/sched/signal.h>
+#include <linux/fiemap.h>

#include "f2fs.h"
#include "node.h"
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 4167e54081518..9686ffea177e7 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -8,6 +8,7 @@

#include <linux/fs.h>
#include <linux/f2fs_fs.h>
+#include <linux/fiemap.h>

#include "f2fs.h"
#include "node.h"
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 70b2d3a1e8668..4842f313a8084 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -17,6 +17,7 @@
#include <linux/crc32.h>
#include <linux/iomap.h>
#include <linux/security.h>
+#include <linux/fiemap.h>
#include <linux/uaccess.h>

#include "gfs2.h"
diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c
index b36abf9cb345a..62959a8e43ad8 100644
--- a/fs/hpfs/file.c
+++ b/fs/hpfs/file.c
@@ -9,6 +9,7 @@

#include "hpfs_fn.h"
#include <linux/mpage.h>
+#include <linux/fiemap.h>

#define BLOCKS(size) (((size) + 511) >> 9)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index f55f53c7824bb..cbc84e23d00bd 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -18,6 +18,7 @@
#include <linux/buffer_head.h>
#include <linux/falloc.h>
#include <linux/sched/signal.h>
+#include <linux/fiemap.h>

#include "internal.h"

diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
index bccf305ea9ce2..fca3dfb9d964a 100644
--- a/fs/iomap/fiemap.c
+++ b/fs/iomap/fiemap.c
@@ -6,6 +6,7 @@
#include <linux/compiler.h>
#include <linux/fs.h>
#include <linux/iomap.h>
+#include <linux/fiemap.h>

struct fiemap_ctx {
struct fiemap_extent_info *fi;
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 671085512e0fd..6e1aca38931f3 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -14,6 +14,7 @@
#include <linux/pagemap.h>
#include <linux/writeback.h>
#include <linux/uio.h>
+#include <linux/fiemap.h>
#include "nilfs.h"
#include "btnode.h"
#include "segment.h"
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index b0d42ece4d7cc..b5fec34105569 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -10,6 +10,7 @@
#include <linux/xattr.h>
#include <linux/posix_acl.h>
#include <linux/ratelimit.h>
+#include <linux/fiemap.h>
#include "overlayfs.h"


diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index f7a99b3bbcf7a..44c353998ac5c 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -25,6 +25,7 @@
#include <linux/posix_acl.h>
#include <linux/security.h>
#include <linux/iversion.h>
+#include <linux/fiemap.h>

/*
* Directories have different lock order w.r.t. mmap_sem compared to regular
diff --git a/include/linux/fiemap.h b/include/linux/fiemap.h
new file mode 100644
index 0000000000000..240d4f7d9116a
--- /dev/null
+++ b/include/linux/fiemap.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_FIEMAP_H
+#define _LINUX_FIEMAP_H 1
+
+#include <uapi/linux/fiemap.h>
+#include <linux/fs.h>
+
+struct fiemap_extent_info {
+ unsigned int fi_flags; /* Flags as passed from user */
+ unsigned int fi_extents_mapped; /* Number of mapped extents */
+ unsigned int fi_extents_max; /* Size of fiemap_extent array */
+ struct fiemap_extent __user *fi_extents_start; /* Start of
+ fiemap_extent array */
+};
+
+int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
+ u64 phys, u64 len, u32 flags);
+int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags);
+
+int generic_block_fiemap(struct inode *inode,
+ struct fiemap_extent_info *fieinfo, u64 start, u64 len,
+ get_block_t *get_block);
+
+#endif /* _LINUX_FIEMAP_H 1 */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3104c6f7527b5..09bcd329c0628 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -24,7 +24,6 @@
#include <linux/capability.h>
#include <linux/semaphore.h>
#include <linux/fcntl.h>
-#include <linux/fiemap.h>
#include <linux/rculist_bl.h>
#include <linux/atomic.h>
#include <linux/shrinker.h>
@@ -48,6 +47,7 @@ struct backing_dev_info;
struct bdi_writeback;
struct bio;
struct export_operations;
+struct fiemap_extent_info;
struct hd_geometry;
struct iovec;
struct kiocb;
@@ -1745,19 +1745,6 @@ extern long compat_ptr_ioctl(struct file *file, unsigned int cmd,
extern void inode_init_owner(struct inode *inode, const struct inode *dir,
umode_t mode);
extern bool may_open_dev(const struct path *path);
-/*
- * VFS FS_IOC_FIEMAP helper definitions.
- */
-struct fiemap_extent_info {
- unsigned int fi_flags; /* Flags as passed from user */
- unsigned int fi_extents_mapped; /* Number of mapped extents */
- unsigned int fi_extents_max; /* Size of fiemap_extent array */
- struct fiemap_extent __user *fi_extents_start; /* Start of
- fiemap_extent array */
-};
-int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
- u64 phys, u64 len, u32 flags);
-int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags);

/*
* This is the "filldir" function type, used by readdir() to let
@@ -3299,10 +3286,6 @@ static inline int vfs_fstat(int fd, struct kstat *stat)
extern const char *vfs_get_link(struct dentry *, struct delayed_call *);
extern int vfs_readlink(struct dentry *, char __user *, int);

-extern int generic_block_fiemap(struct inode *inode,
- struct fiemap_extent_info *fieinfo, u64 start,
- u64 len, get_block_t *get_block);
-
extern struct file_system_type *get_filesystem(struct file_system_type *fs);
extern void put_filesystem(struct file_system_type *fs);
extern struct file_system_type *get_fs_type(const char *name);
diff --git a/include/uapi/linux/fiemap.h b/include/uapi/linux/fiemap.h
index 7a900b2377b60..24ca0c00cae36 100644
--- a/include/uapi/linux/fiemap.h
+++ b/include/uapi/linux/fiemap.h
@@ -9,8 +9,8 @@
* Andreas Dilger <[email protected]>
*/

-#ifndef _LINUX_FIEMAP_H
-#define _LINUX_FIEMAP_H
+#ifndef _UAPI_LINUX_FIEMAP_H
+#define _UAPI_LINUX_FIEMAP_H

#include <linux/types.h>

@@ -67,4 +67,4 @@ struct fiemap {
#define FIEMAP_EXTENT_SHARED 0x00002000 /* Space shared with other
* files. */

-#endif /* _LINUX_FIEMAP_H */
+#endif /* _UAPI_LINUX_FIEMAP_H */
--
2.26.1

2020-04-27 10:00:00

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 4/8] ext4: remove the call to fiemap_check_flags in ext4_fiemap

iomap_fiemap already calls fiemap_check_flags first thing, so this
additional check is redundant.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/ext4/extents.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index d2a2a3ba5c44a..a41ae7c510170 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4866,9 +4866,6 @@ int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
fieinfo->fi_flags &= ~FIEMAP_FLAG_CACHE;
}

- if (fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR))
- return -EBADR;
-
/*
* For bitmap files the maximum size limit could be smaller than
* s_maxbytes, so check len here manually instead of just relying on the
--
2.26.1

2020-04-27 10:00:00

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 8/8] fs: move fiemap range validation into the file systems instances

Replace fiemap_check_flags with a fiemap_validate helpers that also takes
the inode and mapped range, and performs the sanity check and truncation
previously done in fiemap_check_range. This way the validation is inside
the file system itself and thus properly works for the stacked overlayfs
case as well.

Signed-off-by: Christoph Hellwig <[email protected]>
---
Documentation/filesystems/fiemap.txt | 8 ++---
fs/btrfs/inode.c | 2 +-
fs/cifs/smb2ops.c | 3 +-
fs/ext4/extents.c | 5 +--
fs/f2fs/data.c | 3 +-
fs/ioctl.c | 51 +++++++++++-----------------
fs/iomap/fiemap.c | 2 +-
fs/nilfs2/inode.c | 2 +-
fs/ocfs2/extent_map.c | 3 +-
include/linux/fiemap.h | 3 +-
10 files changed, 37 insertions(+), 45 deletions(-)

diff --git a/Documentation/filesystems/fiemap.txt b/Documentation/filesystems/fiemap.txt
index ac87e6fda842b..cf757a9191ef2 100644
--- a/Documentation/filesystems/fiemap.txt
+++ b/Documentation/filesystems/fiemap.txt
@@ -203,15 +203,13 @@ EINTR once fatal signal received.


Flag checking should be done at the beginning of the ->fiemap callback via the
-fiemap_check_flags() helper:
-
-int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags);
+fiemap_validate() helper.

The struct fieinfo should be passed in as received from ioctl_fiemap(). The
set of fiemap flags which the fs understands should be passed via fs_flags. If
-fiemap_check_flags finds invalid user flags, it will place the bad values in
+fiemap_validate finds invalid user flags, it will place the bad values in
fieinfo->fi_flags and return -EBADR. If the file system gets -EBADR, from
-fiemap_check_flags(), it should immediately exit, returning that error back to
+fiemap_validate(), it should immediately exit, returning that error back to
ioctl_fiemap().


diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 320d1062068d3..8382b9edcbb09 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8250,7 +8250,7 @@ static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
{
int ret;

- ret = fiemap_check_flags(fieinfo, BTRFS_FIEMAP_FLAGS);
+ ret = fiemap_validate(inode, fieinfo, start, &len, BTRFS_FIEMAP_FLAGS);
if (ret)
return ret;

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 09047f1ddfb66..bf00329e720e4 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3408,7 +3408,8 @@ static int smb3_fiemap(struct cifs_tcon *tcon,
int i, num, rc, flags, last_blob;
u64 next;

- if (fiemap_check_flags(fei, FIEMAP_FLAG_SYNC))
+ if (fiemap_validate(cfile->dentry->d_inode, fei, start, &len,
+ FIEMAP_FLAG_SYNC))
return -EBADR;

xid = get_xid();
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index a41ae7c510170..4541ff2ecc469 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4908,8 +4908,9 @@ int ext4_get_es_cache(struct inode *inode, struct fiemap_extent_info *fieinfo,
fieinfo->fi_flags &= ~FIEMAP_FLAG_CACHE;
}

- if (fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC))
- return -EBADR;
+ error = fiemap_validate(inode, fieinfo, start, &len, FIEMAP_FLAG_SYNC);
+ if (error)
+ return error;

error = ext4_fiemap_check_ranges(inode, start, &len);
if (error)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 25abbbb65ba09..a33e03ecd5e89 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1825,7 +1825,8 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
return ret;
}

- ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR);
+ ret = fiemap_validate(inode, fieinfo, start, &len,
+ FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR);
if (ret)
return ret;

diff --git a/fs/ioctl.c b/fs/ioctl.c
index cbc84e23d00bd..11e95fa010ae8 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -141,8 +141,11 @@ int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical,
EXPORT_SYMBOL(fiemap_fill_next_extent);

/**
- * fiemap_check_flags - check validity of requested flags for fiemap
+ * fiemap_validate - check validity of requested flags for fiemap
+ * @inode: Inode to operate on
* @fieinfo: Fiemap context passed into ->fiemap
+ * @start: Start of the mapped range
+ * @len: Length of the mapped range, can be truncated by this function.
* @fs_flags: Set of fiemap flags that the file system understands
*
* Called from file system ->fiemap callback. This will compute the
@@ -154,48 +157,38 @@ EXPORT_SYMBOL(fiemap_fill_next_extent);
*
* Returns 0 on success, -EBADR on bad flags.
*/
-int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags)
+int fiemap_validate(struct inode *inode, struct fiemap_extent_info *fieinfo,
+ u64 start, u64 *len, u32 fs_flags)
{
+ u64 maxbytes = inode->i_sb->s_maxbytes;
u32 incompat_flags;

- incompat_flags = fieinfo->fi_flags & ~(FIEMAP_FLAGS_COMPAT & fs_flags);
- if (incompat_flags) {
- fieinfo->fi_flags = incompat_flags;
- return -EBADR;
- }
- return 0;
-}
-EXPORT_SYMBOL(fiemap_check_flags);
-
-static int fiemap_check_ranges(struct super_block *sb,
- u64 start, u64 len, u64 *new_len)
-{
- u64 maxbytes = (u64) sb->s_maxbytes;
-
- *new_len = len;
-
- if (len == 0)
+ if (*len == 0)
return -EINVAL;
-
if (start > maxbytes)
return -EFBIG;

/*
* Shrink request scope to what the fs can actually handle.
*/
- if (len > maxbytes || (maxbytes - len) < start)
- *new_len = maxbytes - start;
+ if (*len > maxbytes || (maxbytes - *len) < start)
+ *len = maxbytes - start;
+

+ incompat_flags = fieinfo->fi_flags & ~(FIEMAP_FLAGS_COMPAT & fs_flags);
+ if (incompat_flags) {
+ fieinfo->fi_flags = incompat_flags;
+ return -EBADR;
+ }
return 0;
}
+EXPORT_SYMBOL(fiemap_validate);

static int ioctl_fiemap(struct file *filp, struct fiemap __user *ufiemap)
{
struct fiemap fiemap;
struct fiemap_extent_info fieinfo = { 0, };
struct inode *inode = file_inode(filp);
- struct super_block *sb = inode->i_sb;
- u64 len;
int error;

if (!inode->i_op->fiemap)
@@ -207,11 +200,6 @@ static int ioctl_fiemap(struct file *filp, struct fiemap __user *ufiemap)
if (fiemap.fm_extent_count > FIEMAP_MAX_EXTENTS)
return -EINVAL;

- error = fiemap_check_ranges(sb, fiemap.fm_start, fiemap.fm_length,
- &len);
- if (error)
- return error;
-
fieinfo.fi_flags = fiemap.fm_flags;
fieinfo.fi_extents_max = fiemap.fm_extent_count;
fieinfo.fi_extents_start = ufiemap->fm_extents;
@@ -224,7 +212,8 @@ static int ioctl_fiemap(struct file *filp, struct fiemap __user *ufiemap)
if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
filemap_write_and_wait(inode->i_mapping);

- error = inode->i_op->fiemap(inode, &fieinfo, fiemap.fm_start, len);
+ error = inode->i_op->fiemap(inode, &fieinfo, fiemap.fm_start,
+ fiemap.fm_length);
fiemap.fm_flags = fieinfo.fi_flags;
fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
@@ -312,7 +301,7 @@ static int __generic_block_fiemap(struct inode *inode,
bool past_eof = false, whole_file = false;
int ret = 0;

- ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
+ ret = fiemap_validate(inode, fieinfo, start, &len, FIEMAP_FLAG_SYNC);
if (ret)
return ret;

diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
index dd04e4added15..eb21a483ee776 100644
--- a/fs/iomap/fiemap.c
+++ b/fs/iomap/fiemap.c
@@ -75,7 +75,7 @@ int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
ctx.fi = fi;
ctx.prev.type = IOMAP_HOLE;

- ret = fiemap_check_flags(fi, FIEMAP_FLAG_SYNC);
+ ret = fiemap_validate(inode, fi, start, &len, FIEMAP_FLAG_SYNC);
if (ret)
return ret;

diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 6e1aca38931f3..93402e7fd2104 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -1006,7 +1006,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
unsigned int blkbits = inode->i_blkbits;
int ret, n;

- ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
+ ret = fiemap_validate(inode, fieinfo, start, &len, FIEMAP_FLAG_SYNC);
if (ret)
return ret;

diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index e3e2d1b2af51a..5b6c2b52a68bd 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -746,7 +746,8 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
struct buffer_head *di_bh = NULL;
struct ocfs2_extent_rec rec;

- ret = fiemap_check_flags(fieinfo, OCFS2_FIEMAP_FLAGS);
+ ret = fiemap_validate(inode, fieinfo, map_start, &map_len,
+ OCFS2_FIEMAP_FLAGS);
if (ret)
return ret;

diff --git a/include/linux/fiemap.h b/include/linux/fiemap.h
index 240d4f7d9116a..a5b328bbb69b6 100644
--- a/include/linux/fiemap.h
+++ b/include/linux/fiemap.h
@@ -13,9 +13,10 @@ struct fiemap_extent_info {
fiemap_extent array */
};

+int fiemap_validate(struct inode *inode, struct fiemap_extent_info *fieinfo,
+ u64 start, u64 *len, u32 fs_flags);
int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
u64 phys, u64 len, u32 flags);
-int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags);

int generic_block_fiemap(struct inode *inode,
struct fiemap_extent_info *fieinfo, u64 start, u64 len,
--
2.26.1

2020-04-27 10:00:10

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 5/8] fs: mark __generic_block_fiemap static

There is no caller left outside of ioctl.c.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/ioctl.c | 4 +---
include/linux/fs.h | 4 ----
2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 282d45be6f453..f55f53c7824bb 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -299,8 +299,7 @@ static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
* If you use this function directly, you need to do your own locking. Use
* generic_block_fiemap if you want the locking done for you.
*/
-
-int __generic_block_fiemap(struct inode *inode,
+static int __generic_block_fiemap(struct inode *inode,
struct fiemap_extent_info *fieinfo, loff_t start,
loff_t len, get_block_t *get_block)
{
@@ -445,7 +444,6 @@ int __generic_block_fiemap(struct inode *inode,

return ret;
}
-EXPORT_SYMBOL(__generic_block_fiemap);

/**
* generic_block_fiemap - FIEMAP for block based inodes
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4f6f59b4f22a8..3104c6f7527b5 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3299,10 +3299,6 @@ static inline int vfs_fstat(int fd, struct kstat *stat)
extern const char *vfs_get_link(struct dentry *, struct delayed_call *);
extern int vfs_readlink(struct dentry *, char __user *, int);

-extern int __generic_block_fiemap(struct inode *inode,
- struct fiemap_extent_info *fieinfo,
- loff_t start, loff_t len,
- get_block_t *get_block);
extern int generic_block_fiemap(struct inode *inode,
struct fiemap_extent_info *fieinfo, u64 start,
u64 len, get_block_t *get_block);
--
2.26.1

2020-04-27 10:01:48

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 7/8] iomap: fix the iomap_fiemap prototype

iomap_fiemap should take u64 start and len arguments, just like the
->fiemap prototype.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/iomap/fiemap.c | 2 +-
include/linux/iomap.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
index fca3dfb9d964a..dd04e4added15 100644
--- a/fs/iomap/fiemap.c
+++ b/fs/iomap/fiemap.c
@@ -66,7 +66,7 @@ iomap_fiemap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
}

int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
- loff_t start, loff_t len, const struct iomap_ops *ops)
+ u64 start, u64 len, const struct iomap_ops *ops)
{
struct fiemap_ctx ctx;
loff_t ret;
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 8b09463dae0db..63db02528b702 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -178,7 +178,7 @@ int iomap_truncate_page(struct inode *inode, loff_t pos, bool *did_zero,
vm_fault_t iomap_page_mkwrite(struct vm_fault *vmf,
const struct iomap_ops *ops);
int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
- loff_t start, loff_t len, const struct iomap_ops *ops);
+ u64 start, u64 len, const struct iomap_ops *ops);
loff_t iomap_seek_hole(struct inode *inode, loff_t offset,
const struct iomap_ops *ops);
loff_t iomap_seek_data(struct inode *inode, loff_t offset,
--
2.26.1