From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0687C19067C for ; Tue, 5 Aug 2025 02:51:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1754362313; cv=none; b=LdO6qCrT4UViWzeQSaxGThh4+VPmnWovs8ioWivA3vkWgiJ9DP77TDOdTzSZKpNFQr/Xx3pXdzJflfnHAapHUAXZwAenOGflGfUvfe8WfuZEv+z5B3flTJlfhXEcOe+y8JBsowcy79B4b7y8WPYL0T3bhzCFR16/+FhR8OGHeYk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1754362313; c=relaxed/simple; bh=hIYpjPGTOj1ZFCpNe9DNjh2U9NIYW7pnDQtucuxMe/o=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=fRdvf9eE030RQBx57Y2fPUtC/m9q3OZkJE33p4+aB5KCbfwHZ12X9WNG0cCvLW4a/OcI6bX2lpJAT94jrq4xhPAOoto7sKoHefK89y6eafQnObrrKhj0ZWXu4kYgZaphoE3GJ9uBDXsOJYEr1BEdm/GfpcdMDroZoDscgcaFUVQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=eBMr+Pxi; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="eBMr+Pxi" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4C97AC4CEE7; Tue, 5 Aug 2025 02:51:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1754362312; bh=hIYpjPGTOj1ZFCpNe9DNjh2U9NIYW7pnDQtucuxMe/o=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=eBMr+Pxi93e1EErEztqbXyMumcZn8qQqiq75hvP9d7440nFgYWKbtTz6xWsB3j7Yx uLWuiHYEcUfYcQ8E/DxsrVpyCBaBGcR+wq+4HDHdBvPULqHqyoJRs+rgE4HHeXL9s+ VVs2casHDyWArNgDeGep8hPfhUqk2tDi27oqz+AUW3uq+4rfzY3VOZb6DPqoyGWiG2 XAD/NhX3GQ3ii/RfWf2OuAJat+fFVc7sXhRKjlLdYu3Fjb3uhdWata/qyu4wWIZCaY j7VG80C7F3O1uXV8/2W21By8bLW32MhJtud4X6WJ+p2DNtQrMyAS90JLVGKjjCa0jW /xCBvhlffKskA== Message-ID: <1fa9d0c7-0d6f-4752-879b-f277682e82ae@kernel.org> Date: Tue, 5 Aug 2025 10:51:49 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: chao@kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, Jan Prusakowski Subject: Re: [PATCH v2 3/3] f2fs: fix to zero data after EOF for compressed file correctly To: Jaegeuk Kim References: <20250804014340.2537172-1-chao@kernel.org> <20250804014340.2537172-3-chao@kernel.org> Content-Language: en-US From: Chao Yu In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Jaegeuk, sure, let me change the order. On 8/5/25 10:42, Jaegeuk Kim wrote: > Chao, it seems you made a clean up before this? Can you post it first? > > On 08/04, Chao Yu wrote: >> generic/091 may fail, then it bisects to the bad commit ba8dac350faf >> ("f2fs: fix to zero post-eof page"). >> >> What will cause generic/091 to fail is something like below Testcase #1: >> 1. write 16k as compressed blocks >> 2. truncate to 12k >> 3. truncate to 20k >> 4. verify data in range of [12k, 16k], however data is not zero as >> expected >> >> Script of Testcase #1 >> mkfs.f2fs -f -O extra_attr,compression /dev/vdb >> mount -t f2fs -o compress_extension=* /dev/vdb /mnt/f2fs >> dd if=/dev/zero of=/mnt/f2fs/file bs=12k count=1 >> dd if=/dev/random of=/mnt/f2fs/file bs=4k count=1 seek=3 conv=notrunc >> sync >> truncate -s $((12*1024)) /mnt/f2fs/file >> truncate -s $((20*1024)) /mnt/f2fs/file >> dd if=/mnt/f2fs/file of=/mnt/f2fs/data bs=4k count=1 skip=3 >> od /mnt/f2fs/data >> umount /mnt/f2fs >> >> Analisys: >> in step 2), we will redirty all data pages from #0 to #3 in compressed >> cluster, and zero page #3, >> in step 3), f2fs_setattr() will call f2fs_zero_post_eof_page() to drop >> all page cache post eof, includeing dirtied page #3, >> in step 4) when we read data from page #3, it will decompressed cluster >> and extra random data to page #3, finally, we hit the non-zeroed data >> post eof. >> >> However, the commit ba8dac350faf ("f2fs: fix to zero post-eof page") just >> let the issue be reproduced easily, w/o the commit, it can reproduce this >> bug w/ below Testcase #2: >> 1. write 16k as compressed blocks >> 2. truncate to 8k >> 3. truncate to 12k >> 4. truncate to 20k >> 5. verify data in range of [12k, 16k], however data is not zero as >> expected >> >> Script of Testcase #2 >> mkfs.f2fs -f -O extra_attr,compression /dev/vdb >> mount -t f2fs -o compress_extension=* /dev/vdb /mnt/f2fs >> dd if=/dev/zero of=/mnt/f2fs/file bs=12k count=1 >> dd if=/dev/random of=/mnt/f2fs/file bs=4k count=1 seek=3 conv=notrunc >> sync >> truncate -s $((8*1024)) /mnt/f2fs/file >> truncate -s $((12*1024)) /mnt/f2fs/file >> truncate -s $((20*1024)) /mnt/f2fs/file >> echo 3 > /proc/sys/vm/drop_caches >> dd if=/mnt/f2fs/file of=/mnt/f2fs/data bs=4k count=1 skip=3 >> od /mnt/f2fs/data >> umount /mnt/f2fs >> >> Anlysis: >> in step 2), we will redirty all data pages from #0 to #3 in compressed >> cluster, and zero page #2 and #3, >> in step 3), we will truncate page #3 in page cache, >> in step 4), expand file size, >> in step 5), hit random data post eof w/ the same reason in Testcase #1. >> >> Root Cause: >> In f2fs_truncate_partial_cluster(), after we truncate partial data block >> on compressed cluster, all pages in cluster including the one post eof >> will be dirtied, after another tuncation, dirty page post eof will be >> dropped, however on-disk compressed cluster is still valid, it includes >> invalid data post eof, result in exposing previous data post eof while >> reading. >> >> Fix: >> In f2fs_truncate_partial_cluster(), let change as below to fix: >> - call filemap_write_and_wait_range() to flush dirty page >> - call truncate_pagecache() to drop pages or zero partial page post eof >> - call f2fs_do_truncate_blocks() to truncate non-compress cluster to >> last vali block >> >> Fixes: 3265d3db1f16 ("f2fs: support partial truncation on compressed inode") >> Reported-by: Jan Prusakowski >> Signed-off-by: Chao Yu >> --- >> v2: >> - should dirty & flush all pages in cluster and truncate blocks post eof >> later >> fs/f2fs/compress.c | 20 ++++++++++++++------ >> 1 file changed, 14 insertions(+), 6 deletions(-) >> >> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c >> index e37a7ed801e5..6ad8d3bc6df7 100644 >> --- a/fs/f2fs/compress.c >> +++ b/fs/f2fs/compress.c >> @@ -1245,17 +1245,25 @@ int f2fs_truncate_partial_cluster(struct inode *inode, u64 from, bool lock) >> for (i = (1 << log_cluster_size) - 1; i >= 0; i--) { >> struct folio *folio = page_folio(rpages[i]); >> loff_t start = (loff_t)folio->index << PAGE_SHIFT; >> + loff_t offset = from > start ? from - start : 0; >> >> - if (from > start) { >> - folio_zero_segment(folio, from - start, >> - folio_size(folio)); >> + folio_zero_segment(folio, offset, folio_size(folio)); >> + >> + if (from >= start) >> break; >> - } >> - folio_zero_segment(folio, 0, folio_size(folio)); >> } >> >> f2fs_compress_write_end(inode, fsdata, start_idx, true); >> - return 0; >> + >> + err = filemap_write_and_wait_range(inode->i_mapping, >> + round_down(from, 1 << log_cluster_size << PAGE_SHIFT), >> + LLONG_MAX); >> + if (err) >> + return err; >> + >> + truncate_pagecache(inode, from); >> + >> + return f2fs_do_truncate_blocks(inode, round_up(from, PAGE_SIZE), lock); >> } >> >> static int f2fs_write_compressed_pages(struct compress_ctx *cc, >> -- >> 2.49.0