linux-ext4 - Re: [PATCH] ext4: fix reading new encrypted symlinks on no-journal filesystems

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]

Date:   Thu, 1 Dec 2016 14:27:05 -0500
From:   Theodore Ts'o <tytso@....edu>
To:     Andreas Dilger <adilger@...ger.ca>
Cc:     Eric Biggers <ebiggers@...gle.com>,
        linux-ext4 <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] ext4: fix reading new encrypted symlinks on no-journal
 filesystems

So in the long term I think we can move to using i_size to determine
fast symlinks, but I think there's a bigger issue hiding here, which
is that we shouldn't be using delayed allocation for symlinks in the
first place.  In the first place, symlinks will never be more than a
block, so there's no advantage in using delalloc.  In the second
place, it means that on a crash the symlink could invalid (zero
length) --- and on a commit the symlink should be commited to disk.

Eric, do you have a test case which verifies this?  Normally I would
think this rarely happens because the dentry cache should hide this
particular issue.  I think a simpler fix up, which also avoids the
"symlink could be lost on a crash" problem, is this:

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index b48ca0392b9c..4ffb680780e5 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2902,7 +2902,8 @@ static int ext4_da_write_begin(struct file *file, struct address_space *mapping,

 	index = pos >> PAGE_SHIFT;

-	if (ext4_nonda_switch(inode->i_sb)) {
+	if (ext4_nonda_switch(inode->i_sb) ||
+	    S_ISLNK(inode->i_mode)) {
 		*fsdata = (void *)FALL_BACK_TO_NONDELALLOC;
 		return ext4_write_begin(file, mapping, pos,
 					len, flags, pagep, fsdata);

					     	    - Ted

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html