linux-ext4 - [PATCH] fs/ext4: Allow for the last group to be marked as trimmed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231213051635.37731-1-surajjs@amazon.com>
Date: Wed, 13 Dec 2023 16:16:35 +1100
From: Suraj Jitindar Singh <surajjs@...zon.com>
To: <tytso@....edu>
CC: <adilger.kernel@...ger.ca>, <jack@...e.cz>, <linux-ext4@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <sjitindarsingh@...il.com>, "Suraj Jitindar
 Singh" <surajjs@...zon.com>, <stable@...r.kernel.org>
Subject: [PATCH] fs/ext4: Allow for the last group to be marked as trimmed

The ext4 filesystem tracks the trim status of blocks at the group level.
When an entire group has been trimmed then it is marked as such and subsequent
trim invocations with the same minimum trim size will not be attempted on that
group unless it is marked as able to be trimmed again such as when a block is
freed.

Currently the last group can't be marked as trimmed due to incorrect logic
in ext4_last_grp_cluster(). ext4_last_grp_cluster() is supposed to return the
zero based index of the last cluster in a group. This is then used by
ext4_try_to_trim_range() to determine if the trim operation spans the entire
group and as such if the trim status of the group should be recorded.

ext4_last_grp_cluster() takes a 0 based group index, thus the valid values
for grp are 0..(ext4_get_groups_count - 1). Any group index less than
(ext4_get_groups_count - 1) is not the last group and must have
EXT4_CLUSTERS_PER_GROUP(sb) clusters. For the last group we need to calculate
the number of clusters based on the number of blocks in the group. Finally
subtract 1 from the number of clusters as zero based indexing is expected.
Rearrange the function slightly to make it clear what we are calculating
and returning.

Reproducer:
// Create file system where the last group has fewer blocks than blocks per group
$ mkfs.ext4 -b 4096 -g 8192 /dev/nvme0n1 8191
$ mount /dev/nvme0n1 /mnt

Before Patch:
$ fstrim -v /mnt
/mnt: 25.9 MiB (27156480 bytes) trimmed
// Group not marked as trimmed so second invocation still discards blocks
$ fstrim -v /mnt
/mnt: 25.9 MiB (27156480 bytes) trimmed

After Patch:
fstrim -v /mnt
/mnt: 25.9 MiB (27156480 bytes) trimmed
// Group marked as trimmed so second invocation DOESN'T discard any blocks
fstrim -v /mnt
/mnt: 0 B (0 bytes) trimmed

Fixes: 45e4ab320c9b ("ext4: move setting of trimmed bit into ext4_try_to_trim_range()")
Cc: stable@...r.kernel.org # 4.19+
Signed-off-by: Suraj Jitindar Singh <surajjs@...zon.com>
---
 fs/ext4/mballoc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 454d5612641ee..c15d8b6f887dd 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -6731,11 +6731,16 @@ __acquires(bitlock)
 static ext4_grpblk_t ext4_last_grp_cluster(struct super_block *sb,
 					   ext4_group_t grp)
 {
-	if (grp < ext4_get_groups_count(sb))
-		return EXT4_CLUSTERS_PER_GROUP(sb) - 1;
-	return (ext4_blocks_count(EXT4_SB(sb)->s_es) -
-		ext4_group_first_block_no(sb, grp) - 1) >>
-					EXT4_CLUSTER_BITS(sb);
+	unsigned long nr_clusters_in_group;
+
+	if (grp < (ext4_get_groups_count(sb) - 1))
+		nr_clusters_in_group = EXT4_CLUSTERS_PER_GROUP(sb);
+	else
+		nr_clusters_in_group = (ext4_blocks_count(EXT4_SB(sb)->s_es) -
+					ext4_group_first_block_no(sb, grp))
+				       >> EXT4_CLUSTER_BITS(sb);
+
+	return nr_clusters_in_group - 1;
 }

 static bool ext4_trim_interrupted(void)
-- 
2.34.1