[PATCH v2 2/2] befs: Fix allocation group consistency check

[Axel Dörfler]

befs_check_sb() complains if (1 << ag_shift) != blocks_per_ag, but the
two fields do not describe the same thing: ag_shift is the log2 of the
number of blocks in an allocation group (which is also how befs uses it
to convert between inode addresses and block numbers), while
blocks_per_ag is the number of bitmap blocks per allocation group.

As a result, the check triggers on basically every valid volume.

Replace it with the check the BFS reference implementation uses: the
number of allocation groups must match the number of blocks divided by
the allocation group size, rounded up. Also reject ag_shift values
larger than 16: the start of a block_run within an allocation group is
only 16 bits wide, so larger allocation groups cannot be addressed, and
such values could otherwise make the shifts in iaddr2blockno() and
blockno2iaddr(), and the division in the new check, misbehave.

Like the other structural checks in befs_check_sb(), a mismatch now
fails the mount instead of only printing an error.

Signed-off-by: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
---

Notes:
    v2:
    - Replace the incorrect check with the one from the BFS reference
      implementation instead of just removing it, and limit ag_shift to
      what a block_run can address.
    - Explain what ag_shift and blocks_per_ag actually mean.

 fs/befs/super.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/fs/befs/super.c b/fs/befs/super.c
index 7c50025c99d8..6496b4d9338c 100644
--- a/fs/befs/super.c
+++ b/fs/befs/super.c
@@ -99,12 +99,24 @@ befs_check_sb(struct super_block *sb)
 		return BEFS_ERR;
 	}
 
-
-	/* ag_shift also encodes the same information as blocks_per_ag in a
-	 * different way, non-fatal consistency check
+	/*
+	 * ag_shift is the log2 of the number of blocks in an allocation
+	 * group, so it determines how many allocation groups are needed to
+	 * cover all blocks of the file system. Since the start of a block
+	 * run within an allocation group is only 16 bits wide, an allocation
+	 * group cannot have more than 65536 blocks.
 	 */
-	if ((1 << befs_sb->ag_shift) != befs_sb->blocks_per_ag)
-		befs_error(sb, "ag_shift disagrees with blocks_per_ag.");
+
+	if (befs_sb->ag_shift < 1 || befs_sb->ag_shift > 16) {
+		befs_error(sb, "invalid ag_shift: %u", befs_sb->ag_shift);
+		return BEFS_ERR;
+	}
+
+	if (DIV_ROUND_UP_ULL(befs_sb->num_blocks, 1ULL << befs_sb->ag_shift) !=
+	    befs_sb->num_ags) {
+		befs_error(sb, "num_ags disagrees with num_blocks and ag_shift");
+		return BEFS_ERR;
+	}
 
 	if (befs_sb->log_start != befs_sb->log_end ||
 	    befs_sb->flags == BEFS_DIRTY) {
-- 
2.56.0