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

Message-ID: <877cem2xtc.fsf@brahms.olymp>
Date: Tue, 18 Jun 2024 14:19:11 +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 Tue 18 Jun 2024 10:52:55 AM +01, Luis Henriques wrote;

> 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.

Hmm... looking closer, this patch seems to work with this specific test
script, but only because file data is probably small enough to fit in
inode->i_block.  However, it may actually truncate files that have inlined
data if the file data is also stored in the extended attribute space
(i.e. > 60 bytes).

So, the correct fix is probably something like the below patch (which I'll
send out soon).

Cheers,
-- 
Luís

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 87c009e0c59a..d3a67bc06d10 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -649,6 +649,12 @@ void ext4_fc_track_range(handle_t *handle, struct inode *inode, ext4_lblk_t star
 	if (ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE))
 		return;
 
+	if (ext4_has_inline_data(inode)) {
+		ext4_fc_mark_ineligible(inode->i_sb, EXT4_FC_REASON_XATTR,
+					handle);
+		return;
+	}
+
 	args.start = start;
 	args.end = end;