Hello Qu,
On 12.12.2024 21:37, Qu Wenruo wrote:
>
>
> 在 2024/12/13 00:20, Mikhail Zaslonko 写道:
>> Since the input data length passed to zlib_compress_folios() can be
>> arbitrary, always setting strm.avail_in to a multiple of PAGE_SIZE may
>> cause read-in bytes to exceed the input range. Currently this triggers
>> an assert in btrfs_compress_folios() on the debug kernel. But it may
>> potentially lead to data corruption.
>
> Mind to provide the real world ASSERT() call trace?
Here is the call trace triggered by one of our tests (wasn't sure whether to include it to the commit message):
[ 2928.542300] BTRFS: device fsid 98138b99-a1bc-47cd-9b77-9e64fbba11de devid 1 transid 55 /dev/dasdc1 (94:9) scanned by mount (2000)
[ 2928.543029] BTRFS info (device dasdc1): first mount of filesystem 98138b99-a1bc-47cd-9b77-9e64fbba11de
[ 2928.543051] BTRFS info (device dasdc1): using crc32c (crc32c-vx) checksum algorithm
[ 2928.543058] BTRFS info (device dasdc1): using free-space-tree
[ 2964.842146] assertion failed: *total_in <= orig_len, in fs/btrfs/compression.c:1041
[ 2964.842226] ------------[ cut here ]------------
[ 2964.842229] kernel BUG at fs/btrfs/compression.c:1041!
[ 2964.842306] monitor event: 0040 ilc:2 [#1] PREEMPT SMP
[ 2964.842314] Modules linked in: nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat n
f_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set nf_tables pkey_pckmo s390_trng rng_core vfio_ccw mdev vfio_iommu_type1 vfio sch_fq_codel drm loop i2c_co
re dm_multipath drm_panel_orientation_quirks nfnetlink vsock_loopback vmw_vsock_virtio_transport_common vsock lcs ctcm fsm zfcp scsi_transport_fc ghash_s390 prn
g chacha_s390 aes_s390 des_s390 libdes sha3_512_s390 sha3_256_s390 sha512_s390 sha256_s390 sha1_s390 sha_common scsi_dh_rdac scsi_dh_emc scsi_dh_alua pkey autof
s4 ecdsa_generic ecc
[ 2964.842387] CPU: 16 UID: 0 PID: 325 Comm: kworker/u273:3 Not tainted 6.13.0-20241204.rc1.git6.fae3b21430ca.300.fc41.s390x+debug #1
[ 2964.842406] Hardware name: IBM 3931 A01 703 (z/VM 7.4.0)
[ 2964.842409] Workqueue: btrfs-delalloc btrfs_work_helper
[ 2964.842420] Krnl PSW : 0704d00180000000 0000021761df6538 (btrfs_compress_folios+0x198/0x1a0)
[ 2964.842426] R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:1 PM:0 RI:0 EA:3
[ 2964.842430] Krnl GPRS: 0000000080000000 0000000000000001 0000000000000047 0000000000000000
[ 2964.842433] 0000000000000006 ffffff01757bb000 000001976232fcc0 000000000000130c
[ 2964.842436] 000001976232fcd0 000001976232fcc8 00000118ff4a0e30 0000000000000001
[ 2964.842438] 00000111821ab400 0000011100000000 0000021761df6534 000001976232fb58
[ 2964.842446] Krnl Code: 0000021761df6528: c020006f5ef4 larl %r2,0000021762be2310
[ 2964.842446] 0000021761df652e: c0e5ffbd09d5 brasl %r14,00000217615978d8
[ 2964.842446] #0000021761df6534: af000000 mc 0,0
[ 2964.842446] >0000021761df6538: 0707 bcr 0,%r7
[ 2964.842446] 0000021761df653a: 0707 bcr 0,%r7
[ 2964.842446] 0000021761df653c: 0707 bcr 0,%r7
[ 2964.842446] 0000021761df653e: 0707 bcr 0,%r7
[ 2964.842446] 0000021761df6540: c004004bb7ec brcl 0,000002176276d518
[ 2964.842463] Call Trace:
[ 2964.842465] [<0000021761df6538>] btrfs_compress_folios+0x198/0x1a0
[ 2964.842468] ([<0000021761df6534>] btrfs_compress_folios+0x194/0x1a0)
[ 2964.842708] [<0000021761d97788>] compress_file_range+0x3b8/0x6d0
[ 2964.842714] [<0000021761dcee7c>] btrfs_work_helper+0x10c/0x160
[ 2964.842718] [<0000021761645760>] process_one_work+0x2b0/0x5d0
[ 2964.842724] [<000002176164637e>] worker_thread+0x20e/0x3e0
[ 2964.842728] [<000002176165221a>] kthread+0x15a/0x170
[ 2964.842732] [<00000217615b859c>] __ret_from_fork+0x3c/0x60
[ 2964.842736] [<00000217626e72d2>] ret_from_fork+0xa/0x38
[ 2964.842744] INFO: lockdep is turned off.
[ 2964.842746] Last Breaking-Event-Address:
[ 2964.842748] [<0000021761597924>] _printk+0x4c/0x58
[ 2964.842755] Kernel panic - not syncing: Fatal exception: panic_on_oops
Let me know if I can provide any other details.
>
> AFAIK the range passed into btrfs_compress_folios() should always have
> its start/length aligned to sector size.
Based on my tests, btrfs_compress_folios() input length (total_out) is not always a
multiple of PAGE_SIZE. One can see this when writing less than 4K of data to an
empty btrfs file.
>
> Since s390 only supports 4K page size, that means the range is always
> aligned to page size, and the existing code is also doing full page copy
> anyway, thus I see no problem with the existing read.
The code is doing full page copy to the workspace buffer for further compression. But the
number of bytes actually processed by zlib_deflate() is controlled by strm.avail_in
parameter.
>
> Thanks,
> Qu
>
>> Fix strm.avail_in calculation for S390 hardware acceleration path.
>>
>> Signed-off-by: Mikhail Zaslonko <zaslonko@xxxxxxxxxxxxx>
>> Fixes: fd1e75d0105d ("btrfs: make compression path to be subpage compatible")
>> ---
>> fs/btrfs/zlib.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
>> index ddf0d5a448a7..c9e92c6941ec 100644
>> --- a/fs/btrfs/zlib.c
>> +++ b/fs/btrfs/zlib.c
>> @@ -174,10 +174,10 @@ int zlib_compress_folios(struct list_head *ws, struct address_space *mapping,
>> copy_page(workspace->buf + i * PAGE_SIZE,
>> data_in);
>> start += PAGE_SIZE;
>> - workspace->strm.avail_in =
>> - (in_buf_folios << PAGE_SHIFT);
>> }
>> workspace->strm.next_in = workspace->buf;
>> + workspace->strm.avail_in = min(bytes_left,
>> + in_buf_folios << PAGE_SHIFT);
>> } else {
>> unsigned int pg_off;
>> unsigned int cur_len;
>
