From c76c1e5d7d3b043549f69c8dc8d6b878b1db0231 Mon Sep 17 00:00:00 2001
From: Scott Shambarger <devel@shambarger.net>
Date: Thu, 12 May 2022 16:27:26 -0700
Subject: fstrim: Remove all skipped entries before de-duplication

When processing fstab entries, de-duplication is performed based on the
source before all tests on the target have been checked, resulting in
some entries being skipped when a removed duplicate with a different
target would not have been.

The fix is to move all the target checks before the source
de-duplication.

Addresses: #1686
Signed-off-by: Scott Shambarger <devel@shambarger.net>
Addresses: https://bugzilla.redhat.com/show_bug.cgi?id=2165981
---
 sys-utils/fstrim.c | 53 ++++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 23 deletions(-)

diff --git a/sys-utils/fstrim.c b/sys-utils/fstrim.c
index 88397f0ec..0b05e590e 100644
--- a/sys-utils/fstrim.c
+++ b/sys-utils/fstrim.c
@@ -230,8 +230,17 @@ static int is_unwanted_fs(struct libmnt_fs *fs, const char *tgt)
 		return 1;
 	rc = fstatfs(fd, &vfs) != 0 || vfs.f_type == STATFS_AUTOFS_MAGIC;
 	close(fd);
+	if (rc)
+		return 1;
 
-	return rc;
+	/* FITRIM on read-only filesystem can fail, and it can fail */
+	if (access(tgt, W_OK) != 0) {
+		if (errno == EROFS)
+			return 1;
+		if (errno == EACCES)
+			return 1;
+	}
+	return 0;
 }
 
 static int uniq_fs_target_cmp(
@@ -317,6 +326,8 @@ static int fstrim_all_from_file(struct fstrim_control *ctl, const char *filename
 	while (mnt_table_next_fs(tab, itr, &fs) == 0) {
 		const char *src = mnt_fs_get_srcpath(fs),
 			   *tgt = mnt_fs_get_target(fs);
+		char *path;
+		int rc = 1;
 
 		if (!tgt || is_unwanted_fs(fs, tgt)) {
 			mnt_table_remove_fs(tab, fs);
@@ -339,19 +350,6 @@ static int fstrim_all_from_file(struct fstrim_control *ctl, const char *filename
 			mnt_table_remove_fs(tab, fs);
 			continue;
 		}
-	}
-
-	/* de-duplicate by source */
-	mnt_table_uniq_fs(tab, MNT_UNIQ_FORWARD, uniq_fs_source_cmp);
-
-	mnt_reset_iter(itr, MNT_ITER_BACKWARD);
-
-	/* Do FITRIM */
-	while (mnt_table_next_fs(tab, itr, &fs) == 0) {
-		const char *src = mnt_fs_get_srcpath(fs),
-			   *tgt = mnt_fs_get_target(fs);
-		char *path;
-		int rc = 1;
 
 		/* Is it really accessible mountpoint? Not all mountpoints are
 		 * accessible (maybe over mounted by another filesystem) */
@@ -359,20 +357,29 @@ static int fstrim_all_from_file(struct fstrim_control *ctl, const char *filename
 		if (path && streq_paths(path, tgt))
 			rc = 0;
 		free(path);
-		if (rc)
+		if (rc) {
+			mnt_table_remove_fs(tab, fs);
 			continue;	/* overlaying mount */
-
-		/* FITRIM on read-only filesystem can fail, and it can fail */
-		if (access(tgt, W_OK) != 0) {
-			if (errno == EROFS)
-				continue;
-			if (errno == EACCES)
-				continue;
 		}
 
 		if (!is_directory(tgt, 1) ||
-		    !has_discard(src, &wholedisk))
+		    !has_discard(src, &wholedisk)) {
+			mnt_table_remove_fs(tab, fs);
 			continue;
+		}
+	}
+
+	/* de-duplicate by source */
+	mnt_table_uniq_fs(tab, MNT_UNIQ_FORWARD, uniq_fs_source_cmp);
+
+	mnt_reset_iter(itr, MNT_ITER_BACKWARD);
+
+	/* Do FITRIM */
+	while (mnt_table_next_fs(tab, itr, &fs) == 0) {
+		const char *src = mnt_fs_get_srcpath(fs),
+			   *tgt = mnt_fs_get_target(fs);
+		int rc;
+
 		cnt++;
 
 		/*
-- 
2.39.1