linux-ext4 - Re: e2fsck on encrypted directories

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170801143343.7kphdxfvw7qzth3z@thunk.org>
Date:   Tue, 1 Aug 2017 10:33:43 -0400
From:   Theodore Ts'o <tytso@....edu>
To:     Sahitya Tummala <stummala@...eaurora.org>
Cc:     linux-ext4@...r.kernel.org
Subject: Re: e2fsck on encrypted directories

On Fri, Jul 28, 2017 at 03:32:24PM +0530, Sahitya Tummala wrote:
> Hi Ted,
> 
> I got a question on usage of e2fsck with -D option on EXT4 FS, where
> file based encryption is enabled. Is this option supposed to work on
> encrypted directories?
> 
> I have encountered a case where e2fsck gets stuck
> in duplicate_search_and_fix() due to strncmp checks done in this function
> on encrypted file names, which may contain NUL characters.
> 
> Also, even without -D option passed, there can be a case of duplicate
> entries (i.e.,
> ctx->dirs_to_hash is not NULL). In this case too we may see the same issue
> in duplicate_search_and_fix() if the directory is encrypted.
> Is my understanding correct? Please share your comments.

Thanks, your understanding is indeed correct.  Thanks for pointing
this out.  The following patch should fix the problem; can you
confirm, if you have a convenient test case?

					- Ted

>From 5b8d7c51e51aef7879150b07f3967da4028862f3 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <tytso@....edu>
Date: Tue, 1 Aug 2017 10:26:11 -0400
Subject: [PATCH] e2fsck: fix e2fsck -D for encrypted directories

If the directory entry is encrypted there may be embedded NUL
characters; hence, we should use memcmp instead of strncmp when
comparing strings.  Otherwise, e2fsck can erroneously report that a
directory have duplicant entries when doing an e2fsck -D check.

Reported-by: Sahitya Tummala <stummala@...eaurora.org>
Signed-off-by: Theodore Ts'o <tytso@....edu>
---
 e2fsck/rehash.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/e2fsck/rehash.c b/e2fsck/rehash.c
index 22a58f333..77129e50d 100644
--- a/e2fsck/rehash.c
+++ b/e2fsck/rehash.c
@@ -219,7 +219,7 @@ static EXT2_QSORT_TYPE name_cmp(const void *a, const void *b)
 	if (min_len > he_b_len)
 		min_len = he_b_len;
 
-	ret = strncmp(he_a->dir->name, he_b->dir->name, min_len);
+	ret = memcmp(he_a->dir->name, he_b->dir->name, min_len);
 	if (ret == 0) {
 		if (he_a_len > he_b_len)
 			ret = 1;
@@ -386,7 +386,7 @@ static int duplicate_search_and_fix(e2fsck_t ctx, ext2_filsys fs,
 		if (!ent->dir->inode ||
 		    (ext2fs_dirent_name_len(ent->dir) !=
 		     ext2fs_dirent_name_len(prev->dir)) ||
-		    strncmp(ent->dir->name, prev->dir->name,
+		    memcmp(ent->dir->name, prev->dir->name,
 			     ext2fs_dirent_name_len(ent->dir)))
 			continue;
 		pctx.dirent = ent->dir;
@@ -404,7 +404,7 @@ static int duplicate_search_and_fix(e2fsck_t ctx, ext2_filsys fs,
 			if ((i==j) ||
 			    (new_len !=
 			     (unsigned) ext2fs_dirent_name_len(fd->harray[j].dir)) ||
-			    strncmp(new_name, fd->harray[j].dir->name, new_len))
+			    memcmp(new_name, fd->harray[j].dir->name, new_len))
 				continue;
 			mutate_name(new_name, &new_len);
 
-- 
2.11.0.rc0.7.gbe5a750