btrfs: fix off-by-one end related to inode_need_compress()

In most cases btrfs uses @end as the inclusive end bytenr for a range,
and this applies to inode_need_compress().

However we have several sites not following the inclusive bytenr:

- run_delalloc_inline()
  Which assigned @blocksize as @end for inode_need_compress()

  This makes inode_need_compress() always skip the disk_i_size check.

- heuristic_collect_sample()
  Which assigned "start + BTRFS_MAX_UNCOMPRESSED" to @end, which is
  the exclusive bytenr.

Neither is really causing any real problem, as
heuristic_collect_sample() has proper checks to avoid reading anything
beyond @end, and the sampling read size is 16 bytes, so it has enough
headroom to handle that off-by-one problem.

But still I do not like anything out of the common scheme, so fix the
off-by-one @end for both call sites, and add extra ASSERT()s to catch
such unaligned parameters.

Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 228d1cdd7c2950231b6cfe479ca098ce5b12b248..7c1c018dbd6fd7bf3b02cf79f5382e92e567e75a 100644 (file)
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1488,11 +1488,14 @@ static bool sample_repeated_patterns(struct heuristic_ws *ws)
 static void heuristic_collect_sample(struct inode *inode, u64 start, u64 end,
                                     struct heuristic_ws *ws)
 {
+       const u32 blocksize = BTRFS_I(inode)->root->fs_info->sectorsize;
        struct folio *folio;
        u64 index, index_end;
        u32 i, curr_sample_pos;
        u8 *in_data;
 
+       ASSERT(IS_ALIGNED(start, blocksize) && IS_ALIGNED(end + 1, blocksize));
+
        /*
         * Compression handles the input data by chunks of 128KiB
         * (defined by BTRFS_MAX_UNCOMPRESSED)
@@ -1502,18 +1505,14 @@ static void heuristic_collect_sample(struct inode *inode, u64 start, u64 end,
         * MAX_SAMPLE_SIZE - calculated under assumption that heuristic will
         * process no more than BTRFS_MAX_UNCOMPRESSED at a time.
         */
-       if (end - start > BTRFS_MAX_UNCOMPRESSED)
-               end = start + BTRFS_MAX_UNCOMPRESSED;
+       if (end + 1 - start > BTRFS_MAX_UNCOMPRESSED)
+               end = start + BTRFS_MAX_UNCOMPRESSED - 1;
 
        index = start >> PAGE_SHIFT;
        index_end = end >> PAGE_SHIFT;
 
-       /* Don't miss unaligned end */
-       if (!PAGE_ALIGNED(end))
-               index_end++;
-
        curr_sample_pos = 0;
-       while (index < index_end) {
+       while (index <= index_end) {
                folio = filemap_get_folio(inode->i_mapping, index);
                ASSERT(!IS_ERR(folio));
                in_data = kmap_local_folio(folio,
@@ -1522,7 +1521,7 @@ static void heuristic_collect_sample(struct inode *inode, u64 start, u64 end,
                i = start % PAGE_SIZE;
                while (i < PAGE_SIZE - SAMPLING_READ_SIZE) {
                        /* Don't sample any garbage from the last page */
-                       if (start > end - SAMPLING_READ_SIZE)
+                       if (start > end + 1 - SAMPLING_READ_SIZE)
                                break;
                        memcpy(&ws->sample[curr_sample_pos], &in_data[i],
                                        SAMPLING_READ_SIZE);

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a85a7c561cf8e9842e71df875a3c90690276ffb8..79f2181dc6310ac0418d51b5c919a1673785c99b 100644 (file)
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -730,6 +730,9 @@ static inline int inode_need_compress(struct btrfs_inode *inode, u64 start,
                                      u64 end, bool check_inline)
 {
        struct btrfs_fs_info *fs_info = inode->root->fs_info;
+       const u32 blocksize = fs_info->sectorsize;
+
+       ASSERT(IS_ALIGNED(start, blocksize) && IS_ALIGNED(end + 1, blocksize));
 
        if (unlikely(!btrfs_inode_can_compress(inode))) {
                DEBUG_WARN("BTRFS: unexpected compression for ino %llu", btrfs_ino(inode));
@@ -2331,7 +2334,7 @@ static int run_delalloc_inline(struct btrfs_inode *inode, struct folio *locked_f
        btrfs_check_folio_write_protected(locked_folio);
 
        if (btrfs_inode_can_compress(inode) &&
-           inode_need_compress(inode, 0, blocksize, true)) {
+           inode_need_compress(inode, 0, blocksize - 1, true)) {
                if (inode->defrag_compress > 0 &&
                    inode->defrag_compress < BTRFS_NR_COMPRESS_TYPES) {
                        compress_type = inode->defrag_compress;