Bug 216372
| Summary: | quota: Invalid block number reading from quota file after written faiulre | ||
|---|---|---|---|
| Product: | File System | Reporter: | Zhihao Cheng (chengzhihao1) |
| Component: | Other | Assignee: | fs_other |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | normal | CC: | brauner, jack |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 6.0.0-rc1 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
| Attachments: |
test.sh
diff b.c |
||
|
Description
Zhihao Cheng
2022-08-17 08:30:42 UTC
Created attachment 301588 [details]
test.sh
Created attachment 301589 [details]
diff
Created attachment 301590 [details]
b.c
(In reply to Zhihao Cheng from comment #0) 0. gcc -o bb b.c > > 1. Apply diff > 2. ./test.sh Is this reproducible with v5.19? 在 2022/8/17 17:17, bugzilla-daemon@kernel.org 写道: > https://bugzilla.kernel.org/show_bug.cgi?id=216372 > > --- Comment #5 from Christian Brauner (brauner@kernel.org) --- > Is this reproducible with v5.19? > I guess yes. I found this problem firstly in 5.10-lts. PS: I'm trying to fix it. Init: v2_read_file_info: <3> dqi_free_blk 0 dqi_free_entry 5 dqi_blks 6 Step 1. chown bin f_a -> dquot_acquire -> v2_write_dquot: qtree_write_dquot do_insert_tree find_free_dqentry get_free_dqblk write_blk(info->dqi_blocks) // info->dqi_blocks = 6, failure. The content in physical block (corresponding to blk 6) is random. Step 2. chown root f_a -> dquot_transfer -> dqput_all -> dqput -> ext4_release_dquot -> v2_release_dquot -> qtree_delete_dquot: dquot_release remove_tree free_dqentry put_free_dqblk(6) info->dqi_free_blk = blk // info->dqi_free_blk = 6 Step 3. drop cache (buffer head for block 6 is released) Step 4. chown bin f_b -> dquot_acquire -> commit_dqblk -> v2_write_dquot: qtree_write_dquot do_insert_tree find_free_dqentry get_free_dqblk dh = (struct qt_disk_dqdbheader *)buf blk = info->dqi_free_blk // 6 ret = read_blk(info, blk, buf) // The content of buf is random info->dqi_free_blk = le32_to_cpu(dh->dqdh_next_free) // random blk Step 5. chown bin f_c -> notify_change -> ext4_setattr -> dquot_transfer: dquot = dqget -> acquire_dquot -> ext4_acquire_dquot -> dquot_acquire -> commit_dqblk -> v2_write_dquot -> dq_insert_tree: do_insert_tree find_free_dqentry get_free_dqblk blk = info->dqi_free_blk // If blk < 0 and blk is not an error code, it will be returned as dquot transfer_to[USRQUOTA] = dquot // A random negative value __dquot_transfer(transfer_to) dquot_add_inodes(transfer_to[cnt]) spin_lock(&dquot->dq_dqb_lock) // page fault Thanks for the report. As far as I'm reading the reproducer, it basically corrupts the quota file (by cleverly inserting some IO failures). We have added some more sanity checking of data loaded from quota file after 5.10 (e.g. commit 9bf3d2033129 ("quota: check block number when reading the block in quota file") in 5.16). Now AFAICS we do not check validity of the free blocks list (we should check dqdh_next_free before using it) so your reproducer will still cause more damage than necessary. Will you send a patch to add such checks please?
(In reply to Jan Kara from comment #7) > Thanks for the report. As far as I'm reading the reproducer, it basically > corrupts the quota file (by cleverly inserting some IO failures). We have > added some more sanity checking of data loaded from quota file after 5.10 > (e.g. commit 9bf3d2033129 ("quota: check block number when reading the block > in quota file") in 5.16). Now AFAICS we do not check validity of the free > blocks list (we should check dqdh_next_free before using it) so your > reproducer will still cause more damage than necessary. Will you send a > patch to add such checks please? Yes, sure. I will send the fix patch as soon as possible, before that, I need do more verifications to make sure I find all situations. Patches were merged as:
6c8ea8b8cd47 ("quota: Check next/prev free block number after reading from quota file")
3fc61e0e96a3 ("quota: Replace all block number checking with helper function")
191249f70889 ("quota: Add more checking after reading from quota file")
Closing the bug.
|