linux-kernel - Re: [PATCH v2] f2fs: compress: don't redirty sparse cluster during {,de}compress

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: <8530b8c3-ba11-42f8-970f-1cd5d729fc0f@kernel.org>
Date: Mon, 16 Dec 2024 23:38:48 +0800
From: Chao Yu <chao@...nel.org>
To: Jaegeuk Kim <jaegeuk@...nel.org>, Dan Carpenter <dan.carpenter@...aro.org>
Cc: Chao Yu <chao@...nel.org>, Yeongjin Gil <youngjin.gil@...sung.com>,
 daehojeong@...gle.com, linux-f2fs-devel@...ts.sourceforge.net,
 linux-kernel@...r.kernel.org, Sungjong Seo <sj1557.seo@...sung.com>,
 Sunmin Jeong <s_min.jeong@...sung.com>, Jaewook Kim <jw5454.kim@...sung.com>
Subject: Re: [PATCH v2] f2fs: compress: don't redirty sparse cluster during
 {,de}compress

On 2024/12/13 0:11, Jaegeuk Kim wrote:
> On 12/12, Dan Carpenter wrote:
>> On Mon, Aug 19, 2024 at 05:34:30PM +0900, Yeongjin Gil wrote:
>>> In f2fs_do_write_data_page, when the data block is NULL_ADDR, it skips
>>> writepage considering that it has been already truncated.
>>> This results in an infinite loop as the PAGECACHE_TAG_TOWRITE tag is not
>>> cleared during the writeback process for a compressed file including
>>> NULL_ADDR in compress_mode=user.
>>>
>>> This is the reproduction process:
>>>
>>> 1. dd if=/dev/zero bs=4096 count=1024 seek=1024 of=testfile
>>> 2. f2fs_io compress testfile
>>> 3. dd if=/dev/zero bs=4096 count=1 conv=notrunc of=testfile
>>> 4. f2fs_io decompress testfile
>>>
>>> To prevent the problem, let's check whether the cluster is fully
>>> allocated before redirty its pages.
>>>
>>
>> We were discussing how to detect these sorts of things in the future.
>> Presumably a user found this by chance?  Xfstests has two tests which deal
>> with compression tests/f2fs/002 and tests/f2fs/007.  But it feels like
>> xfstests is not really the right place for this sort of thing, it would
>> be better as part of some sort of fuzz testing.
>>
>> What do you think?
> 
> Yeah, agreed that we must have tests to catch this. One way may be creating
> some basic disk images having some possible valid layout to see f2fs can
> work as intended. I feel we can put it in xfstests as wel?
 > > Chao, thoughts?

Hi Jaegeuk, Dan,

I'd like to continue to add regression testcases like f2fs/[003-008]
to xfstests, so that, we can make sure last change of f2fs won't cause
any regression by checking w/ those testcases.

Recently, Sheng Yong has introduced a new tool named inject.f2fs [1], and
then, built an auto testsuit in f2fs-tools based on inject.f2fs [2], w/
this testsuit, we can construct image by injecting specified fields, and
check the image w/ fsck to see whether result is as expected.

IMO, it's fine to add new testcases into testsuit to check whether f2fs'
behavior is as expected on constructed image w/ valid data layout.

So, regression testcase goes into xfstests, and fuzz/common image testcase
goes into testsuit of f2fs-tools? what do you think?

[1] "f2fs-tools: introduce inject.f2fs" https://lore.kernel.org/linux-f2fs-devel/20240704025740.527171-1-shengyong@oppo.com/
[2] "f2fs-tools: add testcases" https://lore.kernel.org/linux-f2fs-devel/20241029120956.4186731-1-shengyong@oppo.com/

Thanks,

> 
>>
>> regards,
>> dan carpenter