Re: Bug#1039883: linux-image-6.3.0-1-amd64: ext4 corruption with symlinks

Re: Bug#1039883: linux-image-6.3.0-1-amd64: ext4 corruption with symlinks

[Salvatore Bonaccorso]

Control: tags -1 + upstream

Hi,

On Thu, Jun 29, 2023 at 10:38:07AM +0200, dud225 wrote:
> Package: linux-image-6.3.0-1-amd64
> Version: linux-image-6.3.0-1-amd64
> Severity: important
> Tags: upstream
> X-Debbugs-Cc: dud225@hotmail.com
> 
> Hello
> 
> I've stored data on a USB external hard drive using ext4 over LUKS2 and I'm getting the following error:
> 	kernel: EXT4-fs error (device dm-11): ext4_map_blocks:607: inode #8159552: block 959787320: comm git-annex:w: lblock 0 mapped>
> 
> I then stumbled upon that kernel bug [1] which matches my case as git-annex is making heavy use of symlinks. However I've faced this issue on the kernel 6.3.0 (linux-image-6.3.0-1-amd64 version 6.3.7-1) so it doesn't look to be actually addressed.
> 
> I've got 3 disks, 2 HDDs and 1 SSD, and oddly the failure only happens on the HDD, the SSD is running fine.
> After reformatting my HDD without the inline_data feature, the issue has disappeared.
> 
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=216317

I believe this should be reported upstream, doing so now to get some
ideas from Ted and Andreas. 

Ted, Andreas the above was reported in Debian at
https://bugs.debian.org/1039883 .

Any insights here?

Regards,
Salvatore

Re: linux: ext4 corruption with symlinks

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]

On Sun, 5 Nov 2023 16:12:41 +0000 Hervé Werner <dud225@hotmail.com>
wrote:
> Hello
> 
> I'm sorry for the delay.
> 
> > Are you able to reliably preoeduce the issue and can bisect it to
> > the introducing commit?
> I faced this issue on real data but I struggled to find a reliable
> scenario to reproduce it. Here is what I just came up with:
>   sudo mkfs -t ext4 -O fast_commit,inline_data /dev/sdb
>   sudo mount /dev/sdb /mnt/
>   sudo install -d -o myuser /mnt/annex
>   cd /mnt/annex
>   git init && git annex init
>   for i in {1..2}; do
>     for i in {1..10000}; do
>       dd if=/dev/urandom of=file-${i} bs=1K count=1 2>/dev/null
>     done
>     git annex add -J cpus . >/dev/null && git annex sync -J cpus && git annex fsck -J cpus >/dev/null
>     git rm * && git annex sync  && git annex dropunused all
>   done
> 
> Then at some point the following error appears:
>   EXT4-fs error (device sdb): ext4_map_blocks:577: inode #3942343: block 4: comm git-annex:w: lblock 1 mapped to illegal pblock 4 (length 1)
[...]

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

Ben.

-- 
Ben Hutchings
Reality is just a crutch for people who can't handle science fiction.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: linux: ext4 corruption with symlinks

[Luis Henriques]

On Mon 10 Jun 2024 06:03:58 PM +02, Ben Hutchings wrote;

> On Sun, 5 Nov 2023 16:12:41 +0000 Hervé Werner <dud225@hotmail.com>
> wrote:
>> Hello
>> 
>> I'm sorry for the delay.
>> 
>> > Are you able to reliably preoeduce the issue and can bisect it to
>> > the introducing commit?
>> I faced this issue on real data but I struggled to find a reliable
>> scenario to reproduce it. Here is what I just came up with:
>>   sudo mkfs -t ext4 -O fast_commit,inline_data /dev/sdb
>>   sudo mount /dev/sdb /mnt/
>>   sudo install -d -o myuser /mnt/annex
>>   cd /mnt/annex
>>   git init && git annex init
>>   for i in {1..2}; do
>>     for i in {1..10000}; do
>>       dd if=/dev/urandom of=file-${i} bs=1K count=1 2>/dev/null
>>     done
>>     git annex add -J cpus . >/dev/null && git annex sync -J cpus && git annex fsck -J cpus >/dev/null
>>     git rm * && git annex sync  && git annex dropunused all
>>   done
>> 
>> Then at some point the following error appears:
>>   EXT4-fs error (device sdb): ext4_map_blocks:577: inode #3942343: block 4: comm git-annex:w: lblock 1 mapped to illegal pblock 4 (length 1)
> [...]
>
> 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@bytedance.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.

I'm CC'ing Harshad, maybe he has some idea.

[1] https://lore.kernel.org/all/20240529092030.9557-2-luis.henriques@linux.dev

Cheers,
-- 
Luís

Re: linux: ext4 corruption with symlinks

[Luis Henriques]

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@bytedance.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;
 	}

Re: linux: ext4 corruption with symlinks

[Luis Henriques]

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@bytedance.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;