linux-ext4 - Re: linux: ext4 corruption with symlinks

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: <87tthq37d4.fsf@brahms.olymp>
Date: Tue, 18 Jun 2024 10:52:55 +0100
From: Luis Henriques <luis.henriques@...ux.dev>
To: Luis Henriques <luis.henriques@...ux.dev>
Cc: Ben Hutchings <ben@...adent.org.uk>,  linux-ext4@...r.kernel.org,
  Hervé
 Werner <dud225@...mail.com>,  Diederik de Haas <didi.debian@...ow.org>,
  1039883@...s.debian.org,  Salvatore Bonaccorso <carnil@...ian.org>,
  harshad shirwadkar <harshadshirwadkar@...il.com>
Subject: Re: linux: ext4 corruption with symlinks

On Fri 14 Jun 2024 05:18:45 PM +01, Luis Henriques wrote;
[...}
>>
>> I can also reproduce this error message using the above script and:
>>
>> - Linux 6.10-rc2
>> - A 2 GiB loopback devic instead of /dev/sdb
>>
>> I bisected this back to:
>>
>> commit 9725958bb75cdfa10f2ec11526fdb23e7485e8e4
>> Author: Xin Yin <yinxin.x@...edance.com>
>> Date:   Thu Dec 23 11:23:37 2021 +0800
>>  
>>     ext4: fast commit may miss tracking unwritten range during ftruncate
>>
>> It is still possible to cleanly revert that commit from 6.10-rc2, and
>> doing so removes the error message.
>
> Because I recently fixed an issue in the fast commit code[1] I was hoping
> that you were hitting the same bug.  I've executed the reproducer with the
> fix (which hasn't been merged yet) and realised it's definitely a
> different problem.
>
> Debugged the issue a bit, it seems to be related with the fact that
> ext4_fc_write_inode_data() isn't able to cope with the fact that
> 'ei->i_fc_lblk_len' is set to EXT_MAX_BLOCKS.

OK, I've looked into this again.  And something I didn't pay attention
before was that the filesystem was created with both fast_commit *and*
inline_data features.  And after some more debugging, I _think_ the patch
bellow should be the fix for this bug.

If I understand it correctly, when an inode has inlined data it means that
there's no inode data to be written and this case should be handled as if
the inode length was zero.

I'll send out a patch later after running a few more tests just to make
sure it doesn't break something else.  But it would awesome if you could
test it too.

Cheers,
-- 
Luís

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 87c009e0c59a..c56b39a51865 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -897,7 +897,7 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
 	int ret;
 
 	mutex_lock(&ei->i_fc_lock);
-	if (ei->i_fc_lblk_len == 0) {
+	if ((ei->i_fc_lblk_len == 0) || (ext4_has_inline_data(inode))) {
 		mutex_unlock(&ei->i_fc_lock);
 		return 0;
 	}