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
| ||
|
Message-ID: <638AB59B.2030505@huawei.com> Date: Sat, 3 Dec 2022 10:34:03 +0800 From: "yebin (H)" <yebin10@...wei.com> To: Eric Whitney <enwlinux@...il.com>, Ye Bin <yebin@...weicloud.com> CC: <tytso@....edu>, <adilger.kernel@...ger.ca>, <linux-ext4@...r.kernel.org>, <linux-kernel@...r.kernel.org>, <jack@...e.cz>, <syzbot+05a0f0ccab4a25626e38@...kaller.appspotmail.com> Subject: Re: [PATCH v2 1/3] ext4: fix incorrect calculate 'reserved' in '__es_remove_extent' when enable bigalloc feature On 2022/12/2 6:21, Eric Whitney wrote: > * Ye Bin <yebin@...weicloud.com>: >> From: Ye Bin <yebin10@...wei.com> >> >> Syzbot report issue as follows: >> EXT4-fs error (device loop0): ext4_validate_block_bitmap:398: comm rep: bg 0: block 5: invalid block bitmap >> EXT4-fs (loop0): Delayed block allocation failed for inode 18 at logical offset 0 with max blocks 32 with error 28 >> EXT4-fs (loop0): This should not happen!! Data will be lost >> >> EXT4-fs (loop0): Total free blocks count 0 >> EXT4-fs (loop0): Free/Dirty block details >> EXT4-fs (loop0): free_blocks=0 >> EXT4-fs (loop0): dirty_blocks=32 >> EXT4-fs (loop0): Block reservation details >> EXT4-fs (loop0): i_reserved_data_blocks=2 >> EXT4-fs (loop0): Inode 18 (00000000845cd634): i_reserved_data_blocks (1) not cleared! >> >> Above issue happens as follows: >> Assume: >> sbi->s_cluster_ratio = 16 >> Step1: Insert delay block [0, 31] -> ei->i_reserved_data_blocks=2 >> Step2: >> ext4_writepages >> mpage_map_and_submit_extent -> return failed >> mpage_release_unused_pages -> to release [0, 30] >> ext4_es_remove_extent -> remove lblk=0 end=30 >> __es_remove_extent -> len1=0 len2=31-30=1 >> __es_remove_extent: >> ... >> if (len2 > 0) { >> ... >> if (len1 > 0) { >> ... >> } else { >> es->es_lblk = end + 1; >> es->es_len = len2; >> ... >> } >> if (count_reserved) >> count_rsvd(inode, lblk, orig_es.es_len - len1 - len2, &orig_es, &rc); >> goto out; -> will return but didn't calculate 'reserved' >> ... >> Step3: ext4_destroy_inode -> trigger "i_reserved_data_blocks (1) not cleared!" >> >> To solve above issue if 'len2>0' call 'get_rsvd()' before goto out. >> >> Reported-by: syzbot+05a0f0ccab4a25626e38@...kaller.appspotmail.com >> Signed-off-by: Ye Bin <yebin10@...wei.com> >> --- >> fs/ext4/extents_status.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c >> index cd0a861853e3..4684eaea9471 100644 >> --- a/fs/ext4/extents_status.c >> +++ b/fs/ext4/extents_status.c >> @@ -1371,7 +1371,7 @@ static int __es_remove_extent(struct inode *inode, ext4_lblk_t lblk, >> if (count_reserved) >> count_rsvd(inode, lblk, orig_es.es_len - len1 - len2, >> &orig_es, &rc); >> - goto out; >> + goto count; >> } >> >> if (len1 > 0) { >> @@ -1413,6 +1413,7 @@ static int __es_remove_extent(struct inode *inode, ext4_lblk_t lblk, >> } >> } >> >> +count: >> if (count_reserved) >> *reserved = get_rsvd(inode, end, es, &rc); >> out: >> -- >> 2.31.1 >> > I'm unable to find the sysbot report for this patch, so I can't verify that > this fix works. The more serious problem would be whatever is causing As I reproduce "[syzbot] memory leak in __insert_pending" issue , after merge my previous patch 1b8f787ef547 "ext4: fix warning in 'ext4_da_release_space'", I found there is no memleak but report "i_reserved_data_blocks not cleared". You can use C reproducer on linux-next to reproduce this issue. C reproducer:https://syzkaller.appspot.com/x/repro.c?x=13a9300a880000 > the invalid block bitmap and delayed allocation failure messages before the > i_reserved_data_blocks message. Perhaps that's simply what syzkaller set > up, but it's not clear from this posting. Have you looked for the cause > of those first two messages? > > However, by inspection this patch should fix an obvious bug causing that last > message, introduced by 8fcc3a580651 ("ext4: rework reserved cluster accounting > when invalidating pages"). A Fixes tag should be added to the patch. Also, > the readability of the code should be improved by changing the label "count" to > the more descriptive "out_get_reserved". > > With those two changes, feel free to add: > > Reviewed-by: Eric Whitney <enwlinux@...il.com> > > Eric > . >
Powered by blists - more mailing lists