linux-ext4 - [PATCH] ext4: fix dir

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]

Date:   Fri, 21 Jul 2017 16:49:27 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     tytso@....edu
Cc:     linux-ext4@...r.kernel.org, eggert@...ucla.edu,
        Andreas Dilger <adilger@...ger.ca>
Subject: [PATCH] ext4: fix dir_nlink behaviour

The dir_nlink feature has been enabled by default for new ext4
filesystems since e2fsprogs-1.41 in 2008, and was automatically
enabled by the kernel for older ext4 filesystems since the
dir_nlink feature was added with ext4 in kernel 2.6.28+ when
the subdirectory count exceeded EXT4_LINK_MAX.

We shouldn't really be enabling filesystem features automatically,
as this prevents the administrator from disabling the feature at
format time, or via tune2fs.  This should not affect many users by
this point, but allows limiting subdirectory counts to those that
can strictly fit into i_links_count rather than using "1" to
indicate that the number of links on the directory is not tracked.
This avoids a bug in glibc fts_read() that incorrectly optimizes
the directory traversal for such directories.

This also addresses a minor bug in ext4_inc_count() where i_nlinks
was wrapped at (EXT4_LINK_MAX - 1) links rather than allowing the
full EXT4_LINK_MAX links on the parent directory (including "."
and "..") before storing i_links_count = 1.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196405
Signed-off-by: Andreas Dilger <adilger@...ger.ca>
---
 fs/ext4/ext4.h  |  3 ++-
 fs/ext4/namei.c | 21 ++++++++++++---------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 8e80461..e163b87 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2016,7 +2016,8 @@ static inline __le16 ext4_rec_len_to_disk(unsigned len, unsigned blocksize)
 
 #define is_dx(dir) (ext4_has_feature_dir_index((dir)->i_sb) && \
 		    ext4_test_inode_flag((dir), EXT4_INODE_INDEX))
-#define EXT4_DIR_LINK_MAX(dir) (!is_dx(dir) && (dir)->i_nlink >= EXT4_LINK_MAX)
+#define EXT4_DIR_LINK_MAX(dir) unlikely((dir)->i_nlink >= EXT4_LINK_MAX && \
+		    !(ext4_has_feature_dir_nlink((dir)->i_sb) && is_dx(dir)))
 #define EXT4_DIR_LINK_EMPTY(dir) ((dir)->i_nlink == 2 || (dir)->i_nlink == 1)
 
 /* Legal values for the dx_root hash_version field: */
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index b81f7d4..566c149 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2351,19 +2351,22 @@ static int ext4_delete_entry(handle_t *handle,
 }
 
 /*
- * DIR_NLINK feature is set if 1) nlinks > EXT4_LINK_MAX or 2) nlinks == 2,
- * since this indicates that nlinks count was previously 1.
+ * Set directory link count to 1 if nlinks > EXT4_LINK_MAX, or if nlinks == 2
+ * since this indicates that nlinks count was previously 1 to avoid overflowing
+ * the 16-bit i_links_count field on disk.  Directories with i_nlink == 1 mean
+ * that subdirectory link counts are not being maintained accurately.
+ *
+ * The caller has already checked for i_nlink overflow in case the DIR_LINK
+ * feature is not enabled and returned -EMLINK.  The is_dx() check is a proxy
+ * for checking S_ISDIR(inode) (since the INODE_INDEX feature will not be set
+ * on regular files) and to avoid creating huge/slow non-HTREE directories.
  */
 static void ext4_inc_count(handle_t *handle, struct inode *inode)
 {
 	inc_nlink(inode);
-	if (is_dx(inode) && inode->i_nlink > 1) {
-		/* limit is 16-bit i_links_count */
-		if (inode->i_nlink >= EXT4_LINK_MAX || inode->i_nlink == 2) {
-			set_nlink(inode, 1);
-			ext4_set_feature_dir_nlink(inode->i_sb);
-		}
-	}
+	if (is_dx(inode) &&
+	    (inode->i_nlink > EXT4_LINK_MAX || inode->i_nlink == 2))
+		set_nlink(inode, 1);
 }
 
 /*
-- 
1.8.0